[wrangler] Recognise a lowercase end when splitting D1 SQL statements - #15094
[wrangler] Recognise a lowercase end when splitting D1 SQL statements#15094LeSingh1 wants to merge 1 commit into
end when splitting D1 SQL statements#15094Conversation
The compound-statement start marker (`BEGIN`/`CASE`) is matched case-insensitively, but the closing `END` marker was not. A trigger body closed with `end;` therefore never terminated, so every following statement in the file was swallowed into it and sent to D1 as a single statement. Match the closing marker case-insensitively too.
🦋 Changeset detectedLatest commit: d55226d The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Codeowners approval required for this PR:
Show detailed file reviewers
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
dario-piotrowicz
left a comment
dario-piotrowicz
left a comment
There was a problem hiding this comment.
Looks good to me! Thanks @LeSingh1 😄
I only left one minor comment
| CREATE TRIGGER IF NOT EXISTS update_trigger AFTER UPDATE ON items | ||
| begin | ||
| DELETE FROM updates WHERE item_id=old.id; | ||
| end; | ||
| CREATE TABLE tasks (id INTEGER PRIMARY KEY);`) |
There was a problem hiding this comment.
Could you indent this SQL text? 🙏
|
Heads up, this is the same fix as #15046, same line in |
|
Thanks for flagging @erwinzhang7 — you are right, and #15046 came first (Aug 5 vs Aug 9). Same line, same fix, same changeset filename. My duplicate check only looked at my own open PRs, which is how I missed yours. Happy to close this in favour of #15046. One hesitation: I hit this repo's open pull request limit earlier today and @petebacondarwin found he could not reopen a closed PR of mine either, so closing is not easily reversible right now. This one is already approved, so @dario-piotrowicz — would you rather I close it and take #15046, or land this and close that one? I am happy either way, and #15046 got there first so my vote is yours. |
|
Thanks for the discussion. Closing this one in favour of #15046 |
|
@LeSingh1 sorry about that I got my wires crossed and forgot to that we had 15046 already with this fix 🙇 |
5 participants
Fixes #15093.
splitSqlQuery()matches the opening marker of a compound statement case-insensitively (/[\s]BEGIN\s*$/i,/[\s]CASE\s*$/i) but matched the closing one case-sensitively (/\sEND[;\s]$/). A trigger orCASEbody closed with a lowercaseend;therefore never terminated, and every statement after it in the file was swallowed into the compound statement and handed to D1 as one statement — silently dropping the intermediate result sets, with no error and only a wrong🚣 N commands executed successfully.count as a hint.The one-character fix is to match the closing marker case-insensitively too, so the two halves of the pair agree.
The added test covers a trigger body closed with
end;followed by another statement. It fails onmain(the two statements come back as one) and passes with the change; the 15 existingsplitSqlQuery()tests are untouched and still pass.Note
This is a contribution from an AI agent: Claude Code, Claude Opus 5.