fix(completion): keep auto alias and schema-qualify SQL Server FK JOIN candidates
This commit is contained in:
parent
a4129c02fa
commit
cdfa993642
|
|
@ -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<string, Set<string>> {
|
||||
const schemasByTableName = new Map<string, Set<string>>();
|
||||
for (const table of tables) {
|
||||
const tableName = normalizeIdentifierPart(table.name);
|
||||
const schemas = schemasByTableName.get(tableName) ?? new Set<string>();
|
||||
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<string, Set<string>>,
|
||||
): { 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<SqlCompletionContext, "prefix" | "qualifier">,
|
||||
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<string, Set<string>>();
|
||||
for (const table of matchingTables) {
|
||||
const tableName = normalizeIdentifierPart(table.name);
|
||||
const schemas = schemasByTableName.get(tableName) ?? new Set<string>();
|
||||
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<string, SqlCompletionForeignKey[]>, dialect?: "mysql" | "postgres" | "sqlserver"): SqlCompletionItem[] {
|
||||
function buildForeignKeyRelatedTableItems(
|
||||
context: SqlCompletionContext,
|
||||
tables: SqlCompletionTable[],
|
||||
foreignKeysByTable?: Map<string, SqlCompletionForeignKey[]>,
|
||||
dialect?: "mysql" | "postgres" | "sqlserver",
|
||||
autoAliasTables = false,
|
||||
databaseType?: DatabaseType,
|
||||
keywordCase?: SqlKeywordCase,
|
||||
currentSchema?: string,
|
||||
): SqlCompletionItem[] {
|
||||
if (!foreignKeysByTable || context.referencedTables.length === 0) return [];
|
||||
const candidates = new Map<string, { table: SqlCompletionTable; detail: string }>();
|
||||
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);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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<string, SqlCompletionForeignKey[]>([
|
||||
["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<string, SqlCompletionForeignKey[]>([
|
||||
["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<string, SqlCompletionForeignKey[]>([
|
||||
["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<string, SqlCompletionForeignKey[]>([["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", () => {
|
||||
|
|
|
|||
Loading…
Reference in New Issue