Skip to content

fix(editor): stop completing another schema's tables without their schema - #3070

Merged
datlechin merged 2 commits into
mainfrom
fix/completion-other-schema-unqualified
Sep 23, 2026
Merged

datlechin merged 2 commits into
mainfrom
fix/completion-other-schema-unqualified

Conversation

@datlechin

Copy link
Copy Markdown
Member

The defect

On PostgreSQL with a tab on public, typing SELECT * FROM attendance. completes attendance's tables, which is right. From then on, a plain SELECT * FROM times offers timesheet and inserts it bare. The server answers relation "timesheet" does not exist, because attendance is not on the search_path.

The same happens without any dot completion: expand attendance in the sidebar and every one of its tables is offered bare in FROM, JOIN, INTO, DROP, CREATE INDEX, CREATE VIEW and at statement start.

Root cause

SQLSchemaProvider.tables is every table the scope's owner loaded. For the browse scope that is the union of every schema the sidebar has expanded (SchemaRefreshService.syncAutocompleteProvider fills it from SchemaService.allLoadedTables). Two readers treated it as "names this statement can write without a schema":

  • tableCompletionItems(), behind every bare table slot.
  • The last pass of resolveAlias, which matched timesheet. to a table only another schema holds and then fetched its columns from the tab's own schema.

On top of that, tableCompletionItems(inSchema:) folded the tables it fetched on demand into tables through mergeTables. That made it a second, unordered writer of a list SchemaRefreshService and SchemaProviderRegistry each own alone, and the widening lasted until the next reset.

The fix

  • tablesResolvableUnqualified derives the bare-name set from tables and getDefaultSchema(), the provider's existing answer to "the schema a table can be named in without its schema" (the engine's implicit schema, otherwise the schema the scope's driver is on). A table with no schema, and every table on an engine that reports no current schema (MySQL, SQLite, MongoDB and the rest), stays in it. tableCompletionItems() and resolveAlias read it.
  • Another schema's tables fetched on demand live in their own cache, keyed by schema and cleared by resetForDatabase. mergeTables is gone, so tables has one writer again. Concurrent completions of one schema await the fetch in flight, the way loadTask and fieldPaths coalesce, and a fetch that a database switch overtook answers its own caller without writing into the new scope. An empty schema is cached as an answer; a failed fetch is not.
  • getTables(), the AI context and the Copilot preamble keep the full union. They already render every table outside the default schema qualified through SchemaQualifiedName. No API that Open Quickly calls on main changed.

This is a refactor of the provider's table model rather than a patch on one method: the list keeps one meaning, and the two readers that needed a narrower one get it from a single derived property, so a third writer that widens the list cannot bring the bug back.

Behaviour chosen: exclude, not qualify

Bare table completion now offers only tables the server resolves without a schema. A table anywhere else is reached by typing its schema and a dot, which lists that schema's tables whether or not the sidebar has opened it.

Why not offer attendance.timesheet in the bare list instead:

  • DataGrip's default for SQL completion is the current scope: the console's schema plus system schemas. Objects from every schema come from a separate setting or a second invocation of completion, and only then does its "Qualify objects with" option matter. TablePlus has an "Auto prefix schemas" option for the same case. Neither puts another schema's tables in the default bare list.
  • The docs already promise that a tab's suggestions come from its own database and schema, and describe schema. as the way into another one.
  • What the union holds depends on which schemas the sidebar happens to have expanded. Bare completion should not change with tree state.
  • Inserting a qualified name correctly needs a quote-if-needed rule that knows each engine's case folding and reserved words. The completion layer has no such rule for any identifier today (bare names insert unquoted everywhere), so adding one for this item kind alone would give qualified and bare insertions two different rules. An "all schemas, qualified" mode is a feature of its own if wanted.

Tests

SQLSchemaProviderUnqualifiedScopeTests, 10 cases:

Case Before the fix
An expanded schema's table is not offered bare fails: timesheet offered
Completing attendance. leaves bare completion and getTables() alone fails: merged into both
SELECT * FROM times after SELECT * FROM attendance., through SQLCompletionProvider fails: timesheet offered
resolveAlias("timesheet") with no reference naming it fails: resolves
Spanner: the implicit schema, not the browsed one, is what a bare name reaches fails
An empty schema is fetched once fails: fetched every keystroke
Concurrent completions of one schema share one fetch fails: two fetches
A fetch a database switch overtook does not answer for the new scope fails: stale tables
An engine with no current schema offers every table passes (guard)
A failed fetch is asked again passes (guard)

Before the fix (provider restored to main, tests unchanged): 10 executed, 8 failed, 2 passed, every failure on the expectation it names (labels → ["users", "timesheet"], script.calls → 2, fresh → ["timesheet"]).

After the fix, the new suite plus the 20 suites that own the provider, its registry and the completion layer: 428 executed, 411 passed. SQLSchemaProviderUnqualifiedScopeTests 10/10, SQLSchemaProviderTests 30/30, SQLSchemaProviderFallbackTests 18/18, SQLCompletionProviderTests 127/127, SchemaProviderRegistryTests 19/19, SchemaRefreshServiceTests 11/11, SchemaContextForAITests 6/6, CopilotPreambleBuilderTests 1/1, and the rest of the completion suites all green. The 17 failures are all QuickSwitcherViewModelTests filter cases that sleep a fixed 200ms for a 40ms debounce; none touches the provider, and the identical 17 failed on another branch without this change while the machine's load average was above 400. The build is the one the test step compiled.

swiftlint lint --strict: 0 violations. verify.sh docs: pass.

Review

Codex could not run either pass: the workspace hit its usage limit until Sep 29. The code-review fallback ended on an API session limit before reporting anything. A separate reviewer on another model read the diff and traced the reentrancy fence in onDemandTables(inSchema:), the task's isolation, how getDefaultSchema() is populated on the browse and tab scopes, and the remaining readers of tables, and found no defect.

Known limitation, unchanged by this PR

On SQL Server, switchSchema moves the plugin's currentSchema as a browse cursor, while bare names in user SQL resolve through the login's default schema (SCHEMA_NAME(), read at connect). With the tab on a non-default schema, bare completion now offers that schema's tables, as it did before, and no longer offers an expanded dbo's. The AI context, the Copilot preamble and the fallback column labels already read getDefaultSchema() the same way. Separating "browsed schema" from "schema bare names resolve in" needs a driver-level value and a PluginKit change.

Docs: docs/features/autocomplete.mdx, the Schema names section, now says where a bare name completes from and how to reach another schema.

Found while investigating #3048 (see #3060).

@mintlify

mintlify Bot commented Sep 23, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated
TablePro 🟢 Ready View Preview Sep 23, 2026, 7:46 AM

💡 Tip: Enable Automations to automatically generate PRs for you.

Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
@datlechin
datlechin merged commit 5cff78b into main Sep 23, 2026
3 checks passed
@datlechin
datlechin deleted the fix/completion-other-schema-unqualified branch September 23, 2026 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant