Skip to content

fix(wrangler): match D1 compound END case-insensitively - #15091

Closed
HaoChiBao wants to merge 2 commits into
cloudflare:mainfrom
HaoChiBao:fix/d1-splitter-end-case-insensitive
Closed

fix(wrangler): match D1 compound END case-insensitively#15091
HaoChiBao wants to merge 2 commits into
cloudflare:mainfrom
HaoChiBao:fix/d1-splitter-end-case-insensitive

Conversation

@HaoChiBao

@HaoChiBao HaoChiBao commented Aug 8, 2026

Copy link
Copy Markdown

Summary

  • isCompoundStatementStart already matches BEGIN/CASE case-insensitively, but isCompoundStatementEnd required uppercase END.
  • A lowercase end; left the splitter inside a compound statement and broke statement boundaries (e.g. triggers + following SQL).
  • Add a regression test for lowercase end.

Test plan

  • Added splitter unit coverage for lowercase end;
  • CI: packages/wrangler d1 splitter tests

Open in Devin Review

BEGIN/CASE already use the i flag; lowercase end; left the splitter
inside a trigger body and broke statement boundaries.
@changeset-bot

changeset-bot Bot commented Aug 8, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 266d3cf

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@workers-devprod
workers-devprod requested review from a team and jamesopstad and removed request for a team August 8, 2026 19:14
@workers-devprod

Copy link
Copy Markdown
Contributor

Codeowners approval required for this PR:

  • @cloudflare/d1
  • @cloudflare/wrangler
Show detailed file reviewers
  • packages/wrangler/src/tests/d1/splitter.test.ts: [@cloudflare/d1 @cloudflare/wrangler]
  • packages/wrangler/src/d1/splitter.ts: [@cloudflare/d1 @cloudflare/wrangler]

@devin-ai-integration devin-ai-integration Bot left a comment

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.

Devin Review found 1 potential issue.

Open in Devin Review

*/
function isCompoundStatementEnd(str: string) {
return /\sEND[;\s]$/.test(str);
return /\sEND[;\s]$/i.test(str);

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.

🟡 Bug fix to a published package ships without a changeset, so users never get the release note

The behavior fix to D1 statement splitting (/\sEND[;\s]$/i at packages/wrangler/src/d1/splitter.ts:169) is added without any accompanying changeset entry, so the fix will not trigger a release or appear in the changelog.
Impact: Users won't see the fix documented and it may not be published.

Repository rule requiring changesets for published package changes

AGENTS.md states "All changes to published packages require a changeset" and CONTRIBUTING.md requires a changeset for every non-trivial change. The PR touches packages/wrangler/src/d1/splitter.ts (a published package's source) but .changeset/ only contains README.md, config.json, and an unrelated dependabot-update-15072.md. A patch changeset for wrangler describing the user-facing fix should be added.

Prompt for agents
Add a patch changeset for the wrangler package describing this user-facing bug fix: D1 SQL files using a lowercase 'end;' to close a compound statement (e.g. triggers) were previously split incorrectly. Follow .changeset/README.md rules: no conventional-commit prefix in the title, focus on user impact, no h1/h2/h3 headers.
Open in Devin Review

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

Keep the case-insensitive END regression in its own test case.

@petebacondarwin petebacondarwin left a comment

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.

Thanks for this. Please add a changeset as is described in our Contributing guidelines.

@petebacondarwin
petebacondarwin marked this pull request as draft August 11, 2026 11:54
@petebacondarwin
petebacondarwin removed request for a team and jamesopstad August 11, 2026 11:54
@alsuren

alsuren commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

This seems to duplicate #15094 , which I already approved and only needs minor tweaks.

To avoid duplication of work: please follow that PR, and only re-open this PR if that one stalls and gets closed.

@alsuren alsuren closed this Aug 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants