-
-
Notifications
You must be signed in to change notification settings - Fork 291
fix(datagrid): cancel in-progress query before Cmd+R refresh#1650
fix(datagrid): cancel in-progress query before Cmd+R refresh #1650J2TeamNNL wants to merge 1 commit into
Conversation
...s runs currentQueryTask?.cancel() is async; runQuery() bails on isExecuting == true. Fix: nil the task and reset isExecuting synchronously before rebuildTableQuery + runQuery.
💡 Codex Review
P2 Badge Call driver cancellation before restarting refresh
When Cmd+R is pressed while a table query is still running in the database driver, this clears currentQueryTask before runQuery() reaches executeQueryInternal. That method only calls driver.cancelQuery() inside if currentQueryTask != nil (TablePro/Views/Main/MainContentCoordinator.swift:1036-1043), so the backend query is never cancelled; it can keep occupying the connection and later unwind the cancelled task while the fresh query is already running. Keep the task reference long enough to invoke the existing cancellation path, or explicitly call cancelQuery() here before setting it to nil.
i️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Root cause
currentQueryTask?.cancel()initiates cooperative cancellation but does not synchronously resettab.execution.isExecuting. The subsequentrunQuery()call hitsguard !tab.execution.isExecuting else { return }and silently bails, so the refresh never executes when a query is running.Fix
After
currentQueryTask?.cancel(), immediately setcurrentQueryTask = niland resettab.execution.isExecuting = falsebefore callingrebuildTableQuery+runQuery. Applied to both the discard-confirmation branch and the no-pending-changes branch ofhandleRefresh.File:
TablePro/Views/Main/Extensions/MainContentCoordinator+Refresh.swiftTest plan