From ba422f96f8b83c4d1f6cb555a7920f316c415c01 Mon Sep 17 00:00:00 2001 From: zipg Date: Fri, 25 Sep 2026 09:37:11 +0800 Subject: [PATCH] fix(query): single-column results editable on case-preserving engines --- .../sql/editableQueryHiddenKeys.spec.ts | 19 ++++++++++ .../metadataColumnNameCaseFallback.spec.ts | 38 +++++++++++++++++++ apps/desktop/src/lib/sql/sqlAnalysis.ts | 20 +++++++--- .../queryStore.hiddenPrimaryKey.spec.ts | 36 ++++++++++++++++++ apps/desktop/src/stores/queryStore.ts | 7 +++- 5 files changed, 113 insertions(+), 7 deletions(-) create mode 100644 apps/desktop/src/lib/__tests__/sql/metadataColumnNameCaseFallback.spec.ts diff --git a/apps/desktop/src/lib/__tests__/sql/editableQueryHiddenKeys.spec.ts b/apps/desktop/src/lib/__tests__/sql/editableQueryHiddenKeys.spec.ts index daed7b4c5a..3c2a38021d 100644 --- a/apps/desktop/src/lib/__tests__/sql/editableQueryHiddenKeys.spec.ts +++ b/apps/desktop/src/lib/__tests__/sql/editableQueryHiddenKeys.spec.ts @@ -29,6 +29,25 @@ describe("editable query hidden primary keys", () => { expect(result?.projections.map((projection) => projection.alias)).toEqual(["__DBX_PK_1", "__DBX_PK_2"]); }); + it("quotes the appended key with each reported engine's own dialect", () => { + // #10233 reports OceanBase (Oracle mode), Dameng and HighGo. All three draw their + // quoting from the dialect adapter rather than a hard-coded quote character: + // Dameng/OceanBase Oracle mode ride the Oracle adapter, HighGo the PostgreSQL one. + for (const databaseType of ["dameng", "oceanbase-oracle", "highgo"] as const) { + expect( + buildQueryWithHiddenPrimaryKeys({ + sql: "SELECT name FROM users", + databaseType, + primaryKeys: ["id"], + existingResultNames: ["name"], + }), + ).toEqual({ + sql: 'SELECT name, "id" AS "__DBX_PK_0" FROM users', + projections: [{ sourceName: "id", alias: "__DBX_PK_0" }], + }); + } + }); + it("preserves SQL Server TOP and Oracle optimizer hints", () => { expect( buildQueryWithHiddenPrimaryKeys({ diff --git a/apps/desktop/src/lib/__tests__/sql/metadataColumnNameCaseFallback.spec.ts b/apps/desktop/src/lib/__tests__/sql/metadataColumnNameCaseFallback.spec.ts new file mode 100644 index 0000000000..97c3e07621 --- /dev/null +++ b/apps/desktop/src/lib/__tests__/sql/metadataColumnNameCaseFallback.spec.ts @@ -0,0 +1,38 @@ +import { describe, expect, it } from "vitest"; +import { resolveMetadataColumnName } from "@/lib/sql/sqlAnalysis"; + +/** + * Dialect folding (Oracle-compatible -> upper case, PostgreSQL-compatible -> + * lower case) is only an assumption about how the server stores an unquoted + * identifier. Dameng keeps the case the identifier was created with, so a table + * created as `(id int primary key, name varchar(50))` reports lowercase columns + * while the analyzer folds the query's `name` to `NAME` (issue #10233). The + * folded spelling must stay the primary answer, and a case-only match is only a + * rescue when it identifies exactly one column. + */ +describe("metadata column name resolution tolerates server-preserved casing", () => { + it("rescues a lowercase dameng column when the folded spelling does not exist", () => { + expect(resolveMetadataColumnName("dameng", "name", false, ["id", "name", "amount"])).toBe("name"); + }); + + it("still prefers the folded spelling when the server stored upper case", () => { + expect(resolveMetadataColumnName("dameng", "name", false, ["ID", "NAME"])).toBe("NAME"); + }); + + it("rescues a PostgreSQL-compatible engine that reports an upper-case column", () => { + expect(resolveMetadataColumnName("highgo", "label", false, ["ID", "LABEL"])).toBe("LABEL"); + }); + + it("keeps PostgreSQL lower-case folding unchanged", () => { + expect(resolveMetadataColumnName("highgo", "Label", false, ["id", "label"])).toBe("label"); + }); + + it("refuses a case-only match that is ambiguous", () => { + expect(resolveMetadataColumnName("dameng", "value", false, ["value", "Value"])).toBeUndefined(); + }); + + it("keeps quoted identifiers exact", () => { + expect(resolveMetadataColumnName("dameng", "name", true, ["NAME"])).toBeUndefined(); + expect(resolveMetadataColumnName("dameng", "NAME", true, ["NAME"])).toBe("NAME"); + }); +}); diff --git a/apps/desktop/src/lib/sql/sqlAnalysis.ts b/apps/desktop/src/lib/sql/sqlAnalysis.ts index 126b189709..eeae86c993 100644 --- a/apps/desktop/src/lib/sql/sqlAnalysis.ts +++ b/apps/desktop/src/lib/sql/sqlAnalysis.ts @@ -75,23 +75,31 @@ export function resolveMetadataColumnName(databaseType: string, sourceName: stri if (sourceNameQuoted === undefined) { const exact = metadataColumns.find((column) => column === sourceName); if (exact || POSTGRES_FOLDED_IDENTIFIER_TYPES.has(databaseType) || ORACLE_FOLDED_IDENTIFIER_TYPES.has(databaseType)) return exact; - const caseOnlyMatches = metadataColumns.filter((column) => column.toLowerCase() === sourceName.toLowerCase()); - return caseOnlyMatches.length === 1 ? caseOnlyMatches[0] : undefined; + return uniqueCaseInsensitiveColumn(metadataColumns, sourceName); } + // Dialect folding is only an assumption about what the server stored: Dameng + // keeps the case an unquoted identifier was created with (issue #10233), so + // the folded spelling can miss even though the column exists. Fall back to a + // case-only match when it is unambiguous -- never when the folded name itself + // resolves, so quoted or equally-spelled columns keep their exact identity. if (POSTGRES_FOLDED_IDENTIFIER_TYPES.has(databaseType)) { const folded = sourceName.toLowerCase(); - return metadataColumns.find((column) => column === folded); + return metadataColumns.find((column) => column === folded) ?? uniqueCaseInsensitiveColumn(metadataColumns, folded); } if (ORACLE_FOLDED_IDENTIFIER_TYPES.has(databaseType)) { const folded = sourceName.toUpperCase(); - return metadataColumns.find((column) => column === folded); + return metadataColumns.find((column) => column === folded) ?? uniqueCaseInsensitiveColumn(metadataColumns, folded); } const exact = metadataColumns.find((column) => column === sourceName); if (exact) return exact; - const caseOnlyMatches = metadataColumns.filter((column) => column.toLowerCase() === sourceName.toLowerCase()); - return caseOnlyMatches.length === 1 ? caseOnlyMatches[0] : undefined; + return uniqueCaseInsensitiveColumn(metadataColumns, sourceName); +} + +function uniqueCaseInsensitiveColumn(metadataColumns: readonly string[], name: string): string | undefined { + const matches = metadataColumns.filter((column) => column.toLowerCase() === name.toLowerCase()); + return matches.length === 1 ? matches[0] : undefined; } export interface ResolvedSourceColumnRef { diff --git a/apps/desktop/src/stores/__tests__/queryStore.hiddenPrimaryKey.spec.ts b/apps/desktop/src/stores/__tests__/queryStore.hiddenPrimaryKey.spec.ts index 95abf53318..09249f45e4 100644 --- a/apps/desktop/src/stores/__tests__/queryStore.hiddenPrimaryKey.spec.ts +++ b/apps/desktop/src/stores/__tests__/queryStore.hiddenPrimaryKey.spec.ts @@ -159,6 +159,42 @@ describe("queryStore hidden primary key editing", () => { expect(tab.queryEditabilityReason).toBeUndefined(); }, 10_000); + it("appends the hidden primary key for Dameng tables whose columns keep the created case", async () => { + // #10233: Dameng keeps the case an unquoted identifier was created with, so the + // server reports `name` while the Oracle-family dialect handling folds the SQL + // token to `NAME`. The binding has to survive that mismatch, otherwise the result + // stays read-only even though the table has a primary key. + getConnectionConfig.mockReturnValue({ id: "dameng-1", name: "Dameng", db_type: "dameng", database: "SYSDBA", query_timeout_secs: 30 }); + getColumns.mockResolvedValue([ + { name: "id", data_type: "INT", is_nullable: false, column_default: null, is_primary_key: true, extra: null }, + { name: "name", data_type: "VARCHAR(50)", is_nullable: true, column_default: null, is_primary_key: false, extra: null }, + ]); + // Mirror what the analyzer reports for this dialect: the user's unquoted token + // keeps its spelling and quoting flag (the store then folds it, like the real + // Oracle-family path), while the hidden key dbx appends is quoted as `"id"`. + analyzeEditableQueryEditability.mockImplementation(async (sql: string) => ({ + editable: true, + analysis: { + schema: undefined, + tableName: "users", + selectStar: false, + columns: [{ sourceName: "name", sourceNameQuoted: false, resultName: "name", expression: "name" }, ...(sql.includes("__DBX_PK_0") ? [{ sourceName: "id", sourceNameQuoted: true, resultName: "__DBX_PK_0", expression: '"id"' }] : [])], + }, + })); + + const { useQueryStore } = await import("@/stores/queryStore"); + const store = useQueryStore(); + const tabId = store.createTab("dameng-1", "SYSDBA", "Query"); + + await store.executeTabSql(tabId, "SELECT name FROM users"); + + expect(executeMulti).toHaveBeenCalledWith("dameng-1", "SYSDBA", 'SELECT name, "id" AS "__DBX_PK_0" FROM users', undefined, expect.any(String), expect.objectContaining({ timeoutSecs: 30 })); + const tab = store.tabs.find((item) => item.id === tabId)!; + expect(tab.result?.hidden_column_indexes).toEqual([1]); + await vi.waitFor(() => expect(tab.querySourceColumns).toEqual(["name", "id"])); + expect(tab.queryEditabilityReason).toBeUndefined(); + }, 10_000); + it("keeps MySQL expression columns read-only without disabling direct columns", async () => { const sql = "SELECT id, status, extra->>'$.mode' mode, extra->>'$.template' tmpl FROM items"; getColumns.mockResolvedValue([ diff --git a/apps/desktop/src/stores/queryStore.ts b/apps/desktop/src/stores/queryStore.ts index 5ac729d12c..2925747f4f 100644 --- a/apps/desktop/src/stores/queryStore.ts +++ b/apps/desktop/src/stores/queryStore.ts @@ -141,7 +141,12 @@ const groupedDisplayMetadataLimiter = new MetadataTaskLimiter(GROUPED_DISPLAY_ME console.debug("[DBX][metadata-load:grouped-display-limiter]", event); }); const UPPERCASE_FOLDED_METADATA_TYPES = new Set([...ORACLE_LIKE_METADATA_TYPES, "saphana"]); -const HIDDEN_QUERY_KEY_DATABASE_TYPES = new Set(["mysql", "postgres", "sqlserver", "oracle", "xugu"]); +// Engines whose SQL takes dbx's appended hidden primary-key projection. The rewrite +// quotes the key through the engine's own semantic dialect adapter, so Dameng and +// OceanBase Oracle mode ride the Oracle adapter and HighGo rides the PostgreSQL one +// (issue #10233: without this, a query that omits the primary key stays read-only +// even though the table has one). +const HIDDEN_QUERY_KEY_DATABASE_TYPES = new Set(["mysql", "postgres", "sqlserver", "oracle", "xugu", "dameng", "highgo", "oceanbase-oracle"]); const QUERY_RESULT_EXPORT_UNSUPPORTED_ERROR = "Streaming export is unsupported for this query. Simplify it or use a supported driver."; const BACKGROUND_CLIENT_SESSION_SUFFIXES = ["count", "explain", "export"] as const; const CANCEL_QUERY_TIMEOUT_MS = 10_000;