fix(datagrid): apply default row sort on the first table load (#1603)#1604
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: 89b79066c6
ℹ️ 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".
| filterCoordinator.rebuildTableQuery(at: index) | ||
| } | ||
|
|
||
| runQuery() |
There was a problem hiding this comment.
Because loadSchemaColumns awaits metadata before this call, the selected tab can change (or the preview tab can be reused for another table while preserving the same tab id) before the resolver resumes. This unconditional runQuery() executes whatever tab is currently selected rather than the tabId being resolved, and in the reused-tab case can apply the stale resolver to the new table; the previous post-load path guarded selectedTabId == tabId before re-querying. Please re-check that the target tab is still selected and still represents the same table/schema before rebuilding/executing.
Useful? React with 👍 / 👎.
…s and cancel stale resolves
Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f48086df21
ℹ️ 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".
| defaultSortResolveTask?.cancel() | ||
| if shouldResolveDefaultSort(for: tab) { | ||
| let tabId = tab.id | ||
| tabManager.mutate(at: index) { $0.execution.didEvaluateDefaultSort = true } |
There was a problem hiding this comment.
If the user triggers another table execution/refresh while loadSchemaColumns is still in flight, the next executeTableTabQueryDirectly() cancels defaultSortResolveTask at line 858, but this flag has already been set, so shouldResolveDefaultSort returns false and the tab runs its original unsorted query; because the flag remains true, the default sort is never retried for that table load. Consider setting this only after the resolver finishes or clearing it when the resolve task is canceled.
Useful? React with 👍 / 👎.
1 participant
Fixes #1603.
Problem
Setting Default row sort to Primary key stopped sorting PostgreSQL tables (regression in v0.49.0). Even when it did work, the rows loaded unsorted first and then re-sorted, instead of arriving sorted like TablePlus.
Root cause
v0.49.0 (#1582) split table loading into two phases: render rows first, fetch metadata second. For PostgreSQL the primary key only arrives in phase 2, but the default-sort step ran in phase 1 with no primary key, produced an empty sort, and permanently set the "already evaluated" flag, so the real primary key from phase 2 was never used. MySQL and MariaDB escaped this because their wire protocol carries primary-key flags inline in every result, so phase 1 already knew the key.
Fix
Resolve the default sort before the first table query, from the table's schema, so a single query comes back already sorted. The
Nonedefault keeps the fast unsorted render from #1582 untouched; only tables opened with a Primary key or First column default sort wait for a quick primary-key lookup, which is what produces the sorted-from-the-start result.MainContentCoordinator+DefaultSort.swift: gate + resolver that loads schema columns/keys, sets the sort state, rebuilds the query, then runs one sorted query.executeTableTabQueryDirectly()detours through the resolver when a non-nonedefault sort is pending.rebuildTableQueryfalls back to cached schema columns when no rows are loaded yet, soORDER BYresolves before the first fetch.Reuses the existing
schemaColumnsCache/loadSchemaColumnsthat the column-scoping path already relies on, so this is not a new fetch pattern.Tests
DefaultSortInitialQueryTests.swiftcovers the schema-columnORDER BYfallback (the exact thing that was missing for PostgreSQL) and the gate logic.swiftlint lint --strictpasses clean.A UI test isn't included because the flow needs a live PostgreSQL connection, which is not deterministic in CI.