From 14bcde806fc3525a6aab8e8e00d8cda126121f2b Mon Sep 17 00:00:00 2001 From: U1 <56990620+CHNHJF@users.noreply.github.com> Date: Tue, 15 Sep 2026 13:28:40 +0800 Subject: [PATCH] fix(sql): honor include-database-name setting for schema-aware engines --- .../lib/__tests__/sql/newQueryContext.spec.ts | 40 ++++++++--- .../__tests__/table/tableSelectSql.spec.ts | 26 +++++++ apps/desktop/src/lib/table/tableSelectSql.ts | 18 +++++ packages/app-tests/tableSqlTemplates.test.ts | 67 ++++++++++++++++--- 4 files changed, 131 insertions(+), 20 deletions(-) diff --git a/apps/desktop/src/lib/__tests__/sql/newQueryContext.spec.ts b/apps/desktop/src/lib/__tests__/sql/newQueryContext.spec.ts index 50cc793279..3cd1e80b6f 100644 --- a/apps/desktop/src/lib/__tests__/sql/newQueryContext.spec.ts +++ b/apps/desktop/src/lib/__tests__/sql/newQueryContext.spec.ts @@ -204,16 +204,21 @@ describe("buildSelectAllSql", () => { expect(buildSelectAllSql("mysql", { database: "mydb", tableName: "users" }, undefined, undefined, true)).toBe("SELECT * FROM `mydb`.`users`"); }); - it("qualifies and quotes a PostgreSQL table with its schema", () => { - expect(buildSelectAllSql("postgres", { schema: "public", tableName: "users" })).toBe('SELECT * FROM "public"."users"'); + it("omits the PostgreSQL schema qualifier by default; including it is opt-in (#9110)", () => { + expect(buildSelectAllSql("postgres", { schema: "public", tableName: "users" })).toBe('SELECT * FROM "users"'); + expect(buildSelectAllSql("postgres", { schema: "public", tableName: "users" }, undefined, undefined, true)).toBe('SELECT * FROM "public"."users"'); }); - it("can omit identifier quotes while preserving schema qualification", () => { - expect(buildSelectAllSql("postgres", { schema: "public", tableName: "users" }, undefined, undefined, false, false)).toBe("SELECT * FROM public.users"); + it("can omit identifier quotes for a PostgreSQL table", () => { + expect(buildSelectAllSql("postgres", { schema: "public", tableName: "users" }, undefined, undefined, true, false)).toBe("SELECT * FROM public.users"); }); - it("preserves the Phoenix schema for new-query prefill", () => { - expect(buildSelectAllSql("jdbc", { schema: "APP", tableName: "USERS" }, '"', "phoenix")).toBe('SELECT * FROM "APP"."USERS"'); + it("preserves the Phoenix schema for new-query prefill when qualification is on", () => { + expect(buildSelectAllSql("jdbc", { schema: "APP", tableName: "USERS" }, '"', "phoenix", true)).toBe('SELECT * FROM "APP"."USERS"'); + }); + + it("prefills Phoenix tables unqualified by default (#9110)", () => { + expect(buildSelectAllSql("jdbc", { schema: "APP", tableName: "USERS" }, '"', "phoenix")).toBe('SELECT * FROM "USERS"'); }); it("scopes InfluxDB 1.x / 2.x prefill to a rolling InfluxQL window", () => { @@ -243,13 +248,15 @@ describe("buildSelectAllSql", () => { expect(buildSelectAllSql("starrocks", { catalog: "paimon_catalog", database: "bi", tableName: "events" })).toBe("SELECT * FROM `paimon_catalog`.`bi`.`events`"); }); it("uses the driver-reported identifier quote for Kingbase MySQL compat mode", () => { - expect(buildSelectAllSql("kingbase", { schema: "audit_schema", tableName: "events" }, "`")).toBe("SELECT * FROM `audit_schema`.`events`"); + expect(buildSelectAllSql("kingbase", { schema: "audit_schema", tableName: "events" }, "`", undefined, true)).toBe("SELECT * FROM `audit_schema`.`events`"); + expect(buildSelectAllSql("kingbase", { schema: "audit_schema", tableName: "events" }, "`")).toBe("SELECT * FROM `events`"); }); it("uses the driver-reported identifier quote for Kingbase PostgreSQL mode", () => { - expect(buildSelectAllSql("kingbase", { schema: "audit_schema", tableName: "events" }, '"')).toBe('SELECT * FROM "audit_schema"."events"'); + expect(buildSelectAllSql("kingbase", { schema: "audit_schema", tableName: "events" }, '"', undefined, true)).toBe('SELECT * FROM "audit_schema"."events"'); + expect(buildSelectAllSql("kingbase", { schema: "audit_schema", tableName: "events" }, '"')).toBe('SELECT * FROM "events"'); }); it("falls back to double quotes for Kingbase when no identifier quote is reported", () => { - expect(buildSelectAllSql("kingbase", { schema: "audit_schema", tableName: "events" })).toBe('SELECT * FROM "audit_schema"."events"'); + expect(buildSelectAllSql("kingbase", { schema: "audit_schema", tableName: "events" }, undefined, undefined, true)).toBe('SELECT * FROM "audit_schema"."events"'); }); }); @@ -272,7 +279,7 @@ describe("isNewQueryPrefillSupported", () => { }); describe("resolveNewQueryInitialSql", () => { - it("prefills SQL from the active table when enabled", () => { + it("prefills bare table SQL from the active table by default; the database name is opt-in", () => { expect( resolveNewQueryInitialSql({ activeTab: dataTab(), @@ -281,6 +288,16 @@ describe("resolveNewQueryInitialSql", () => { targetDatabase: "app_db", databaseType: "postgres", }), + ).toBe('SELECT * FROM "users"'); + expect( + resolveNewQueryInitialSql({ + activeTab: dataTab(), + prefillEnabled: true, + targetConnectionId: "conn-1", + targetDatabase: "app_db", + databaseType: "postgres", + includeDatabaseName: true, + }), ).toBe('SELECT * FROM "public"."users"'); }); @@ -292,6 +309,7 @@ describe("resolveNewQueryInitialSql", () => { targetConnectionId: "conn-1", targetDatabase: "app_db", databaseType: "postgres", + includeDatabaseName: true, quoteIdentifiers: false, }), ).toBe("SELECT * FROM public.users"); @@ -308,7 +326,7 @@ describe("resolveNewQueryInitialSql", () => { driverProfile: "phoenix", identifierQuote: '"', }), - ).toBe('SELECT * FROM "APP"."USERS"'); + ).toBe('SELECT * FROM "USERS"'); }); it("leaves new queries empty when the setting is disabled", () => { diff --git a/apps/desktop/src/lib/__tests__/table/tableSelectSql.spec.ts b/apps/desktop/src/lib/__tests__/table/tableSelectSql.spec.ts index 252b40aac4..4fe35f3222 100644 --- a/apps/desktop/src/lib/__tests__/table/tableSelectSql.spec.ts +++ b/apps/desktop/src/lib/__tests__/table/tableSelectSql.spec.ts @@ -141,6 +141,32 @@ describe("qualifiedTableName — schema-aware JDBC profiles", () => { }); }); +describe("qualifiedTableName — includeDatabaseName on quoted-identifier paths (#9110)", () => { + it("drops the schema qualifier for JDBC-reported Postgres-family connections", () => { + expect(qualifiedTableName({ databaseType: "postgres", identifierQuote: '"', schema: "public", tableName: "Users", includeDatabaseName: false })).toBe('"Users"'); + }); + + it("keeps the schema qualifier by default for quoted-identifier connections", () => { + expect(qualifiedTableName({ databaseType: "postgres", identifierQuote: '"', schema: "public", tableName: "Users" })).toBe('public."Users"'); + }); + + it("drops the schema qualifier for kingbase JDBC connections", () => { + expect(qualifiedTableName({ databaseType: "kingbase", identifierQuote: '"', schema: "app", tableName: "orders", includeDatabaseName: false })).toBe('"orders"'); + }); + + it("drops the schema qualifier for generic JDBC profiles that qualify schemas", () => { + expect(qualifiedTableName({ databaseType: "jdbc", driverProfile: "phoenix", identifierQuote: '"', schema: "MY_SCHEMA", tableName: "ORDER", includeDatabaseName: false })).toBe('"ORDER"'); + }); + + it("drops the schema qualifier for native Informix connections", () => { + expect(qualifiedTableName({ databaseType: "informix", identifierQuote: "", schema: "gbasedbt", tableName: "connection_smoke", includeDatabaseName: false })).toBe("connection_smoke"); + }); + + it("keeps the qualifier for databases that require fully qualified names", () => { + expect(qualifiedTableName({ databaseType: "sqlserver", schema: "dbo", tableName: "users", includeDatabaseName: false })).toBe("[dbo].[users]"); + }); +}); + describe("qualifiedTableName — GBase 8s", () => { it("omits the metadata owner for GBase 8s table data", () => { expect(qualifiedTableName({ databaseType: "informix", driverProfile: "gbase8s", identifierQuote: "", schema: "gbasedbt", tableName: "connection_smoke" })).toBe("connection_smoke"); diff --git a/apps/desktop/src/lib/table/tableSelectSql.ts b/apps/desktop/src/lib/table/tableSelectSql.ts index 6452eaec5a..ba2659c3e0 100644 --- a/apps/desktop/src/lib/table/tableSelectSql.ts +++ b/apps/desktop/src/lib/table/tableSelectSql.ts @@ -1,5 +1,6 @@ import type { DatabaseType } from "@/types/database.ts"; import { isSchemaAware, usesDatabaseObjectTreeMode } from "@/lib/database/databaseCapabilities.ts"; +import { DATABASE_SCHEMA_QUALIFIED_TYPES } from "@/lib/database/databaseCapabilitySets"; import { jdbcDriverProfileUsesSchemaQualification } from "@/lib/database/jdbcDialect"; import * as api from "@/lib/backend/api.ts"; import { parseSqlServerLinkedSchema, sqlServerLinkedTableName } from "@/lib/database/sqlServerLinkedServers.ts"; @@ -39,6 +40,14 @@ export interface BuildTableSelectSqlOptions { const DATABASE_QUALIFIED_TABLE_TYPES = new Set(["mysql", "clickhouse", "doris", "starrocks", "goldendb"]); +// `includeDatabaseName === false` drops the schema qualifier — the "database +// name" on schema-aware engines — except for databases that can only address +// objects through their full qualified name (`catalog.schema.table` / +// `database.schema.table`), where dropping it would break the query. +function dropsSchemaQualifier(databaseType: DatabaseType | undefined, includeDatabaseName?: boolean): boolean { + return includeDatabaseName === false && databaseType !== undefined && !DATABASE_SCHEMA_QUALIFIED_TYPES.has(databaseType); +} + function sqlStatementSpans(sql: string, dialectId: string): Array<{ start: number; end: number }> { const spans: Array<{ start: number; end: number }> = []; let start = 0; @@ -193,6 +202,7 @@ export function qualifiedTableName(options: Pick quoteTableIdentifierIfNeeded(databaseType, name)).join(".") : sqlServerLinkedTableName(linked, tableName); } } + // The schema qualifier is the "database name" on schema-aware engines + // (Oracle's SYSTEM, PG's public, ...). `dropsSchemaQualifier` keeps it + // for databases whose queries would not resolve without it. + if (dropsSchemaQualifier(databaseType, includeDatabaseName)) { + return quoteTable(tableName); + } return `${quoteTable(schema)}.${quoteTable(tableName)}`; } // MySQL-style engines use the selected database as their table namespace. diff --git a/packages/app-tests/tableSqlTemplates.test.ts b/packages/app-tests/tableSqlTemplates.test.ts index 856cdd54d6..f57d186e3d 100644 --- a/packages/app-tests/tableSqlTemplates.test.ts +++ b/packages/app-tests/tableSqlTemplates.test.ts @@ -13,11 +13,7 @@ function col(overrides: Partial & { name: string; data_type: string }; } -const columns: ColumnInfo[] = [ - col({ name: "id", data_type: "integer", is_primary_key: true, extra: "auto_increment" }), - col({ name: "name", data_type: "varchar" }), - col({ name: "created_at", data_type: "timestamp" }), -]; +const columns: ColumnInfo[] = [col({ name: "id", data_type: "integer", is_primary_key: true, extra: "auto_increment" }), col({ name: "name", data_type: "varchar" }), col({ name: "created_at", data_type: "timestamp" })]; test("builds SELECT template with explicit table columns", () => { assert.equal( @@ -87,10 +83,7 @@ test("builds GaussDB M templates with the detected backtick identifier mode", () identifierQuote: "`", schema: "app_schema", tableName: "order", - columns: [ - col({ name: "id", data_type: "integer", is_primary_key: true }), - col({ name: "DisplayName", data_type: "varchar" }), - ], + columns: [col({ name: "id", data_type: "integer", is_primary_key: true }), col({ name: "DisplayName", data_type: "varchar" })], }; assert.equal(buildTableSelectTemplate(options), "SELECT id, `DisplayName`\nFROM app_schema.`order`;"); @@ -114,3 +107,59 @@ test("preserves the Phoenix schema in all SQL templates", () => { assert.equal(buildTableUpdateTemplate(options), `UPDATE "APP"."USERS"\nSET "NAME" = 'NAME_value'\nWHERE "ID" = 0;`); assert.equal(buildTableDeleteTemplate(options), `DELETE FROM "APP"."USERS"\nWHERE "ID" = 0;`); }); + +test("respects includeDatabaseName=false for schema-aware engines (#9110)", () => { + // Oracle's schema qualifier is the "database name" on schema-aware engines; + // turning the setting off must drop it, not just on MySQL-style engines. + const sql = buildTableSelectTemplate({ + databaseType: "oracle", + identifierQuote: `"`, + schema: "SYSTEM", + includeDatabaseName: false, + quoteIdentifiers: false, + tableName: "AQ$_INTERNET_AGENTS", + columns: [{ name: "ID" }], + } as Parameters[0]); + assert.equal(sql, "SELECT ID\nFROM AQ$_INTERNET_AGENTS;"); +}); + +test("respects includeDatabaseName=false on quoted-identifier connections (#9110)", () => { + // The identifierQuote early-return paths (JDBC/agent connections) must + // honor the toggle too, not just the native schema-aware branch. + const base = { + databaseType: "postgres" as const, + identifierQuote: '"', + schema: "public", + tableName: "Users", + columns, + }; + assert.equal(buildTableSelectTemplate(base), 'SELECT id, name, created_at\nFROM public."Users";'); + assert.equal(buildTableSelectTemplate({ ...base, includeDatabaseName: false }), 'SELECT id, name, created_at\nFROM "Users";'); +}); + +test("keeps the schema qualifier for databases that require it and the default (both settings on)", () => { + // Default settings: schema stays. + const defaultSql = buildTableSelectTemplate({ + databaseType: "oracle", + identifierQuote: `"`, + schema: "SYSTEM", + includeDatabaseName: true, + quoteIdentifiers: true, + tableName: "T1", + columns: [{ name: "ID" }], + } as Parameters[0]); + assert.equal(defaultSql, 'SELECT "ID"\nFROM "SYSTEM"."T1";'); +}); + +test("quoteIdentifiers=false emits bare column names in SELECT templates", () => { + const sql = buildTableSelectTemplate({ + databaseType: "mysql", + schema: undefined, + database: "shopdb", + includeDatabaseName: true, + quoteIdentifiers: false, + tableName: "orders", + columns: [{ name: "id" }, { name: "amount" }], + } as Parameters[0]); + assert.equal(sql, "SELECT id, amount\nFROM shopdb.orders;"); +});