Skip to content

fix(sidebar): key per-schema object lists by database - #3095

Merged
datlechin merged 2 commits into
mainfrom
fix/schema-service-hierarchical-caches
Sep 23, 2026
Merged

datlechin merged 2 commits into
mainfrom
fix/schema-service-hierarchical-caches

Conversation

@datlechin

Copy link
Copy Markdown
Member

Found while investigating #3048 (#3060). The two query storms from the same investigation follow in a PR stacked on this one.

The defect

On an engine grouped by hierarchical schema that changes database on a live connection (Snowflake and Trino), switching database left the previous database's objects under the new one's schema rows:

  1. Browse SALES on Snowflake and expand PUBLIC: it lists ORDERS.
  2. Switch the connection to MARKETING, which also has a PUBLIC.
  3. PUBLIC under MARKETING still lists ORDERS until its reload comes round, and keeps listing it for good if that reload fails.

Autocomplete and Open Quickly read the same lists, so they offered SALES tables while browsing MARKETING. Oracle, BigQuery, Spanner, Dameng and Cloudflare R2 SQL are grouped the same way but never change database, so they could not hit it.

Root cause

SchemaService kept its per-schema lists in perSchemaStates and perSchemaSideObjects, keyed by connection and schema name only (SchemaKey(connectionId, schema)). An in-place switch (DatabaseManager.switchDatabase, the PluginDriverAdapter branch) never invalidates SchemaService; only the reconnect-to-switch path does. So once the new database's schema list committed:

  • a PUBLIC that exists in both databases answered with the old database's tables until refreshLoadedSchemaObjects reloaded it, one schema after another;
  • a reload that failed kept the old tables, because a failure never replaces loaded content;
  • loadSchemaObjects returned early for any schema still marked loaded, so expanding PUBLIC under the new database never fetched it;
  • a fetch for the old database that finished after the switch committed into the new database's slot;
  • every loaded schema of the old database was read again against the new one on the refresh that follows a switch.

Open Quickly already carried a workaround for this: on a hierarchical engine the all-schema listing overrode the schema service once it arrived. The investigation behind #3063 hit the same missing key independently and cut its catalog-based path for it.

The fix

A refactor rather than a patch: the key was missing a dimension, and the workaround downstream was compensating for it.

  • SchemaKey carries the database. Every per-schema load names the scope it reads (loadSchemaObjects(schema:in:driver:), reloadSchemaObjects(schema:in:driver:), refreshLoadedSchemaObjects(in:driver:)), and its rows are kept under that scope's database.
  • Every per-schema read answers for the database the schema list was loaded from (loadedScopes), so the schema rows and the objects under them always describe one database, including while a switch settles.
  • When the loaded catalog moves to another database, the lists of the database left are dropped, their fetch generations removed (a late fetch commits nothing) and their in-flight fetches cancelled. Objects read for the new database before its schema list arrived are kept.
  • noteScopeCovered no longer records a scope in another database as covered by what is loaded.
  • Open Quickly's hierarchical exception is removed. The schema service's lists answer for the database they came from, the same rule as every other engine.

Measured

Mock driver counting fetchTables(schema:), Snowflake connection, the refresh that follows a switch from SALES (two schemas expanded) to MARKETING:

Before After
Reads against MARKETING on behalf of SALES's loaded schemas one per loaded schema, each also fetching routines (2 queries a schema; 3 on Oracle with triggers) 0 (measured)
PUBLIC under MARKETING right after the switch ORDERS, from SALES nothing yet; read when expanded

The before column comes from the code path (refreshLoadedSchemaObjects reloaded every loaded or loading key of the connection). My red run of the new suite against unmodified main was lost to a full disk on the build machine (every case reported at 0.000 s with ENOSPC in the log), so the before numbers are not a measurement. The edit that turns the suite red is dropping database from SchemaKey, which is the old shape.

Tests

New SchemaServiceDatabaseSwitchTests (6 cases): a switch does not carry PUBLIC's tables or routines across; a failed load after a switch reports the failure instead of the old tables; the refresh after a switch reads nothing for the old database; a load for the old database finishing after the switch is discarded; objects loaded for the new database before its schema list arrives are kept without a second read; switching back reads the first database again.

Updated in the same commit for the new signatures: SchemaServiceTests, SchemaServiceRefreshTests, SchemaServiceHierarchicalTests, SchemaServiceSideObjectsTests, QuickSwitcherCrossSchemaTests (the hierarchical exception's two cases replaced by one asserting a loaded hierarchical schema keeps the listing's stale rows out, and one for the stand-in before the listing arrives).

Verification

  • verify.sh test over SchemaServiceDatabaseSwitchTests SchemaServiceTests SchemaServiceRefreshTests SchemaServiceHierarchicalTests SchemaServiceSideObjectsTests SchemaServiceRoutinesTests SchemaRefreshServiceTests SchemaRefreshAfterWriteTests QuickSwitcherCrossSchemaTests QuickSwitcherViewModelTests CatalogEditAdoptionTests: PASS, 155 executed, 155 passed (the build step is part of the run).
  • verify.sh lint on the 9 changed Swift files: 0 violations.

Reviewed with a feature-dev:code-reviewer agent (Codex is out of quota until Sep 29). It found nothing at its confidence bar; its one coverage note, the side-object lists across a switch, is now asserted in switchDoesNotCarryTablesAcross.

No UI automation: reproducing it needs a live Snowflake or Trino server, and the switch is covered at the service boundary the sidebar reads.

@datlechin
datlechin merged commit e1aeae1 into main Sep 23, 2026
8 of 9 checks passed
@datlechin
datlechin deleted the fix/schema-service-hierarchical-caches branch September 23, 2026 19:24
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