feat(plugins): exclude the AUTO_INCREMENT counter and DEFINER clauses from an SQL export - #2608
Merged
Merged
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
… from an SQL export Claude-Session: https://claude.ai/code/session_01J6xU4Zx4DRJ5JaxMP437uT
datlechin
force-pushed
the
feat/sql-export-portable-ddl
branch
from
September 2, 2026 11:52
6e4bed1 to
e38c728
Compare
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.
Fixes #2516
Root cause
SQLExportPlugin.writeCreatePhasewrote whateverdataSource.fetchTableDDLreturned, byte for byte. On MySQL and MariaDB that string carries two clauses that belong to the source server, and there was no stage between the driver and the file that could drop either:AUTO_INCREMENT=<n>, which is the source server's next key value;DEFINER=<user>@<host>, which names an account the target may not have.Measured on a MariaDB 12.3 instance:
SHOW CREATE TABLE users) ENGINE=InnoDB AUTO_INCREMENT=4 DEFAULT CHARSET=utf8mb4 …SHOW CREATE TABLE v_users(views go through the same driver call)CREATE ALGORITHM=UNDEFINED DEFINER=\root`@`localhost` SQL SECURITY DEFINER VIEW …`ERROR 1227 (42000): Access denied; you need (at least one of) the SET USER privilege(s) for this operationERROR 1446 (HY000): The user specified as a definer ('ghost'@'nowhere') does not existTwo things the issue assumes are not the case, and the fix is scoped accordingly. SQL export never writes triggers or routines: it exports tables and views only, so a view is the only DEFINER carrier on this path. And a regex is not safe here, because a real
SHOW CREATE TABLEreportsAUTO_INCREMENT=5inside a columnCOMMENTand inside a quoted column name.The fix
SQLExportDDLRewriteris a pure, quote- and comment-aware scanner that removes the two clauses from a statement before it is written. Two exclusions,Exclude the AUTO_INCREMENT counterandExclude DEFINER clauses, both on by default and stored with the rest of the format's options.Each clause is taken only in the position its grammar puts it:
CREATEand the object keyword alone.Neither is taken inside
'…',"…",`…`,--,#or/* … */. The whole rewrite is gated to the MySQL dialect, so every other engine is handed its DDL back untouched.The column-level
AUTO_INCREMENTattribute andSQL SECURITY DEFINERcarry no=, so both survive. DroppingSQL SECURITY, whichmysqlpump --skip-definerdoes, would turn a view declaredSQL SECURITY INVOKERinto a definer-rights one.SQLExportOptionsalso gains a tolerantinit(from:). Probed: a synthesizedDecodablethrowskeyNotFoundfor a key the saved payload predates and never falls back to the property's default, soPluginSettingsStorage.loadwould have answered nil and reset every existing user's gzip and batch-size choice the moment a field was added.What excluding the definer means
The account running the import becomes the view's definer, and
SQL SECURITYis left as the server reported it, so a definer-rights view then runs with the importing account's privileges rather than the original account's. Restoring as an administrator over a schema whose existing grants were not revoked therefore changes the runtime principal for those grantees. The alternative is a dump that cannot be imported at all, which is what the issue reports; the consequence is stated in the checkbox help and on the docs page rather than left silent.Excluding the counter is lossless only up to the rows in the dump: restoring them sets the counter one past the highest key present, so a source counter that had run ahead of its rows, after deletes or a reset, does not carry over.
Verified
verify.sh build, PASS.verify.sh test SQLExportDDLRewriterTests SQLExportOptionsDecodingTests StringCatalogIntegrityTests, PASS, 29 cases.verify.sh lintoverPlugins/SQLExportPluginand both new suites, 0 violations. (The step reports FAIL for a pre-existingAXCellreference inCLAUDE.md, present at the merge base and untouched here.)docs/scripts/check-writing-style.shanddocs/scripts/check-docs-against-source.py, both clean.ERROR 1227for a non-privileged account; the rewritten dump restores clean, both views query, and the table's counter self-heals toMAX(id)+1on the first insert.pluginsaggregate run;SQLExportis bundled and was built by the app build.Every fixture in
SQLExportDDLRewriterTestsis literal server output:SHOW CREATE TABLE/SHOW CREATE VIEWon MariaDB 12.3, including anANSI_QUOTESidentifier, andSELECT sql FROM sqlite_masteron SQLite.Not covered by UI automation
The export dialog needs a live connection to reach, which the
TableProUITestssandbox cannot provide deterministically, and there is no existing export UI suite. The options panel is therefore covered by unit tests and by the end-to-end restore above, not by XCUITest. No before/after screenshot of the panel either: the change adds two checkboxes to the SQL options list, and the docs page'sexport-dialog.pngis still a placeholder rather than a real capture, so nothing existing goes stale.Review
Reviewed by Codex (
reviewandadversarial-review). Its first pass caught a real defect: the rewrite originally ran for every dialect and pattern, and a column namedauto_incrementordefinerin aCHECKconstraint was stripped out of its own expression. Reproduced on SQLite, whosefetchTableDDLreturns the catalog's own text:CHECK (auto_increment = 4)came back asCHECK (). That is what the dialect gate and the two position gates fix, with regression tests for both. Its changelog finding was applied too.Of the adversarial pass's three findings, one was measured and did not reproduce: under
ANSI_QUOTESMariaDB writes a literal backslash doubled and an embedded quote doubled, so the scanner reads that identifier correctly. It is now a regression test. The other two are the definer and counter consequences above, both stated in the help text and the docs.https://claude.ai/code/session_01J6xU4Zx4DRJ5JaxMP437uT