Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/brave-brackets-split.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"wrangler": patch
---

Treat bracket-quoted identifiers as quoted when splitting D1 SQL files

The D1 SQL splitter handled `'`, `"` and backtick quoting but not SQLite's bracket-quoted identifiers (`[name]`), while `normalizeSqlLineEndings()` in the same file already did. A `;` inside such an identifier, e.g. `CREATE TABLE metrics ([value;unit] TEXT);`, was treated as a statement boundary, so `wrangler d1 execute --file` and `wrangler d1 migrations apply` sent broken fragments to D1. The scanner now skips over `[` … `]` like the other quote styles, which also keeps the new punctuation-delimited compound markers from matching keywords inside brackets, such as a `[end]` column in a trigger body.
9 changes: 9 additions & 0 deletions .changeset/olive-donuts-battle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"wrangler": patch
---

Recognise compound statement markers that are not padded with whitespace

`wrangler d1 execute --file` and `wrangler d1 migrations apply` split a SQL file into statements before sending them to D1. The splitter only recognised `BEGIN`, `CASE` and `END` when they were surrounded by whitespace, so SQL that SQLite accepts, such as a trigger body ending in `INSERT ...;END;` or a trigger declared with `WHEN (1=1)BEGIN`, was split incorrectly: statements after the trigger were swallowed into it and silently sent as a single statement.

Markers are now matched when delimited by punctuation as well, while identifiers that merely end in a keyword, such as a `weekend` table, are still left alone.
119 changes: 119 additions & 0 deletions packages/wrangler/src/__tests__/d1/splitter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,17 @@ describe("splitSqlQuery()", () => {
`);
});

it("should handle bracket-quoted identifiers", ({ expect }) => {
expect(
splitSqlQuery(`
CREATE TABLE metrics ([value;unit] TEXT);
SELECT [value;unit] FROM metrics;`)
).toEqual([
"CREATE TABLE metrics ([value;unit] TEXT)",
"SELECT [value;unit] FROM metrics",
]);
});

it("should handle inline comments", ({ expect }) => {
expect(
splitSqlQuery(
Expand Down Expand Up @@ -277,6 +288,114 @@ describe("splitSqlQuery()", () => {
`);
});

it("should end a compound statement when END follows a semicolon", ({
expect,
}) => {
expect(
splitSqlQuery(`
CREATE TRIGGER audit_trigger AFTER INSERT ON items
BEGIN
INSERT INTO audit (item_id) VALUES (new.id);END;
INSERT INTO items (id) VALUES (1);
SELECT * FROM items;`)
).toEqual([
`CREATE TRIGGER audit_trigger AFTER INSERT ON items
BEGIN
INSERT INTO audit (item_id) VALUES (new.id);END`,
"INSERT INTO items (id) VALUES (1)",
"SELECT * FROM items",
]);
});

it("should start a compound statement when BEGIN follows a parenthesis", ({
expect,
}) => {
expect(
splitSqlQuery(`
CREATE TRIGGER audit_trigger AFTER INSERT ON items FOR EACH ROW WHEN (1=1)BEGIN
INSERT INTO audit (item_id) VALUES (new.id);
END;
SELECT * FROM items;`)
).toEqual([
`CREATE TRIGGER audit_trigger AFTER INSERT ON items FOR EACH ROW WHEN (1=1)BEGIN
INSERT INTO audit (item_id) VALUES (new.id);
END`,
"SELECT * FROM items",
]);
});

it("should end a CASE expression closed by a parenthesis or comma", ({
expect,
}) => {
expect(
splitSqlQuery(`
SELECT SUM(CASE WHEN a THEN 1 ELSE 0 END) FROM t;
SELECT CASE WHEN a THEN 1 ELSE 0 END, b FROM t;
SELECT * FROM t;`)
).toEqual([
"SELECT SUM(CASE WHEN a THEN 1 ELSE 0 END) FROM t",
"SELECT CASE WHEN a THEN 1 ELSE 0 END, b FROM t",
"SELECT * FROM t",
]);
});

it("should not treat an identifier ending in END as a compound statement end", ({
expect,
}) => {
expect(
splitSqlQuery(`
CREATE TABLE weekend (id INTEGER PRIMARY KEY);
INSERT INTO weekend (id) VALUES (1);
SELECT * FROM weekend;`)
).toEqual([
"CREATE TABLE weekend (id INTEGER PRIMARY KEY)",
"INSERT INTO weekend (id) VALUES (1)",
"SELECT * FROM weekend",
]);
});

it("should not treat an accented identifier ending in END as a compound statement end", ({
expect,
}) => {
expect(
splitSqlQuery(`
CREATE TRIGGER t AFTER INSERT ON x
BEGIN
UPDATE y SET a = 1 WHERE b = néend;
UPDATE z SET c = 2;
END;
SELECT 1;`)
).toEqual([
`CREATE TRIGGER t AFTER INSERT ON x
BEGIN
UPDATE y SET a = 1 WHERE b = néend;
UPDATE z SET c = 2;
END`,
"SELECT 1",
]);
});

it("should not treat a bracket-quoted identifier as a compound statement marker", ({
expect,
}) => {
expect(
splitSqlQuery(`
CREATE TRIGGER t AFTER INSERT ON items
BEGIN
UPDATE x SET [end] = 1;
UPDATE y SET z = 2;
END;
SELECT 1;`)
).toEqual([
`CREATE TRIGGER t AFTER INSERT ON items
BEGIN
UPDATE x SET [end] = 1;
UPDATE y SET z = 2;
END`,
"SELECT 1",
]);
});

it("should handle compound statements for BEGINs", ({ expect }) => {
expect(
splitSqlQuery(`
Expand Down
19 changes: 17 additions & 2 deletions packages/wrangler/src/d1/splitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,10 @@ function splitSqlIntoStatements(sql: string): string[] {
case "`":
str += char + consumeUntilMarker(iterator, char);
break;
case `[`:
// SQLite bracket-quoted identifiers end at the first `]`; there is no escape sequence.
str += char + consumeUntilMarker(iterator, `]`);
break;
case `$`: {
const dollarQuote =
"$" + consumeWhile(iterator, isDollarQuoteIdentifier);
Expand Down Expand Up @@ -247,16 +251,27 @@ function isDollarQuoteIdentifier(str: string) {
);
}

/**
* Compound statement markers only need to be delimited from surrounding
* identifiers, not padded with whitespace: SQLite accepts `WHEN (1=1)BEGIN` and
* `INSERT ...;END;`. The lookbehind keeps identifiers that merely end in the
* keyword, such as a `weekend` column, from matching; identifier characters
* include letters with diacritical marks, as in `isDollarQuoteIdentifier()`,
* so a `néend` column does not match either.
*/
const COMPOUND_STATEMENT_START = /(?<![\p{L}\p{N}_$])(BEGIN|CASE)\s$/iu;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Parenthesized CASE breaks trigger splitting

When a trigger contains CASE(, COMPOUND_STATEMENT_START misses the case expression. Its END closes the trigger early, splitting remaining statements apart.

Prompt for agents
Update the compound-statement start matching in packages/wrangler/src/d1/splitter.ts so BEGIN and CASE are recognized when followed by any valid non-identifier delimiter, not only whitespace. Preserve the guard against keyword suffixes in identifiers. Add a splitter regression test with a trigger containing CASE(expression) and at least one subsequent statement inside the trigger, followed by another top-level statement, so premature closure is observable.
Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

const COMPOUND_STATEMENT_END = /(?<![\p{L}\p{N}_$])END[^\p{L}\p{N}_$]$/iu;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Decomposed identifiers break trigger splitting

With a combining accent before end, COMPOUND_STATEMENT_END mistakes part of an identifier for the terminator. Remaining trigger statements become invalid commands.

Prompt for agents
Include Unicode combining marks in the identifier character class used by both compound marker regexes in packages/wrangler/src/d1/splitter.ts. Add a regression test using an unquoted decomposed Unicode identifier whose suffix is `end` inside a multi-statement trigger, then verify the trigger and a following top-level statement remain correctly split.
Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


/**
* Returns true if the `str` ends with a compound statement `BEGIN` or `CASE` marker.
*/
function isCompoundStatementStart(str: string) {
return /\s(BEGIN|CASE)\s$/i.test(str);
return COMPOUND_STATEMENT_START.test(str);
}

/**
* Returns true if the `str` ends with a compound statement `END` marker.
*/
function isCompoundStatementEnd(str: string) {
return /\sEND[;\s]$/i.test(str);
return COMPOUND_STATEMENT_END.test(str);
}
Loading