parser: reject inline column-level REFERENCES at parse time by winebarrel · Pull Request #92 · winebarrel/myschema

winebarrel · 2026-05-06T00:51:01Z

Summary

MySQL silently ignores inline column-level `REFERENCES` (the SQL-standard `col INT REFERENCES other(id)` shape). The official 8.0 manual (ansi-diff-foreign-keys.html) calls it "a memo or comment ... no actual effect."

myschema previously rescued the shape by promoting it to an explicit-named `ALTER TABLE ADD CONSTRAINT`, but that produced two real bugs:

The user thought they had an FK; MySQL alone wouldn't have created one. myschema's desired-side parse diverged from "what MySQL would do with this same SQL."
The rescue's auto-name (`ibfk`) didn't match MySQL's auto-name for table-level unnamed FKs created externally (`
ibfk`), so importing an externally-managed schema produced a one-shot no-op DROP+ADD on first plan (the bug filed under TODO.md "Medium — silent diffs / fidelity gaps" → "Inline column-level REFERENCES auto-names ...").
This PR rejects the shape outright with a helpful error pointing at the table-level form.

Changes
- `parseColumnDef` errors when `cd.Type.Options.Reference != nil`.
- Dead rescue branch in `parseCreateTable` removed; the table-level `FOREIGN KEY (...)` path (handled by `addTableConstraint`) is unaffected.
- `TestParseInlineForeignKey` → `TestParseInlineColumnReferencesRejected`.
- `CAVEATS.md` gets a new "Inline column-level REFERENCES is rejected" entry; `AGENTS.md` "In scope (v1)" trims `REFERENCES` from the inline-column list.
- `testdata/apply/kitchen_sink_round_trip.yml` swaps the auto-name-drift rationale comment for the parse-time rejection one.
- `TODO.md` drops the now-fixed bug entry.
Breaking change

Users who previously wrote inline `col TYPE REFERENCES other(col)` in desired SQL will now see a parse error. The fix is mechanical — move the clause to a table-level `[CONSTRAINT name] FOREIGN KEY (col) REFERENCES other(col)` line. `mysqldump` output already uses the table-level form, so dump → apply round-trips are unaffected.

Test plan
- `make lint` clean.
- `make test` all 6 packages green.
- `make test-scenario` 5/5 passing.
- New `TestParseInlineColumnReferencesRejected` asserts the error message points at the table-level alternative.
🤖 Generated with Claude Code

MySQL parses but ignores inline column-level REFERENCES (e.g. `user_id BIGINT REFERENCES users(id)`); the official 8.0 reference manual `ansi-diff-foreign-keys.html` calls it "a memo or comment ... no actual effect." Earlier myschema rescued the shape by promoting it to an explicit-named ALTER TABLE ADD CONSTRAINT, but that diverged from MySQL's semantics on two fronts: - The user thought they had an FK; MySQL alone wouldn't have created one. myschema's desired-side parse drifted from "what MySQL would do with the same SQL." - The rescued FK's auto-name (`<table>_ibfk_<col>`) didn't match MySQL's auto-name for table-level unnamed FKs created externally (`<table>_ibfk_<n>`), so importing an externally-managed schema saw a one-shot no-op DROP+ADD on first plan. Reject the shape outright with a helpful error pointing at the table-level form. Knock-on changes: - parseColumnDef errors when `cd.Type.Options.Reference != nil`. - The dead rescue branch in parseCreateTable is removed; the table-level `FOREIGN KEY (...)` path (handled by addTableConstraint) is unaffected and still uses `_ibfk_<col>` auto-naming. - TestParseInlineForeignKey rewritten as TestParseInlineColumnReferencesRejected. - CAVEATS.md gets a new "Inline column-level REFERENCES is rejected" entry; AGENTS.md "In scope (v1)" trims REFERENCES from the inline-column list. - kitchen_sink_round_trip.yml's "intentional omissions" comment swaps the auto-name-drift rationale for the parse-time rejection one. - TODO.md drops the now-fixed bug entry. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

codecov · 2026-05-06T00:52:40Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.00%. Comparing base (b716b4c) to head (9a987ab).

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #92      +/-   ##
==========================================
+ Coverage   86.90%   87.00%   +0.09%     
==========================================
  Files          30       30              
  Lines        3277     3271       -6     
==========================================
- Hits         2848     2846       -2     
+ Misses        263      261       -2     
+ Partials      166      164       -2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull request overview

This PR tightens desired-SQL fidelity with MySQL by rejecting inline column-level REFERENCES during parsing, since MySQL accepts but ignores that syntax. This prevents myschema from “rescuing” a clause into a real FK (and potentially introducing name drift vs externally-created schemas).

Changes:

Reject inline column-level REFERENCES at parse time with an actionable error message pointing users to the table-level FOREIGN KEY (...) REFERENCES ... form.
Remove the previous inline-FK rescue logic and update/rename the parser test accordingly.
Update docs/fixtures to reflect the new rejection behavior and remove the related TODO entry.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
TODO.md	Removes the previous inline-`REFERENCES` auto-name drift TODO entry.
testdata/apply/kitchen_sink_round_trip.yml	Updates fixture commentary to match the new parse-time rejection behavior.
parser/parser.go	Drops inline-FK rescue branch and adds a parse-time rejection in `parseColumnDef`.
parser/parser_test.go	Renames/repurposes the inline-`REFERENCES` test to assert rejection and check the error message.
CAVEATS.md	Documents that inline column-level `REFERENCES` is rejected and explains rationale/workarounds.
AGENTS.md	Updates “In scope (v1)” docs to remove inline `REFERENCES` and point to the caveat.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- parser_test.go: drop the stale leading comment that still referenced the renamed `TestParseInlineForeignKey` and the rescue-side auto-naming claim. - CAVEATS.md: add a "Note on the unnamed table-level form" flagging that myschema's parser auto-name (`_ibfk_<col>`) differs from MySQL's auto-name (`_ibfk_<n>`), so importing an externally-created schema with unnamed table-level FKs produces a one-shot no-op DROP+ADD on the first plan. Recommend explicit `CONSTRAINT fk_xxx` names or `dump → apply` to skip it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- parser.go: error message now embeds the full URL (https://dev.mysql.com/doc/refman/8.0/en/ansi-diff-foreign-keys.html) instead of the abbreviated `dev.mysql.com docs ansi-diff-foreign-keys` so users can click or grep it directly. - CAVEATS.md: replace the lossy `…` placeholder in the error example with the exact emitted string (just wrapped for display width), and label it as such. The "Why" section also turns the page citation into a proper markdown link. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Three accuracy fixes in CAVEATS.md: - The "exact error string" block omitted the `table <db>.<table>: column <col>:` prefix that parseCreateTable wraps around column-level errors. Updated to show the full wrapped form so a user grepping the error message sees what they'll actually encounter. - "no-op DROP+ADD" was misleading in two places: dropping + re-adding an FK is real DDL (brief table lock + full row re-verification against the FK), not a no-op in execution cost — only the end state is functionally equivalent. Reworded to "redundant DROP+ADD" with a brief note on the actual operational impact, both in the "rescue regression" rationale and in the unnamed-table-level-FK note. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

PR #92 made inline column-level `REFERENCES` error at parse time (CAVEATS.md "Inline column-level REFERENCES is rejected"), so the third sub-bullet under "parseCreateTable column-loop interactions" — `inline REFERENCES × NOT NULL × ON UPDATE` — is no longer a reachable code shape. Drop the bullet, trim the buildFK reference from the parent description, and add a note explaining why the combination went away so the audit history stays legible. The other two sub-bullets (inline UNIQUE × DEFAULT, inline PK × column-level CHECK) are unaffected and still open. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings May 6, 2026 00:51

Copilot started reviewing on behalf of winebarrel May 6, 2026 00:51 View session

Copilot AI reviewed May 6, 2026

View reviewed changes

Comment thread parser/parser_test.go Outdated

Comment thread CAVEATS.md

winebarrel requested review from Copilot May 6, 2026 01:00

Copilot started reviewing on behalf of winebarrel May 6, 2026 01:01 View session

Copilot AI reviewed May 6, 2026

View reviewed changes

Comment thread parser/parser.go Outdated

Comment thread CAVEATS.md

winebarrel requested a review from Copilot May 6, 2026 01:09

Copilot started reviewing on behalf of winebarrel May 6, 2026 01:10 View session

Copilot AI reviewed May 6, 2026

View reviewed changes

Comment thread CAVEATS.md Outdated

Comment thread CAVEATS.md Outdated

Comment thread CAVEATS.md Outdated

winebarrel mentioned this pull request May 6, 2026

parser: promote inline bare column-level KEY to PRIMARY #94

Merged

3 tasks

winebarrel requested a review from Copilot May 6, 2026 01:21

Copilot started reviewing on behalf of winebarrel May 6, 2026 01:22 View session

Copilot AI reviewed May 6, 2026

View reviewed changes

winebarrel merged commit 78eb2df into main May 6, 2026
9 checks passed

winebarrel deleted the parser-reject-inline-references branch May 6, 2026 01:24

winebarrel mentioned this pull request May 6, 2026

docs: drop now-unreachable inline REFERENCES test-coverage entry #97

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

parser: reject inline column-level REFERENCES at parse time#92

parser: reject inline column-level REFERENCES at parse time#92
winebarrel merged 4 commits into
mainfrom
parser-reject-inline-references

winebarrel commented May 6, 2026

Uh oh!

codecov Bot commented May 6, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

winebarrel commented May 6, 2026

Summary

Changes

Breaking change

Test plan

Uh oh!

codecov Bot commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov Bot commented May 6, 2026 •

edited

Loading