Skip to content

feat(plugins): exclude the AUTO_INCREMENT counter and DEFINER clauses from an SQL export - #2608

Merged
datlechin merged 1 commit into
mainfrom
feat/sql-export-portable-ddl
Sep 2, 2026
Merged

feat(plugins): exclude the AUTO_INCREMENT counter and DEFINER clauses from an SQL export#2608
datlechin merged 1 commit into
mainfrom
feat/sql-export-portable-ddl

Conversation

@datlechin

Copy link
Copy Markdown
Member

Fixes #2516

Root cause

SQLExportPlugin.writeCreatePhase wrote whatever dataSource.fetchTableDDL returned, 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:

  • the table option AUTO_INCREMENT=<n>, which is the source server's next key value;
  • a view's 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 …`
restoring that dump as a non-privileged account ERROR 1227 (42000): Access denied; you need (at least one of) the SET USER privilege(s) for this operation
restoring it as root, then querying the view ERROR 1446 (HY000): The user specified as a definer ('ghost'@'nowhere') does not exist
the same dump with the clause dropped creates and queries fine, owned by the importing account

Two 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 TABLE reports AUTO_INCREMENT=5 inside a column COMMENT and inside a quoted column name.

The fix

SQLExportDDLRewriter is 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 counter and Exclude 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:

  • the counter is a table option, so it is taken at parenthesis depth zero alone;
  • the account belongs to the CREATE header, so it is taken between CREATE and 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_INCREMENT attribute and SQL SECURITY DEFINER carry no =, so both survive. Dropping SQL SECURITY, which mysqlpump --skip-definer does, would turn a view declared SQL SECURITY INVOKER into a definer-rights one.

SQLExportOptions also gains a tolerant init(from:). Probed: a synthesized Decodable throws keyNotFound for a key the saved payload predates and never falls back to the property's default, so PluginSettingsStorage.load would 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 SECURITY is 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 lint over Plugins/SQLExportPlugin and both new suites, 0 violations. (The step reports FAIL for a pre-existing AXCell reference in CLAUDE.md, present at the merge base and untouched here.)
  • docs/scripts/check-writing-style.sh and docs/scripts/check-docs-against-source.py, both clean.
  • End to end against MariaDB 12.3: the unmodified dump fails at the view with ERROR 1227 for a non-privileged account; the rewritten dump restores clean, both views query, and the table's counter self-heals to MAX(id)+1 on the first insert.
  • No PluginKit change, so no ABI step. No registry-only plugin touched, so no plugins aggregate run; SQLExport is bundled and was built by the app build.

Every fixture in SQLExportDDLRewriterTests is literal server output: SHOW CREATE TABLE / SHOW CREATE VIEW on MariaDB 12.3, including an ANSI_QUOTES identifier, and SELECT sql FROM sqlite_master on SQLite.

Not covered by UI automation

The export dialog needs a live connection to reach, which the TableProUITests sandbox 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's export-dialog.png is still a placeholder rather than a real capture, so nothing existing goes stale.

Review

Reviewed by Codex (review and adversarial-review). Its first pass caught a real defect: the rewrite originally ran for every dialect and pattern, and a column named auto_increment or definer in a CHECK constraint was stripped out of its own expression. Reproduced on SQLite, whose fetchTableDDL returns the catalog's own text: CHECK (auto_increment = 4) came back as CHECK (). 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_QUOTES MariaDB 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

@mintlify

mintlify Bot commented Sep 2, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
TablePro 🟢 Ready View Preview Sep 2, 2026, 10:51 AM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@datlechin
datlechin force-pushed the feat/sql-export-portable-ddl branch from 6e4bed1 to e38c728 Compare September 2, 2026 11:52
@datlechin
datlechin merged commit 6ad4a60 into main Sep 2, 2026
9 checks passed
@datlechin
datlechin deleted the feat/sql-export-portable-ddl branch September 2, 2026 11:58
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.

Export options for auto_increment and DEFINER

1 participant