fix(datagrid): render query results before loading table metadata (#1574)#1582
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 60179ab190
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| metadata: inlineMeta, | ||
| hasSchema: false, |
There was a problem hiding this comment.
When needsMetadataFetch is true but the driver does not populate resultColumnMeta (currently every plugin except MySQL), this passes metadata: nil and hasSchema: false into applyPhase1Result. That method immediately calls applyDefaultSortIfPending; with the “Default row sort: Primary key” setting it resolves no PKs, marks didEvaluateDefaultSort = true, and later launchPhase2Work only applies the fetched PK metadata without re-running default-sort resolution. On first loading a table through PostgreSQL/SQLite/etc., the table therefore stays unsorted by primary key, whereas before this commit the schema was awaited and the PKs were available for phase 1.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9225af0374
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| parent.mutateActiveTableRows(for: tabId) { rows in | ||
| rows.updateDisplayMetadata( | ||
| columnDefaults: parsed.columnDefaults, | ||
| columnForeignKeys: parsed.columnForeignKeys, | ||
| columnNullable: parsed.columnNullable | ||
| ) |
There was a problem hiding this comment.
When the schema fetch finishes after the user has switched the active result set (for example to a pinned result set), this mutates the tab's currently active TableRows, not necessarily the result set created by the query; switchActiveResultSet swaps the session rows on selection. That lets defaults, foreign keys, and nullability from the newly executed table be written onto a different result set, producing incorrect edit metadata until the rows are reloaded. Carry the originating result-set id through phase 2 or skip the update if the active result set changed.
Useful? React with 👍 / 👎.
…ccess on makeColumnMeta
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e2d4ae316a
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| public let executionTime: TimeInterval | ||
| public let isTruncated: Bool | ||
| public let statusMessage: String? | ||
| public let columnMeta: [PluginColumnInfo]? |
There was a problem hiding this comment.
For user-installed drivers built against PluginKit 18 before this change, the version gate still accepts them (currentPluginKitVersion and minimumCompatiblePluginKitVersion remain 18), but their binary was compiled against the old PluginQueryResult layout/initializer. Adding this stored property changes the Swift ABI, so those plugins can fail when returning query results instead of being rejected with an update message; please bump the PluginKit version or add a compatibility wrapper with this ABI change.
Useful? React with 👍 / 👎.
1 participant
Closes #1574.
Problem
Against a remote MySQL (Alibaba Cloud RDS, ping ~50ms), a small query (~500 rows) shows a query "execution time" of ~500ms, but the grid takes 3-5 seconds to appear with a spinner. Local databases are fast, and DataGrip/Navicat against the same remote DB respond in ~1s. Disabling TLS does not help.
Root cause
The displayed execution time measured only the SELECT on the primary connection. The grid did not render until a separate, sequential metadata fetch finished:
QueryExecutor.executeQuerybefore the rows were applied.information_schema(foreign keys, row count), which is slow on Alibaba RDS.On localhost the round-trip cost is ~0ms, so the work was invisible. On a 50ms remote link with slow system tables it is the 3-5s gap. libmariadb already sets
TCP_NODELAY, so Nagle was not the cause.Fix
Render rows before metadata (all drivers).
executeQuerynow returns only the rows. The coordinator starts the schema fetch alongside the SELECT, renders the grid as soon as rows arrive, then applies the editing metadata in the background: column defaults, foreign keys, nullability, primary keys, enum values, and row count, bumpingmetadataVersionso editability lights up a moment later. Covers the editor-query and parameterized paths.MySQL reads primary key and nullability from the result set.
MariaDBPluginConnectionnow readsPRI_KEY_FLAG,NOT_NULL_FLAG, andAUTO_INCREMENT_FLAGfromMYSQL_FIELDand carries them throughPluginQueryResult.columnMeta. A simpleSELECT * FROM tableis fully editable at first paint with no extra round-trip.Safety
DataChangeManager.setPrimaryKeyColumns, so it cannot wipe edits made in the brief window before metadata lands.SQLStatementGeneratoralready falls back to a full-row WHERE clause when no primary key is known, so that window stays correct for other drivers.queryGeneration, and the schema task is cancelled on query cancel or error.Notes
PluginQueryResultgains an additivecolumnMetafield (defaults tonil). This is ABI-safe per the PluginKit rules, so nocurrentPluginKitVersionbump. The PluginKit ABI gate will report an additive diff; that is expected. The bundled MySQL plugin and the app rebuild together.Tests
QueryExecutor.inlineMetadata(primary keys, nullability, composite keys, absent/empty, count mismatch).makeColumnMetamapping.swiftlint --strictclean on all changed files.