diff --git a/apps/desktop/src/lib/sql/sqlCompletion.ts b/apps/desktop/src/lib/sql/sqlCompletion.ts index 4b0bb0bec..c3b830bb3 100644 --- a/apps/desktop/src/lib/sql/sqlCompletion.ts +++ b/apps/desktop/src/lib/sql/sqlCompletion.ts @@ -1421,7 +1421,7 @@ class SqlCompletionProvider { } if (!context.exclusiveColumnSuggestions && context.suggestTables) { - this.items.push(...buildForeignKeyRelatedTableItems(context, this.input.tables, this.input.foreignKeysByTable, this.dialect)); + this.items.push(...buildForeignKeyRelatedTableItems(context, this.input.tables, this.input.foreignKeysByTable, this.dialect, !!this.input.autoAliasTables && context.autoAliasTableCompletions, this.databaseType, this.input.keywordCase, this.input.currentSchema)); this.items.push(...buildTableItems(context, this.input.tables, this.dialect, !!this.input.autoAliasTables && context.autoAliasTableCompletions, context.referencedTables, this.databaseType, this.input.currentSchema, this.input.keywordCase)); if (this.databaseType === "clickhouse") { this.items.push(...buildClickHouseFunctionItems(context.prefix, context.openingParenAfterCursor, "table")); @@ -2872,6 +2872,49 @@ function quoteSelectStarColumnIdentifier(identifier: string, dialect?: "mysql" | return quoteSqlIdentifier(identifier, dialect); } +/** + * Build a normalized table-name -> set-of-schemas index used to detect when a + * bare table name is ambiguous across schemas. Shared by buildTableItems and + * buildForeignKeyRelatedTableItems so both apply the same ambiguity signal. + */ +function collectSchemasByTableName(tables: SqlCompletionTable[]): Map> { + const schemasByTableName = new Map>(); + for (const table of tables) { + const tableName = normalizeIdentifierPart(table.name); + const schemas = schemasByTableName.get(tableName) ?? new Set(); + schemas.add(normalizeIdentifierPart(table.schema ?? "")); + schemasByTableName.set(tableName, schemas); + } + return schemasByTableName; +} + +/** + * Resolve the schema-qualification signals for a completion table. + * + * Shared by buildTableItems and buildForeignKeyRelatedTableItems so foreign-key + * related candidates stay consistent with regular table candidates: when the + * same table name exists in multiple schemas, both qualify the apply text with + * `schema.table`. Otherwise an FK candidate would insert a bare `customers AS cs` + * that may reference the wrong schema and carry a different dedupeKey than the + * regular candidate, producing a duplicate. Oracle keeps its current-schema + * behavior; the generic/PostgreSQL/SQL Server paths qualify on ambiguity. + */ +function resolveTableSchemaQualification( + table: SqlCompletionTable, + dialect: "mysql" | "postgres" | "sqlserver" | undefined, + databaseType: DatabaseType | undefined, + currentSchema: string | undefined, + schemasByTableName: Map>, +): { ambiguousTableName: boolean; schemaQualification: boolean; defaultApplyName: string } { + const oracleSchemaQualification = databaseType === "oracle" && table.schema && table.schema.toUpperCase() !== "PUBLIC" && (!currentSchema || normalizeIdentifierPart(table.schema) !== normalizeIdentifierPart(currentSchema)); + // A bare table name is ambiguous when metadata contains the same name in multiple schemas. + // Keep Oracle's current-schema behavior, but qualify the generic/PostgreSQL/SQL Server paths. + const ambiguousTableName = databaseType !== "oracle" && (schemasByTableName.get(normalizeIdentifierPart(table.name))?.size ?? 0) > 1; + const schemaQualification = !!table.schema && (oracleSchemaQualification || ambiguousTableName); + const defaultApplyName = schemaQualification ? `${quoteSqlIdentifier(table.schema!, dialect)}.${quoteSqlIdentifier(table.name, dialect)}` : quoteSqlIdentifier(table.name, dialect); + return { ambiguousTableName, schemaQualification, defaultApplyName }; +} + function buildTableItems( context: Pick, tables: SqlCompletionTable[], @@ -2886,22 +2929,12 @@ function buildTableItems( const qualifierSchema = context.qualifier?.split(".").filter(Boolean).pop(); const existingAliases = new Set(referencedTables.map((ref) => ref.alias?.toLowerCase()).filter((alias): alias is string => !!alias)); const matchingTables = tables.filter((table) => matchesPrefix(table.name, prefix)); - const schemasByTableName = new Map>(); - for (const table of matchingTables) { - const tableName = normalizeIdentifierPart(table.name); - const schemas = schemasByTableName.get(tableName) ?? new Set(); - schemas.add(normalizeIdentifierPart(table.schema ?? "")); - schemasByTableName.set(tableName, schemas); - } + // Ambiguity is decided among prefix-matching tables only, matching prior behavior. + const schemasByTableName = collectSchemasByTableName(matchingTables); return matchingTables .map((table) => { const qualifiedByContext = !!qualifierSchema && !!table.schema && normalizeIdentifierPart(qualifierSchema) === normalizeIdentifierPart(table.schema); - const oracleSchemaQualification = databaseType === "oracle" && table.schema && table.schema.toUpperCase() !== "PUBLIC" && (!currentSchema || normalizeIdentifierPart(table.schema) !== normalizeIdentifierPart(currentSchema)); - // A bare table name is ambiguous when metadata contains the same name in multiple schemas. - // Keep Oracle's current-schema behavior, but qualify the generic/PostgreSQL/SQL Server paths. - const ambiguousTableName = databaseType !== "oracle" && (schemasByTableName.get(normalizeIdentifierPart(table.name))?.size ?? 0) > 1; - const schemaQualification = !!table.schema && (oracleSchemaQualification || ambiguousTableName); - const defaultApplyName = schemaQualification ? `${quoteSqlIdentifier(table.schema!, dialect)}.${quoteSqlIdentifier(table.name, dialect)}` : quoteSqlIdentifier(table.name, dialect); + const { ambiguousTableName, defaultApplyName } = resolveTableSchemaQualification(table, dialect, databaseType, currentSchema, schemasByTableName); const suppliedApplyName = table.applyName?.trim(); const suppliedApplyNameIsQualified = suppliedApplyName?.includes(".") === true; const applyName = qualifiedByContext ? quoteSqlIdentifier(table.name, dialect) : ambiguousTableName && !!table.schema && (!suppliedApplyName || !suppliedApplyNameIsQualified) ? defaultApplyName : (suppliedApplyName ?? defaultApplyName); @@ -2919,9 +2952,19 @@ function buildTableItems( .slice(0, MAX_TABLE_COMPLETION_ITEMS); } -function buildForeignKeyRelatedTableItems(context: SqlCompletionContext, tables: SqlCompletionTable[], foreignKeysByTable?: Map, dialect?: "mysql" | "postgres" | "sqlserver"): SqlCompletionItem[] { +function buildForeignKeyRelatedTableItems( + context: SqlCompletionContext, + tables: SqlCompletionTable[], + foreignKeysByTable?: Map, + dialect?: "mysql" | "postgres" | "sqlserver", + autoAliasTables = false, + databaseType?: DatabaseType, + keywordCase?: SqlKeywordCase, + currentSchema?: string, +): SqlCompletionItem[] { if (!foreignKeysByTable || context.referencedTables.length === 0) return []; const candidates = new Map(); + const existingAliases = new Set(context.referencedTables.map((ref) => ref.alias?.toLowerCase()).filter((alias): alias is string => !!alias)); for (const ref of context.referencedTables) { for (const [ownerKey, foreignKeys] of foreignKeysByTable.entries()) { const owner = foreignKeyOwnerFromKey(ownerKey); @@ -2941,14 +2984,27 @@ function buildForeignKeyRelatedTableItems(context: SqlCompletionContext, tables: } } + // Reuse buildTableItems' ambiguity signal so FK candidates qualify with + // `schema.table` exactly when regular candidates would. Built from the same + // prefix-matching table set buildTableItems uses, keeping the two in lockstep. + const schemasByTableName = collectSchemasByTableName(tables.filter((table) => matchesPrefix(table.name, context.prefix))); + return [...candidates.values()] - .map(({ table, detail }) => ({ - label: table.name, - type: "table" as const, - detail, - apply: quoteSqlIdentifier(table.name, dialect), - boost: computeBoost(table.name, context.prefix) + 3600, - })) + .map(({ table, detail }) => { + const { ambiguousTableName, defaultApplyName } = resolveTableSchemaQualification(table, dialect, databaseType, currentSchema, schemasByTableName); + const applyName = defaultApplyName; + const alias = autoAliasTables ? generateTableCompletionAlias(table.name, existingAliases) : ""; + return { + label: table.name, + type: "table" as const, + detail, + apply: formatTableAliasApply(applyName, alias, databaseType, keywordCase), + boost: computeBoost(table.name, context.prefix) + 3600, + // Mirror buildTableItems' dedupeKey so an FK candidate and the regular + // candidate for the same schema-qualified table collapse to one entry. + dedupeKey: ambiguousTableName || (databaseType === "oracle" && table.schema) ? applyName : undefined, + }; + }) .sort(compareCompletionItems); } diff --git a/packages/app-tests/sqlCompletion.test.ts b/packages/app-tests/sqlCompletion.test.ts index 50265a2b7..4f9b3f856 100644 --- a/packages/app-tests/sqlCompletion.test.ts +++ b/packages/app-tests/sqlCompletion.test.ts @@ -3086,6 +3086,82 @@ test("boosts foreign-key related table candidates in JOIN table context", () => assert.ok(items[0]?.detail?.includes("related by")); }); +test("keeps automatic SQL Server aliases on foreign-key related JOIN candidates", () => { + const foreignKeysByTable = new Map([ + ["dbo.orders", [{ name: "orders_customer_id_fkey", column: "customer_id", ref_schema: "dbo", ref_table: "customers", ref_column: "id" }]], + ]); + const sql = "select * from dbo.orders o join cus"; + const items = buildSqlCompletionItems(sql, sql.length, { + tables: [ + { name: "orders", schema: "dbo", type: "table" }, + { name: "customers", schema: "dbo", type: "table" }, + ], + columnsByTable, + foreignKeysByTable, + dialect: "sqlserver", + databaseType: "sqlserver", + autoAliasTables: true, + }); + + assert.equal(items[0]?.label, "customers"); + assert.ok(items[0]?.detail?.includes("related by")); + assert.equal(items[0]?.apply, "customers AS cs"); +}); + +test("does not add aliases to foreign-key related JOIN candidates when disabled", () => { + const foreignKeysByTable = new Map([ + ["dbo.orders", [{ name: "orders_customer_id_fkey", column: "customer_id", ref_schema: "dbo", ref_table: "customers", ref_column: "id" }]], + ]); + const sql = "select * from dbo.orders o join cus"; + const items = buildSqlCompletionItems(sql, sql.length, { + tables: [ + { name: "orders", schema: "dbo", type: "table" }, + { name: "customers", schema: "dbo", type: "table" }, + ], + columnsByTable, + foreignKeysByTable, + dialect: "sqlserver", + databaseType: "sqlserver", + autoAliasTables: false, + }); + + assert.equal(items[0]?.label, "customers"); + assert.ok(items[0]?.detail?.includes("related by")); + assert.equal(items[0]?.apply, "customers"); +}); + +test("schema-qualifies foreign-key related JOIN candidates when the target table name spans schemas", () => { + const foreignKeysByTable = new Map([ + ["dbo.orders", [{ name: "orders_customer_id_fkey", column: "customer_id", ref_schema: "sales", ref_table: "customers", ref_column: "id" }]], + ]); + const sql = "select * from dbo.orders o join cus"; + const items = buildSqlCompletionItems(sql, sql.length, { + tables: [ + { name: "orders", schema: "dbo", type: "table" }, + { name: "customers", schema: "dbo", type: "table" }, + { name: "customers", schema: "sales", type: "table" }, + ], + columnsByTable, + foreignKeysByTable, + dialect: "sqlserver", + databaseType: "sqlserver", + autoAliasTables: true, + }); + + const fkCandidate = items.find((item) => item.type === "table" && item.detail?.includes("related by")); + assert.ok(fkCandidate, "should surface the foreign-key related candidate"); + // customers exists in both dbo and sales, so the FK candidate must qualify with + // the referenced schema (sales.customers) instead of a bare, ambiguous customers. + assert.equal(fkCandidate?.apply, "sales.customers AS cs"); + assert.equal(fkCandidate?.dedupeKey, "sales.customers"); + // The FK candidate (higher boost) should win dedupe against the regular + // sales.customers candidate, leaving no bare `customers AS cs` entry. + assert.ok( + !items.some((item) => item.type === "table" && item.apply === "customers AS cs"), + "should not emit a bare unqualified customers candidate alongside the qualified FK candidate", + ); +}); + test("boosts inbound foreign-key table candidates in JOIN table context", () => { const foreignKeysByTable = new Map([["public.orders", [{ name: "orders_customer_id_fkey", column: "customer_id", ref_schema: "public", ref_table: "customers", ref_column: "id" }]]]); const sql = "select * from public.customers c join ord"; @@ -3115,7 +3191,9 @@ test("uses owner schema when ranking inbound foreign-key table candidates", () = assert.equal(items[0]?.label, "orders"); assert.equal(items[0]?.detail, "related by sales.orders.customer_id → id"); - assert.equal(items[0]?.apply, "orders"); + // orders exists in both public and sales, so the FK candidate must schema-qualify + // with the owner schema (sales.orders) to match buildTableItems' qualification. + assert.equal(items[0]?.apply, "sales.orders"); }); test("suggests composite explicit foreign-key join conditions", () => {