parser/test + docs: close out the column-loop interactions audit#99
Merged
Conversation
Last open audit item from PR #84 postmortem. Two sub-bullets, both verified against the current code: - inline `UNIQUE × DEFAULT 'x'`: vitess parses it cleanly, parseColumnDef wires DEFAULT onto the column, and applyInlineColumnKey promotes UNIQUE to a column-named index. Both attributes survive in the model independently. Pin with `TestParseInlineColumnUniqueWithDefault`. - inline `PRIMARY KEY × CHECK (col > 0)`: the TODO entry framed this as "vitess promotes column-level CHECK to a table-level constraint — the promotion path interacts with the PK promotion path." The premise was wrong: vitess does not parse column-level CHECK at all (every variant — `id INT CHECK (...)`, `id INT NOT NULL CHECK (...)`, `id INT PRIMARY KEY CHECK (...)` — fails with `syntax error near 'CHECK'`). MySQL itself accepts the shape and stores the constraint under an auto-name; vitess simply hasn't implemented the column-level grammar. There is no reachable interaction to test. Drop the TODO entry now that one sub-bullet is pinned and the other is unreachable. CAVEATS.md gets a new "Inline column-level CHECK is rejected by the parser" section so users who hit the vitess error have a clear diagnosis and the table-level workaround. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #99 +/- ##
=======================================
Coverage 86.96% 86.96%
=======================================
Files 30 30
Lines 3321 3321
=======================================
Hits 2888 2888
Misses 266 266
Partials 167 167 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Closes the last open audit items from the PR #84 postmortem around parseCreateTable “column-loop interactions”, by pinning the reachable UNIQUE×DEFAULT interaction in a unit test and documenting that inline column-level CHECK is not supported by the Vitess parser (despite MySQL accepting it).
Changes:
- Add a parser unit test that asserts inline
UNIQUEpromotion andDEFAULTparsing both survive on the same column. - Remove the now-resolved/invalidated “Medium — test coverage gaps” TODO section.
- Document the MySQL-vs-Vitess asymmetry for inline column-level
CHECK, with a table-level workaround.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
parser/parser_test.go |
Adds TestParseInlineColumnUniqueWithDefault to lock in UNIQUE×DEFAULT composed behavior. |
TODO.md |
Removes the completed/invalidated audit item section. |
CAVEATS.md |
Adds guidance that inline column-level CHECK is rejected by Vitess and recommends table-level CHECK. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The new "Inline column-level CHECK is rejected by the parser" section claimed users see the vitess error "verbatim", but ParseSQL wraps the underlying error with "failed to parse SQL: %w" before surfacing it. Update the wording to spell out the wrapping so a user grepping the actual error message finds the right entry. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Last open audit item from the PR #84 postmortem ("parseCreateTable column-loop interactions"). Two sub-bullets remained; verified each against the current code:
Sub-bullet 1: `inline UNIQUE × DEFAULT 'x'` — testable, pinned
vitess parses the shape cleanly. `parseColumnDef` wires the DEFAULT onto the column; `applyInlineColumnKey` promotes UNIQUE to a column-named index. Both attributes survive in the model independently. Added `TestParseInlineColumnUniqueWithDefault` to pin the regression net.
Sub-bullet 2: `inline PRIMARY KEY × CHECK (col > 0)` — unreachable, dropped
The TODO framing said "vitess promotes column-level CHECK to a table-level constraint." Probing showed the premise is wrong: vitess does not parse column-level CHECK at all. Every variant fails with `syntax error near 'CHECK'`:
```
id INT CHECK (...) → syntax error
id INT NOT NULL CHECK (...) → syntax error
id INT PRIMARY KEY CHECK (...) → syntax error
```
MySQL itself accepts these shapes and stores the constraint under `
chk`, but vitess hasn't implemented the column-level grammar. There is no reachable code path to test.Changes
Test plan
🤖 Generated with Claude Code