docs: log silent-drop audit findings (TODO follow-ups from PR #81)#82
Merged
Conversation
Audit triggered by the inline column-level PRIMARY KEY / UNIQUE bug fix on PR #81. Probed adjacent column / table / index attributes that vitess parses but myschema might silently drop. Results: High (silent data loss, not just feature drop): - vitess `Parse2` swallows some syntax errors as WARN logs and returns a partially-parsed AST; myschema treats that as success and can emit an empty `CREATE TABLE t ();` for inputs vitess rejects. Repro: COLUMN_FORMAT + STORAGE on a column. Medium (silent fidelity gaps; parser populates the model but the emitter / catalog reader don't): - Column-level INVISIBLE (MySQL 8.0+) dropped — catalog has it via EXTRA, parser side doesn't. - SRID on spatial columns dropped — catalog surfaces it via ST_GEOMETRY_COLUMNS, parser side doesn't read `Options.SRID`. - Table-level ROW_FORMAT / KEY_BLOCK_SIZE / COMPRESSION / ENCRYPTION dropped — `applyTableOption` only handles ENGINE / CHARSET / COLLATE / COMMENT / AUTO_INCREMENT. - Index-level KEY_BLOCK_SIZE dropped — `addIndex` doesn't read the option, `Index.SQL()` doesn't emit it. Each entry includes the repro path, where in the code to look, and the fix sketch. Logged to TODO so the next contributor working in that area can pick them up; no code change here. 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 #82 +/- ##
=======================================
Coverage 86.62% 86.62%
=======================================
Files 30 30
Lines 3237 3237
=======================================
Hits 2804 2804
Misses 267 267
Partials 166 166 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR documents follow-up audit findings in TODO.md after the parser fix from PR #81, so future work on schema-fidelity gaps has a prioritized backlog.
Changes:
- Adds a new high-severity TODO entry for partially parsed DDL being treated as success.
- Adds medium-severity TODO entries for unmodeled column, table, and index attributes.
- Organizes the audit notes by severity and links them to likely implementation touchpoints.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+45
to
+50
| visible column. Drift never closes because the catalog reads | ||
| `information_schema.COLUMNS.EXTRA` (which has `INVISIBLE`) | ||
| while the parser side has nothing. Fix: surface | ||
| `Options.Invisible` on `model.Column`, emit `INVISIBLE` in | ||
| `columnDefSQL` when set, populate from `EXTRA` in the catalog | ||
| reader. Found during the PR #81 silent-drop audit. |
Comment on lines
+57
to
+61
| round-trip — and the catalog *does* surface SRID via | ||
| `information_schema.ST_GEOMETRY_COLUMNS`, so a desired-side | ||
| schema with `SRID 4326` constants drifts forever. Fix: add | ||
| SRID to `model.Column`, emit in `columnDefSQL`, populate from | ||
| the spatial catalog view. Found during the PR #81 audit. |
Comment on lines
+65
to
+76
| `COLLATE`, `COMMENT`, `AUTO_INCREMENT`. Real production | ||
| attributes that affect on-disk layout / encryption posture | ||
| silently drop on the desired side and continue to drift on | ||
| every plan: | ||
| - `ROW_FORMAT={COMPACT|DYNAMIC|COMPRESSED|REDUNDANT}` — | ||
| affects InnoDB on-disk layout. | ||
| - `KEY_BLOCK_SIZE=N` — page size for compressed InnoDB tables. | ||
| - `COMPRESSION='ZLIB|LZ4|...'` — InnoDB transparent | ||
| compression (8.0+). | ||
| - `ENCRYPTION='Y|N'` — InnoDB transparent encryption (8.0+). | ||
| Decide which are in scope, then add them to `model.Table`, | ||
| `applyTableOption`, the catalog reader, and `Table.SQL()`. |
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
Audit triggered by the inline column-level PRIMARY KEY / UNIQUE bug fix on PR #81. Probed adjacent column / table / index attributes that vitess parses but myschema might silently drop. Adds five entries to
TODO.md, organised by severity:High — silent data loss
Parse2swallows some syntax errors as WARN logs and returns a partially-parsed AST; myschema treats that as success and can emit an emptyCREATE TABLE t ();for inputs vitess actually rejects. Repro:COLUMN_FORMAT DYNAMIC STORAGE DISKon a column. This is silent data loss, not just feature drop.Medium — silent fidelity gaps
INVISIBLE(MySQL 8.0+) dropped — catalog has it viaEXTRA, parser side doesn't readOptions.Invisible.SRID <n>on spatial columns dropped — catalog surfaces it viaST_GEOMETRY_COLUMNS, parser side doesn't readOptions.SRID.ROW_FORMAT/KEY_BLOCK_SIZE/COMPRESSION/ENCRYPTIONdropped —applyTableOptiononly handlesENGINE/CHARSET/COLLATE/COMMENT/AUTO_INCREMENT.KEY_BLOCK_SIZEdropped —addIndexdoesn't read the option,Index.SQL()doesn't emit it.Each entry includes the repro path, where in the code to look, and a fix sketch. No code change in this PR — these are follow-ups for separate work.
Test plan
TODO.mddoc-only edit.🤖 Generated with Claude Code