Skip to content

fix(mysql): report generated columns so SQL export excludes them - #2023

Merged
datlechin merged 2 commits into
TableProApp:mainfrom
MDeev:fix/mysql-generated-columns-export
Aug 5, 2026
Merged

fix(mysql): report generated columns so SQL export excludes them#2023
datlechin merged 2 commits into
TableProApp:mainfrom
MDeev:fix/mysql-generated-columns-export

Conversation

@MDeev

@MDeev MDeev commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

MySQL/MariaDB SQL export wrote STORED/VIRTUAL generated columns into INSERT statements, so re-importing a dump failed with error 3105.

SQLExportPlugin already excludes columns flagged isGenerated, but the MySQL driver never set that flag despite reading Extra (which carries STORED GENERATED / VIRTUAL GENERATED). This maps it in both column-fetch paths — DEFAULT_GENERATED (MySQL 8 expression defaults) intentionally doesn't match, since those columns are insertable.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 708c8ac96a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

internal func mysqlColumnIsGenerated(extra: String?) -> Bool {
guard let extra else { return false }
let upper = extra.uppercased()
return upper.contains("STORED GENERATED") || upper.contains("VIRTUAL GENERATED")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Add MariaDB Extra values to generated-column check

When exporting from MariaDB tables with generated columns, DESCRIBE/SHOW FULL COLUMNS reports Extra as VIRTUAL or PERSISTENT rather than ... GENERATED (MariaDB docs: https://mariadb.com/docs/server/reference/sql-statements/data-definition/create/generated-columns/). Because this predicate still returns false for those values, SQL export will keep writing MariaDB generated columns into INSERT statements and strict re-imports still fail with the generated-column value error this change is meant to avoid.

Useful? React with 👍 / 👎.

@datlechin
datlechin force-pushed the fix/mysql-generated-columns-export branch from 708c8ac to 4ac7786 Compare August 5, 2026 12:53
@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

Copy link
Copy Markdown
Member

Thanks for this. The diagnosis is right and both column-fetch paths were the correct places to set the flag. I verified extra is sourced correctly in each (SHOW FULL COLUMNS index 6, the INFORMATION_SCHEMA projection index 7), that SQLExportPlugin drops the column from the INSERT column list and the values, and that the CREATE TABLE half comes from SHOW CREATE TABLE so the DDL keeps the generated column. Test wiring is right too: the membershipExceptions entry adds the file to TableProTests, so the suite compiles against the same source the plugin ships and runs in CI.

I rebased onto main (the [Unreleased] section had gained a ### Fixed heading, so the PR no longer merged) and pushed two changes on top. Your commit is untouched apart from the rebase.

1. MariaDB was claimed but not covered.

The PR title, the changelog line, and the plugin's Info.plist all cover MariaDB, but the two-word markers are a MySQL vocabulary. I checked this against MariaDB's own recorded test output rather than the docs, because the docs disagree with each other:

MariaDB Extra for VIRTUAL Extra for PERSISTENT/STORED
5.5 – 10.1 VIRTUAL PERSISTENT
10.2+ VIRTUAL GENERATED STORED GENERATED

So 10.2 onward matches MySQL and your predicate already handles it; 10.1 and older report a bare marker and were missed. The KB page showing bare VIRTUAL/PERSISTENT is accurate for pre-10.2 servers and was never updated.

Two more things that turned up and are now covered by tests: MariaDB joins combined attributes with a comma (STORED GENERATED, INVISIBLE) where MySQL uses a space, which is exactly why contains on the two-word phrase is the right operator; and MariaDB's system-versioning ROW START/ROW END columns report STORED GENERATED, INVISIBLE, so they are correctly excluded too.

The predicate is now the phrase match plus an exact-match fallback for the two legacy bare markers. I deliberately did not widen it to bare VIRTUAL/STORED/PERSISTENT substrings; that would pass the same tests but loosen three common words for no extra coverage.

2. The flag stopped at the driver, so the same bug was still live outside export.

isGenerated had exactly one consumer, SQLExportPlugin. PluginDriverAdapter.mapPluginColumns dropped it when converting PluginColumnInfo to the app's ColumnInfo, so nothing past the driver ever knew a column was generated. That left three paths broken on any table with a generated column, independent of export:

  • Add Row put an explicit NULL in the generated column (it has no literal default, so it missed the __DEFAULT__ sentinel that would have omitted it) and the INSERT was rejected every time.
  • Duplicate Row copied the computed value into the new row's INSERT.
  • Cell editing let you type into a generated column; the resulting UPDATE was rejected.

So the flag is now threaded through ColumnInfo to DataChangeManager.generatedColumns, and Add Row, Duplicate Row, cell editability, and SQLStatementGenerator all respect it. That last one is the belt-and-braces layer: a generated column is dropped from the SET clause and from both INSERT paths regardless of how the change was recorded, so paste and multi-row edits are covered too.

I also set the flag for ClickHouse, where system.columns.default_kind was already being selected and just went unused, so MATERIALIZED and ALIAS columns were being written into INSERTs the same way. PostgreSQL already set it.

Still unset for MSSQL (sys.columns.is_computed), Oracle (ALL_TAB_COLUMNS.VIRTUAL_COLUMN), CockroachDB, SQLite and LibSQL (need pragma_table_xinfo), and DuckDB. Those are registry plugins or need a real query change, so I left them for a separate PR rather than growing this one further.

One caveat worth stating plainly: I have not compiled this. Please build before merging and tell me about anything that does not.

@datlechin
datlechin force-pushed the fix/mysql-generated-columns-export branch from 4ac7786 to 1efd543 Compare August 5, 2026 13:03
@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 merged commit c48e390 into TableProApp:main Aug 5, 2026
@MDeev
MDeev deleted the fix/mysql-generated-columns-export branch August 5, 2026 14:10
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.

2 participants