feat(datagrid): offer the engine's own default values on the Structure tab's Default cell - #2690
Merged
Conversation
…e tab's Default cell Claude-Session: https://claude.ai/code/session_01GxbNXNA2bvpPRsWnDRTjQQ
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
Member
Author
|
The Columns tab after the change. Every Default cell carries the chevron that opens the engine's list, and the cell still takes typed SQL. The shot is committed in the branch at |
Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
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
The Structure tab's Default cell now carries the engine's own list of default values, and still takes typed SQL. Clicking the chevron opens No default, NULL, Empty string, the engine's expressions, and Custom….
Fixes #2688.
Root cause
Two layers, and the second is why this could not be only a menu.
Surface: the Default cell was plain free text. No vocabulary, so you had to know that "now" is
CURRENT_TIMESTAMPhere,now()there andGETDATE()somewhere else, and there was no way to say "empty string" as distinct from "no default".Underneath:
defaultValuewas an untypedString?with no agreed contract. Nobody said whether it held a literal or an expression, so each of the 12 DDL writers guessed with its own hand-copied allowlist and quoted whatever it did not recognise into a string literal. Measured against live servers during the investigation:gen_random_uuid()andCURRENT_DATEbecameDEFAULT 'gen_random_uuid()'. Duplicating aserialcolumn emittedDEFAULT 'nextval(''users_id_seq''::regclass)', because the sequence guard tested::REGCLASSas a suffix, a shape PostgreSQL never emitsb DATE DEFAULT (CURRENT_DATE + INTERVAL 1 YEAR)failed with 1067. TheDEFAULT (UUID())variant succeeded and permanently stored the string'uuid()'host String MATERIALIZED domain(url)converted the column toDEFAULT 'domain(url)', and every later row stored the literal textPRAGMA table_inforeports an expression without its parentheses and a bare word without its quotes, and those are not the same thing.(datetime('now'))reads back asdatetime('now')and will not parse bare;DEFAULT abcis a text literal and fails as(abc)withdefault value of column [a] is not constantpendingbecame an invalid identifierSo any menu built on those writers would ship broken DDL. Fixing the contract is the feature.
The fix
defaultValueis now the exact SQL that followsDEFAULT, with nil meaning no clause at all. A literal carries its own quotes, an expression is written the way the engine spells it, andNULLmeansDEFAULT NULLrather than the absence of a default. Every driver emits it verbatim and stops guessing; each read path normalises what its catalog gives it into that form.The three states the issue asks for are already representable once the contract is fixed:
nil,"NULL","''". No model change, noPluginColumnDefinitionfield, no PluginKit ABI bump.Adding syntax the engine's own grammar demands is still the driver's job. MySQL takes a
TEXT,BLOB,JSONorGEOMETRYdefault only in parentheses whatever the value is, somysqlDefaultValueLiteraladds them there and nowhere else.The control
Built from the grid's own parts, which is what the Type column already is. No
NSComboBoxis mounted: the grid draws its cells with CoreText and holds 26 views rather than 12,500, and a per-cell combo box would give that back.NSMenuwithNSMenuItem.sectionHeader(title:)groups (macOS 14, our floor) and a checkmark on the item whose SQL equals the stored value.Text | SQL expressionpicker.Textescapes through the connected driver's ownescapeStringLiteral, which is stricter than the shared helper on the engines that need it.FieldEditorKind.valuePicker.enumPickercannot serve: its list is the whole set of legal values, and a column default is open-ended.The vocabulary
Curated per database type id, the shape
PluginMetadataRegistry.fallbackCategoryalready uses. It cannot be keyed by plugin: MySQL and MariaDB share one and disagree about parentheses, and PostgreSQL, Redshift, CockroachDB and PGlite share one where only two havegen_random_uuid().Two things deliberately absent:
An engine the app has never heard of gets the shared literals and no expression section, rather than another engine's spelling.
Collateral fixed here
These are not extras. The menu is wrong without them, so they are scope.
nextval(...)andgen_random_uuid()into string literals.DEFAULTkind now populatesdefaultValue, andMODIFY COLUMNleaves the other kinds alone, which the plugin already relied on for its reorder path. Clearing an ordinary default now emitsMODIFY COLUMN … REMOVE DEFAULT; omitting the clause left the old default on the server while the save reported a removal.DriverPluginfallback. ScyllaDB is registered as its own type id with its own curated snapshot, so both needed fixing.TrinoColumnSpecmaps name, type, nullability and comment only, so a default typed there reached no statement and the save reported success. The field is gone rather than accepting a change that is discarded.SYSDATEandUSERand SQLite'sX'0102'as quoted strings that run and store something else.COLUMN_DEFAULT. Catalog normalisation is version-gated rather than keyed on the server being MariaDB at all.DEFAULT CURRENT_TIMESTAMPon a non-temporal column. MySQL reports aVARCHARliteral of that text the same way and with noDEFAULT_GENERATEDmarker, so reading it as the expression turned a stored string into a clock reading on the next edit.Visible change
MySQL and SQLite string defaults now display as
'abc'rather thanabc. That is what the DDL says, and what PostgreSQL users already saw. Typing bare text into the cell now means a SQL expression, so a string literal comes from Custom… > Text or from typing the quotes; the SQL preview sheet shows the statement before it runs.Before / After
There is no meaningful "before" shot: the Default cell rendered as plain text with no affordance at all. After, every Default cell carries the chevron:
The menu itself could not be captured. The data grid draws its cells rather than mounting views, so it takes no synthetic click, and a screenshot of an open
NSMenuover it cannot be scripted.Verified
buildtest(13 suites, 172 cases)test(DamengDriverTestsscheme, which CI runs separately)plugins(AllPlugins)oracle-nio@TaskLocalmacro failure in the vendored SPM dependency, which breaks this aggregate locally on every branch. No error names any file in this changedocslintlegacy_swiftui_aspect_ratiofindings, both in files this branch never touches (SupportView.swift,ImportFromAppSourcePicker.swift), both present onmainuitest ColumnDefaultPickerUITestsSQLiteDefaultValueTestsand the MySQL catalog cases pin behaviour measured against real servers rather than assumed.ColumnDefaultRoundTripTestsis the drift guard: every value the menu can choose is run through the writer that would emit it and has to come back byte-identical, so the vocabulary and the writers cannot disagree silently.Reviewed
Both Codex passes ran against this diff. The defect review found seven issues and the adversarial pass returned No-ship on the first draft; everything above the "Known and not fixed here" section below is what those two passes turned up and what is now fixed, including the SQLite bare-word literal, the MySQL
CURRENT_TIMESTAMPmisreading, the MariaDB version floor, ScyllaDB, Trino, the ER exporter, and the ClickHouse removal path. Two more were producers of the contract I had missed:RowImportSheet's Default field, now labelled and documented as SQL, andDamengDriverTests, whose assertion pinned the old quoting and is updated.Known and not fixed here
validateBundleVersionsrejects a plugin above the app's version, never below. Publishing the plugins with the app release is what closes it.SQLTypeFamily.mysql, soneedsTranslationis false and MariaDB'suuid()reaches MySQL's writer unchanged, where MySQL 8 needs(uuid()). Before this change the same value was quoted into'uuid()'and stored as text silently; it now fails loudly instead, which is better but still wrong. Fixing it means giving the translator the target engine rather than its family, and validating target DDL before the Replace plan's destructive cleanup. That is Copy To's own subsystem.PluginColumnDefinitionhas no kind field to refuse on.Not in this PR
DEFAULTclause anywhere, so an edit there is still read, displayed and dropped. Reported, not fixed.Registry plugins need publishing
The driver fixes reach bundled plugins with the next app release. Snowflake, Teradata, Oracle, Dameng, DuckDB, libSQL and Cloudflare D1 are registry-only, so their fixes are inert until those plugins are published.
https://claude.ai/code/session_01GxbNXNA2bvpPRsWnDRTjQQ