fix(coordinator): keep a SQL Server table on the schema it was listed under - #2006
Merged
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
pull Bot
pushed a commit
to CrazyForks/TablePro
that referenced
this pull request
Aug 1, 2026
… under (TableProApp#2004) (TableProApp#2006) 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.
Fixes #2004. Opening a SQL Server table or view outside the login's default schema failed with
Invalid object name.What the error actually says
Zooming the reporter's screenshot shows
Invalid object name 'def_en…with the quote against the name, so the statement carried a bare, unqualified object name. The schema was nil or blank when the SQL was built, not merely wrong. SQL Server has no session-level schema, so an unqualified name has nowhere to resolve; Postgres and MySQL hide the same class of bug behindsearch_pathand the default database.Ruling out the plugin
#1758 taught the MSSQL plugin to qualify browse queries, so the obvious theory was a stale driver. It is not:
nmon the published PluginKit-18 binary (v1.0.32, what 0.61.0 resolves to) showsMSSQLSchemaQueries.qualifiedNameandbrowse(schema:). The PluginKit-17 binary (v1.0.25) has neither and is the build that emitsSELECT * FROM [table].minimumCompatiblePluginKitVersion = 18rejects a kit-17 plugin on 0.61.0 and reconciliation replaces it, so a user on this version cannot be silently running the broken build.Checked against SQL Server 2022 in Docker with the reporter's shape (login default schema
dbo, tables only incustom):SELECT SCHEMA_NAME()returnsdboandINFORMATION_SCHEMA.SCHEMATAdoes listcustom, so the schema list is not being hidden by permissions either. The defect is app-side.Root cause
A table's schema was not part of its identity. It was stamped inconsistently at listing time and then re-guessed from
session.currentSchemaat query-build time, and that session value could itself be wrong. Four defects compound:1. A database switch desynced the session from the driver.
DatabaseManager.switchDatabasewrote the plugin's static default schema into the session for.bySchemaengines and never told the driver. Object listings read the driver's schema and table queries read the session's, so after any database switch the sidebar and the query were on different schemas, and the user's chosen schema was silently discarded.2.
PluginDriverAdapter.fetchTables()claimed every table had no schema. The rows came frompluginDriver.currentSchema, but the no-arg overload stampedschema: nilwhilefetchTables(schema:)directly below it stamped the real value. The no-arg one backsSchemaService's flat list, which feedssession.tables, favourites, the quick switcher, and autocomplete.3. The database tree threw away the schema it already had.
open(_ ref:)calledopenTableTab(ref.table, …)and droppedref.schema. It worked only becausemapPluginTablegained a back-fill on 2026-07-29 (#1989), one day before 0.61.0.4. The tree's corrective schema switch was gated on the wrong value.
activate(_:)compared againsttoolbarState.currentSchema, the window's mirror, which a database switch does not update. Running immediately after defect 1 moved the session schema, the guard skipped the switch exactly when the session needed it.The fix
Resolve a table's schema once, where the row is listed, and stop treating
session.currentSchemaas a source of truth:switchDatabasemoves the driver to the engine's default schema and then records what the driver is actually using, so session and driver can never disagree.MainContentCoordinator.switchDatabaserefreshes the toolbar mirror through the same path.resolvedSchemaNametreats a blank schema as absent at both tiers. A blank name is the one value that reaches a qualifier and produces an unqualified[table].fetchTables()delegates tofetchTables(schema:)so both listing paths stamp the schema the rows came from.ref.schemaexplicitly and gates its schema switch on the live session schema.MSSQLPluginDriver.fetchTablespopulatesPluginTableInfo.schemaat the source, and every schema fallback in the driver routes through oneeffectiveSchemahelper that also rejects a blank name.Plugin release
The MSSQL change is additive (call sites only, no new protocol requirement, no transfer-type layout change), so no PluginKit ABI bump. It needs a
plugin-mssql-v1.0.33release to reach users. The app-side fixes stand on their own without it: the adapter back-fills the schema regardless of what the driver reports.Tests
Ten new tests:
fetchTables()stamps the schema its rows were read from, and stays schema-less for an engine without schemas.[custom].[def_encounter].The existing test asserting
switchDatabasewrites the plugin default into the session encoded defect 1. It now asserts the driver moved too and that both agree.Verification
BUILD SUCCEEDEDfor the app and for theMSSQLDriverscheme (the plugin target is not in the app scheme, so it was built separately).swiftlint lint --strictclean on every changed file; the four violations in the MSSQL plugin files are pre-existing onmainand were left alone.Two things are honestly unverified:
TableProTestsfails 74 tests locally, and 74 onmainas well, with membership shuffling between runs as a parallel worker crashes and drags whatever it was running down with it. The new suites pass in isolation and alongside the suites they collided with.Follow-up
Oracle's
fetchTableshas the same shape and is the other engine with no session-schema fallback. Left out of this PR deliberately.