fix(coordinator): drop materialized views and foreign tables with the correct DROP statement#1801
Conversation
… correct DROP statement (#1800)
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 55f7f7f95d
ℹ️ 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".
| return Set(session.tables.filter { $0.type == .view }.map(\.name)) | ||
| tableInfoProvider: { | ||
| guard let session = DatabaseManager.shared.session(for: self.connectionId) else { return [:] } | ||
| return Dictionary(session.tables.map { ($0.name, $0) }, uniquingKeysWith: { first, _ in first }) |
There was a problem hiding this comment.
When the sidebar/database tree has objects with the same name in different schemas, the pending operation still carries only the bare table name, but this new lookup collapses session.tables by name and keeps an arbitrary/first TableInfo. Because the builder now uses that TableInfo.schema to qualify DROP/TRUNCATE, selecting sales.orders can preview or execute against public.orders if that entry is first or the current schema cache is stale after a schema switch. Please carry the selected schema (or the full TableInfo) through the pending operation instead of resolving destructive SQL by bare name.
Useful? React with 👍 / 👎.
1 participant
Fixes #1800.
Problem
Dropping a materialized view from the sidebar sent
DROP TABLE <name>, which the server rejects (PostgreSQL:"name" is not a table). Foreign tables hit the same bug.Root cause
The metadata pipeline, sidebar sections, and context-menu labels all carry the correct object kind (
TableInfo.TableType), but the drop action stores only names inpendingDeletes: Set<String>. At SQL-generation time the kind was re-derived asisView: Boolfrom a closure that only recognized.view, so materialized views and foreign tables fell into theDROP TABLEbranch.A second bug sat one layer up: the ClickHouse driver classified engines with
engine?.contains("View"), which matches"MaterializedView"as a substring, so ClickHouse materialized views were misfiled as plain views at the metadata level.Fix
TableOperationSQLBuildernow takestableInfoProvider: () -> [String: TableInfo]instead ofviewNamesProvider: () -> Set<String>and maps the kind through an exhaustive switch:VIEW,MATERIALIZED VIEW,FOREIGN TABLE, andTABLEfor tables, system tables, and lookup misses (miss logs an OSLog warning). Drop and truncate statements are now schema-qualified from the same lookup.dropObjectStatement(objectType: String)already accepts a free-form keyword.MaterializedView->MATERIALIZED VIEW;View/LiveView/WindowView->VIEW), adropObjectStatementoverride that emitsDROP VIEWfor materialized views (ClickHouse has noDROP MATERIALIZED VIEWstatement), and the.materializedViewscapability so they get their own sidebar section and the "Drop Materialized View" menu label.Tests
TableOperationSQLBuilderTests: one case per object kind, cascade, schema qualification, and the lookup-miss fallback. The old suite ran with no session registered, soisView: truewas never exercised; this is the gap that let the bug ship.ClickHouseTableOperationsTestsfor the engine classifier and drop statement (compiled intoTableProTestsvia a pbxproj membership exception, same pattern asMySQLQueryTimeoutStatement.swift).PluginDriverAdapterTableOpsTestsgainsMATERIALIZED VIEWandFOREIGN TABLEfallback cases.Notes
pendingDeletesis keyed by bare name, so two same-named objects in different schemas cannot be pending-dropped at once. Worth a separate issue.