fix(plugins): stop the column-default contract mangling literals, expressions and constraints - #2693
Merged
Merged
Conversation
…ressions and constraints
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.
What this is
Follow-up to #2690, which redefined
defaultValueas the exact SQL that followsDEFAULT. Seven defects that change left behind, four of them regressions #2690 introduced. All were found by a multi-agent audit of the merged contract and each was verified against a live server or a compiled probe before being fixed.The regressions #2690 introduced
MySQL 8 reads every
DEFAULT CURRENT_TIMESTAMPback as(CURRENT_TIMESTAMP). MySQL 8.0.13+ marks a plainCURRENT_TIMESTAMPdefaultDEFAULT_GENERATEDinEXTRA, exactly like a real expression, so the temporal branch was skipped and the value was parenthesised. Measured on 8.4.11. The temporal question is now answered before the marker is consulted.MySQL numeric,
BITand binary defaults were quoted. The catalog reports a string default stripped of its quotes, but a number, aBITdefault (b'1') and a binary default (0x61) come back as the literals they already are. Quoting them all was wrong three ways, measured on 8.4.11:qty INT DEFAULT 0DEFAULT '0'0, so Compare & Sync reports the same difference foreverflag BIT(1) DEFAULT b'1'DEFAULT 'b''1'''b VARBINARY(8) DEFAULT 0x61DEFAULT '0x61'0x61to the four characters0x61(HEX()goes from61to30783631)mysqlCatalogReportsLiteralAsSQLnow answers this by column type.A bare keyword default crossed engines unquoted.
CrossEngineDefaultValuekept any bare word on the documented premise that "every target's DDL writer quotes an unrecognised word", which was true before #2690 and false after. Measured: PostgreSQL 17.11 reportssession_user,current_role,current_schemaanduseras bare words, and Copy To emittedDEFAULT SESSION_USERinto MySQL 8.4, which fails 1064/1054. TheCREATE TABLEfails, so that table's rows never arrive. The arm is deleted; such a default now falls to the existing.drop(reason:)and is reported as a conversion note.The enum cell's default badge stopped appearing on MySQL.
EnumMenuPickermatches the column default against the enum's own unquoted tokens, and the default now arrives as'active'. The value crosses out of SQL at that boundary now.The other three
An expression default reached MySQL without the parentheses it requires. MariaDB writes
uuid()bare and MySQL 8 needs(uuid()).mysqlDefaultValueLiteralnow parenthesises every expression for MySQL and leaves MariaDB alone, which fixes it in one place for Copy To, the Structure tab's Custom… > SQL expression mode and SQL import. A value is a literal by shape (quoted string, number, bit or hex literal,NULL), not by an allowlist.SQL Server dropped a column's
DEFAULTconstraint and never re-added it. The drop fired ondefaultChanged || needsTypeChange; the re-add only ondefaultChanged. So changing only a column's type or nullability left it with no default, and every laterINSERTthat omitted it failed with error 515. Pre-existing, verified with a probe over the real generator. The re-add now mirrors the drop.A ClickHouse MATERIALIZED, EPHEMERAL or ALIAS column could still be converted. #2690 stopped the silent conversion; deliberately picking a default for such a column still restated it as
DEFAULT. The kind has nowhere to ride on a column definition andgenerateModifyColumnSQLis synchronous, so the driver now answers from what it read out ofsystem.columnsitself and refuses.Verified
buildtest(11 suites, 201 cases)test(DamengDriverTestsscheme)plugins(AllPlugins)oracle-nio@TaskLocalmacro failure in the vendored dependency, which breaks this aggregate locally on every branch. No error names a file in this changedocslintlegacy_swiftui_aspect_ratiofindings are in files this branch never touches and are present onmainThe MySQL argument tables now pin the measured catalog shapes (
0,b'1',0x61,enum(...), andCURRENT_TIMESTAMPwith and without theDEFAULT_GENERATEDmarker), and the drift guard from #2690 is restated as the invariant it was always meant to check: the writer adds the syntax an engine's grammar demands and never re-quotes a value.Known and not fixed here
SQLTypeFamily.mysql, soneedsTranslationis false and the translator returns the snapshot untouched. The MariaDB-to-MySQL case is closed by the driver-side parenthesising above, but Redshift lackinggen_random_uuid()and CockroachDB'sunique_rowid()are not:CrossEngineDefaultValueis keyed on the family, not the engine, so it cannot tell them apart. Fixing that means giving it the targetDatabaseTypeand adding a defaults-only pass to the same-family branch, without flippingtranslated, which gates sequence copying and the value coercer.supportsTransactionalDDLis false on MySQL and MariaDB, so a failing create loses the dropped tables with no rollback and the result view does not say so. Pre-existing and independent of this change.TableProPluginKitVersion23. Publishing Snowflake, Teradata, Oracle, Dameng, DuckDB, libSQL and Cloudflare D1 with the app release is what closes it.