Skip to content

fix(plugin-oracle): stop infinite loading when switching schemas#1811

Merged
datlechin merged 2 commits into
mainfrom
fix/oracle-schema-switch-hang-1807
Jul 4, 2026
Merged

fix(plugin-oracle): stop infinite loading when switching schemas#1811
datlechin merged 2 commits into
mainfrom
fix/oracle-schema-switch-hang-1807

Conversation

@datlechin

@datlechin datlechin commented Jul 4, 2026

Copy link
Copy Markdown
Member

Fixes #1807.

Root cause

Three layered problems produced the infinite spinner:

  1. Oracle's plugin metadata described a two-tier engine. OraclePlugin never overrode supportsDatabaseSwitching (protocol default true) or defaultSchemaName (default "public"). containerSwitchTarget therefore returned .database, so the switcher routed through switchDatabase, whose .bySchema branch reset session.currentSchema to "public", a schema that does not exist on any Oracle server. Oracle has one tier: a schema is a user and is the only switchable container.
  2. Nothing could time out. The corrupted schema became the MetadataConnectionPool key. The pool opened a fresh Oracle connection and ran the schema switch with no bound. Oracle was the only plugin implementing no query timeout, and its QueryGate serializes all queries, so one stuck statement blocked the connection forever.
  3. The spinner had no failure state. reloadSchema's catch block only logged, so SchemaService never left .loading when driver acquisition failed.

Fix

Plugin side (Plugins/OracleDriverPlugin/, ships via the next plugin-oracle-v* release, no PluginKit ABI change):

  • Declare the one-tier model BigQuery already uses: supportsDatabaseSwitching = false, databaseGroupingStrategy = .hierarchicalSchema, defaultSchemaName = "", containerEntityName = "Schema". The sidebar becomes the hierarchical schema tree (schemas with lazily loaded tables). The old two-level tree ran the same ALL_USERS query at both levels and showed the identical list twice.
  • Implement applyQueryTimeout. Queries race the configured timeout (Settings > General, 60s default). On timeout the connection is closed first, which fails the in-flight OracleNIO call even if it ignores task cancellation, then the query gate is released so queued queries (including the health ping) fail fast instead of hanging.
  • The next query on a closed connection reconnects automatically and re-applies the session schema, so "run the query again" works immediately after a timeout instead of waiting for the 30s health ping.
  • New queryTimedOut error category with a diagnostic (retry, raise the timeout, busy-server hint).
  • No cancelQuery implementation: the app cancels queries in supersede style (refresh, pagination, new run) and expects the connection to survive, which Oracle cannot honor without a server-side cancel. The timeout bounds stuck queries instead.

App side (ships with the next app release):

  • Mirror the four metadata fields in the static registry fallback, and normalize legacy plugin metadata at registration: when the app's registry default declares an engine schema-only but the loaded plugin still reports the old two-tier declaration, the registry applies its own switch-routing fields. Existing users get correct schema switching without updating the plugin.
  • MetadataConnectionPool bounds the post-connect schema switch (15s) for every plugin. The shared withTimeout helper gained an onTimeout hook (additive PluginKit overload) that force-disconnects the driver, so a driver that ignores task cancellation still unwinds; the pool and the Oracle wrapper share one race implementation.
  • SchemaService.markLoadFailed surfaces .failed when driver acquisition fails (without clobbering already-loaded tables); a failed schema-list fetch on the hierarchical path stamps .failed instead of rendering an empty tree; the sidebar error state gained a Retry button, and Retry reports a clear failure when the session is unavailable instead of silently doing nothing.
  • Load-time log warning when a plugin declares the contradictory hierarchicalSchema + supportsDatabaseSwitching pair.

Deliberately unchanged: DatabaseManager.switchDatabase's .bySchema schema reset. MSSQL depends on it (its driver never updates its own schema on a database switch), and a new test locks that in.

Version skew

  • New app + old (already-installed) plugin: schema switching routes correctly via registration-time normalization; the pool bounds metadata loads; the plugin update adds query timeout enforcement and auto-reconnect.
  • Old app + new plugin: metadata is read live from the loaded plugin, so routing is fixed; timeout support works because the app already calls applyQueryTimeout generically.

No currentPluginKitVersion bump: the withTimeout(onTimeout:) overload is a new symbol (additive), and every other touched API already exists with defaults.

Tests

  • ContainerEntityNameTests: Oracle targets .schema, no database switching, Schema container, hierarchical grouping, empty default schema.
  • PluginMetadataSwitchRoutingTests (new): legacy two-tier declarations are detected against a schema-only registry default, and normalization carries over exactly the four routing fields.
  • SwitchContainerTests (new): coordinator-level regression proving switchContainer(to: "HR") on an Oracle connection routes to switchSchema and updates toolbar and session state.
  • DatabaseManagerDatabaseSwitchTests (new): MSSQL database switch still resets the session schema to dbo.
  • MetadataConnectionPoolTests (new): hanging connect and schema switch fail within the deadline; a driver that ignores cancellation is force-disconnected and still fails in time; fast paths pass through.
  • AsyncTimeoutTests: onTimeout unblocks a cancellation-deaf operation and is not invoked when the operation wins.
  • SchemaServiceTests: markLoadFailed semantics, hierarchical schema list success and failure states, and Retry-without-session feedback.

No UI automation: the flow needs a live Oracle server, which the UI test runner does not have. The Oracle wrapper's timeout and reconnect internals need a manual pass against a real server (gvenzl/oracle-xe:21-slim).

Docs

  • docs/customization/settings.mdx: documents Oracle's client-side timeout enforcement (connection reset + auto-reconnect).
  • docs/databases/oracle.mdx: describes the hierarchical schema sidebar.

Manual verification checklist

  • Open an Oracle connection, switch schemas from the toolbar chip or Cmd+K: the switch is immediate, the sidebar lists all schemas, tables load on expand.
  • Kill network mid-load: the sidebar shows an error with Retry within 15s instead of spinning.
  • Long query with a 60s timeout: fails with the query-timeout diagnostic; running it again reconnects immediately and keeps the selected schema.
  • With the OLD Oracle plugin still installed: schema switching works (normalized routing); metadata loads are bounded.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 69cee7c266

ℹ️ 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".

Comment on lines +414 to +415
} catch let timeout as TimeoutError {
throw queryTimeoutError(timeout)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Release the Oracle query gate on timeout

When an Oracle query times out while another operation has already passed requireConnection() and is waiting in queryGate.acquire() (for example a schema reload or a second tab query), this timeout branch throws without releasing the gate. Because withQueryDeadline closes the connection and then throws TimeoutError, the queued waiter is never resumed, leaving that operation stuck indefinitely instead of failing/reconnecting; the streaming timeout branch has the same missing release.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f37b876afd

ℹ️ 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".

static let brandColorHex = "#C3160B"
static let systemDatabaseNames: [String] = ["SYS", "SYSTEM", "OUTLN", "DBSNMP", "APPQOSSYS", "WMSYS", "XDB"]
static let databaseGroupingStrategy: GroupingStrategy = .bySchema
static let databaseGroupingStrategy: GroupingStrategy = .hierarchicalSchema

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Move Oracle system schemas into schema metadata

After switching Oracle to .hierarchicalSchema, the UI paths for hierarchical/schema-only containers filter and mark system entries via systemSchemaNames (e.g. the sidebar tree and schema switcher), not systemDatabaseNames. Since Oracle's SYS/SYSTEM/etc. list is still only declared as systemDatabaseNames while fetchSchemas() returns ALL_USERS, those accounts will now appear as ordinary selectable schemas and won't be hidden behind the existing system-schema controls; the mirrored registry default needs the same migration.

Useful? React with 👍 / 👎.

@datlechin datlechin merged commit 2c89be8 into main Jul 4, 2026
4 checks passed
@datlechin datlechin deleted the fix/oracle-schema-switch-hang-1807 branch July 4, 2026 16:36
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.

[oracle] hangine when switch schema

1 participant