-
-
Notifications
You must be signed in to change notification settings - Fork 285
fix(datagrid): render query results before loading table metadata (#1574) #1582
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,17 +19,6 @@ extension QueryExecutionCoordinator { | |
| QueryExecutor.parseSchemaMetadata(schema) | ||
| } | ||
|
|
||
| func awaitSchemaResult( | ||
| parallelTask: Task<SchemaResult, Error>?, | ||
| tableName: String | ||
| ) async -> SchemaResult? { | ||
| await QueryExecutor.awaitSchemaResult( | ||
| connectionId: parent.connectionId, | ||
| parallelTask: parallelTask, | ||
| tableName: tableName | ||
| ) | ||
| } | ||
|
|
||
| func isMetadataCached(tabId: UUID, tableName: String) -> Bool { | ||
| guard let idx = parent.tabManager.tabs.firstIndex(where: { $0.id == tabId }) else { | ||
| return false | ||
|
|
@@ -301,32 +290,33 @@ extension QueryExecutionCoordinator { | |
| tabId: UUID, | ||
| capturedGeneration: Int, | ||
| connectionType: DatabaseType, | ||
| schemaResult: SchemaResult? | ||
| schemaTask: Task<SchemaResult, Error>? | ||
| ) { | ||
| resolveRowCount( | ||
| tableName: tableName, | ||
| tabId: tabId, | ||
| capturedGeneration: capturedGeneration, | ||
| connectionType: connectionType | ||
| ) | ||
|
|
||
| let isNonSQL = PluginManager.shared.editorLanguage(for: connectionType) != .sql | ||
| guard !isNonSQL else { return } | ||
| Task(priority: .utility) { [weak self, parent] in | ||
| guard let self else { return } | ||
| guard !parent.isTearingDown else { return } | ||
|
|
||
| let columnInfo: [ColumnInfo] | ||
| if let schema = schemaResult { | ||
| columnInfo = schema.columnInfo | ||
| } else { | ||
| columnInfo = (try? await DatabaseManager.shared.withMetadataDriver(connectionId: parent.connectionId) { driver in | ||
| try await driver.fetchColumns(table: tableName) | ||
| }) ?? [] | ||
| let schema = try? await schemaTask?.value | ||
|
|
||
| await MainActor.run { [weak self] in | ||
| guard let self else { return } | ||
| guard capturedGeneration == parent.queryGeneration else { return } | ||
| if let schema { | ||
| applyPhase2Metadata(parsed: QueryExecutor.parseSchemaMetadata(schema), tabId: tabId) | ||
| } | ||
| resolveRowCount( | ||
| tableName: tableName, | ||
| tabId: tabId, | ||
| capturedGeneration: capturedGeneration, | ||
| connectionType: connectionType | ||
| ) | ||
| } | ||
|
|
||
| guard !isNonSQL, let schema else { return } | ||
|
|
||
| let columnEnumValues = await parent.fetchEnumValues( | ||
| columnInfo: columnInfo, | ||
| columnInfo: schema.columnInfo, | ||
| tableName: tableName, | ||
| connectionType: connectionType | ||
| ) | ||
|
|
@@ -361,6 +351,40 @@ extension QueryExecutionCoordinator { | |
| } | ||
| } | ||
|
|
||
| private func applyPhase2Metadata(parsed: ParsedSchemaMetadata, tabId: UUID) { | ||
| guard parent.tabManager.tabs.contains(where: { $0.id == tabId }) else { return } | ||
|
|
||
| parent.mutateActiveTableRows(for: tabId) { rows in | ||
| rows.updateDisplayMetadata( | ||
| columnDefaults: parsed.columnDefaults, | ||
| columnForeignKeys: parsed.columnForeignKeys, | ||
| columnNullable: parsed.columnNullable | ||
| ) | ||
|
Comment on lines
+357
to
+362
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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 Useful? React with 👍 / 👎. |
||
| } | ||
|
|
||
| parent.tabManager.mutate(tabId: tabId) { tab in | ||
| if !parsed.primaryKeyColumns.isEmpty { | ||
| tab.tableContext.primaryKeyColumns = parsed.primaryKeyColumns | ||
| } | ||
| if let approxCount = parsed.approximateRowCount, approxCount > 0, | ||
| !tab.filterState.hasAppliedFilters { | ||
| tab.pagination.totalRowCount = approxCount | ||
| tab.pagination.isApproximateRowCount = true | ||
| } | ||
| tab.metadataVersion += 1 | ||
| } | ||
|
|
||
| if parent.tabManager.selectedTabId == tabId, !parsed.primaryKeyColumns.isEmpty { | ||
| parent.changeManager.setPrimaryKeyColumns(parsed.primaryKeyColumns) | ||
| } | ||
|
|
||
| if let activeIdx = parent.tabManager.selectedTabIndex, | ||
| activeIdx < parent.tabManager.tabs.count, | ||
| parent.tabManager.tabs[activeIdx].id == tabId { | ||
| parent.dataTabDelegate?.tableViewCoordinator?.refreshForeignKeyColumns() | ||
| } | ||
| } | ||
|
|
||
| func launchPhase2Count( | ||
| tableName: String, | ||
| tabId: UUID, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For user-installed drivers built against PluginKit 18 before this change, the version gate still accepts them (
currentPluginKitVersionandminimumCompatiblePluginKitVersionremain 18), but their binary was compiled against the oldPluginQueryResultlayout/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 👍 / 👎.