diff --git a/apps/desktop/src/lib/__tests__/editor/codemirrorInsertValueHints.spec.ts b/apps/desktop/src/lib/__tests__/editor/codemirrorInsertValueHints.spec.ts index 64a714cda7..7c42489b5a 100644 --- a/apps/desktop/src/lib/__tests__/editor/codemirrorInsertValueHints.spec.ts +++ b/apps/desktop/src/lib/__tests__/editor/codemirrorInsertValueHints.spec.ts @@ -1,6 +1,7 @@ import { strict as assert } from "node:assert"; import { test } from "vitest"; import { buildInsertValueHintDecorations, insertValueHintsRefreshDispatchSpec, isInsertValueHintDecorationAt } from "@/lib/editor/codemirrorInsertValueHints"; +import { parseInsertValueHints } from "@/lib/sql/insertValueHints"; test("insertValueHintsRefreshDispatchSpec disables scrollIntoView", () => { assert.equal(insertValueHintsRefreshDispatchSpec.scrollIntoView, false); @@ -18,3 +19,10 @@ test("isInsertValueHintDecorationAt detects widget anchors", () => { assert.equal(isInsertValueHintDecorationAt(decorations, valueTwo), true); assert.equal(isInsertValueHintDecorationAt(decorations, sql.indexOf("INSERT")), false); }); + +test("a projection that already aliases its target column renders no hint widget", () => { + const sql = ["INSERT INTO t (operation_apply_id, pacu_status)", "SELECT", " noprid.ruid as operation_apply_id,", " noprid.flag", "FROM noprid"].join("\n"); + const decorations = buildInsertValueHintDecorations(parseInsertValueHints(sql)); + assert.equal(isInsertValueHintDecorationAt(decorations, sql.indexOf("noprid.ruid")), false); + assert.equal(isInsertValueHintDecorationAt(decorations, sql.indexOf("noprid.flag")), true); +}); diff --git a/apps/desktop/src/lib/editor/codemirrorInsertValueHints.ts b/apps/desktop/src/lib/editor/codemirrorInsertValueHints.ts index 2318d4739b..2b9e097ae5 100644 --- a/apps/desktop/src/lib/editor/codemirrorInsertValueHints.ts +++ b/apps/desktop/src/lib/editor/codemirrorInsertValueHints.ts @@ -99,7 +99,7 @@ function shiftClause(clause: InsertValuesClause, offset: number): InsertValuesCl return { ...clause, span: { start: clause.span.start + offset, end: clause.span.end + offset }, - rows: clause.rows.map((row) => row.map((from) => from + offset)), + rows: clause.rows.map((row) => row.map((source) => ({ ...source, from: source.from + offset }))), }; } diff --git a/apps/desktop/src/lib/sql/insertValueHints.ts b/apps/desktop/src/lib/sql/insertValueHints.ts index 03d0393b67..0549de7a75 100644 --- a/apps/desktop/src/lib/sql/insertValueHints.ts +++ b/apps/desktop/src/lib/sql/insertValueHints.ts @@ -14,11 +14,25 @@ export interface InsertValuesClause { database?: string; /** Explicit column list, or null when `INSERT INTO t VALUES` has no column list. */ columns: string[] | null; - /** For each VALUES row or SELECT projection list, start offsets of top-level expressions. */ - rows: number[][]; + /** For each VALUES row or SELECT projection list, the top-level source expressions. */ + rows: InsertValueSource[][]; span: SqlSemanticSpan; } +/** One source expression of an INSERT, positioned for its inlay hint. */ +export interface InsertValueSource { + /** Document offset where the inlay widget is inserted (before the value expression). */ + from: number; + /** + * Explicit alias of a SELECT projection item (`expr AS alias`, `expr alias`), unquoted. + * + * The positional hint maps a source expression to its target column; when the projection + * already names that same column itself, the hint would only duplicate the alias the user + * typed, so `buildInsertValueHints` drops it. + */ + alias?: string; +} + export interface ParseInsertValueHintsOptions { /** Resolve table columns when the INSERT has no explicit column list. */ resolveTableColumns?: (table: string, schema?: string, database?: string) => string[] | undefined; @@ -115,11 +129,11 @@ function parseColumnList(tokens: readonly SqlSemanticToken[], openIndex: number) return { columns, nextIndex: index }; } -function valueStartsInRow(tokens: readonly SqlSemanticToken[], openIndex: number): { starts: number[]; nextIndex: number } | null { +function valueStartsInRow(tokens: readonly SqlSemanticToken[], openIndex: number): { starts: InsertValueSource[]; nextIndex: number } | null { const open = tokens[openIndex]; if (!open || open.text !== "(") return null; const contentDepth = open.depth + 1; - const starts: number[] = []; + const starts: InsertValueSource[] = []; let expectValue = true; let index = openIndex + 1; @@ -130,7 +144,7 @@ function valueStartsInRow(tokens: readonly SqlSemanticToken[], openIndex: number return { starts, nextIndex: index + 1 }; } if (expectValue && item.depth === contentDepth) { - starts.push(item.span.start); + starts.push({ from: item.span.start }); expectValue = false; } if (item.text === "," && item.depth === contentDepth) { @@ -141,8 +155,8 @@ function valueStartsInRow(tokens: readonly SqlSemanticToken[], openIndex: number return { starts, nextIndex: index }; } -function parseValuesRows(tokens: readonly SqlSemanticToken[], valuesIndex: number): number[][] { - const rows: number[][] = []; +function parseValuesRows(tokens: readonly SqlSemanticToken[], valuesIndex: number): InsertValueSource[][] { + const rows: InsertValueSource[][] = []; let index = valuesIndex + 1; while (index < tokens.length) { const item = tokens[index]; @@ -205,7 +219,7 @@ function isWildcardProjection(tokens: readonly SqlSemanticToken[], from: number, return topLevel.length % 2 === 1 && topLevel.every((item, index) => (index % 2 === 0 ? (index === topLevel.length - 1 ? item.text === "*" : tokenIsIdentifier(item)) : item.text === ".")); } -function selectProjectionStarts(tokens: readonly SqlSemanticToken[], selectIndex: number): number[] { +function selectProjectionStarts(tokens: readonly SqlSemanticToken[], selectIndex: number): InsertValueSource[] { const select = tokens[selectIndex]; if (!select) return []; const depth = select.depth; @@ -239,7 +253,7 @@ function selectProjectionStarts(tokens: readonly SqlSemanticToken[], selectIndex if (tokens[index]?.depth === depth && tokens[index]?.normalized === "with" && tokens[index + 1]?.depth === depth && tokens[index + 1]?.normalized === "ties") index += 2; } - const starts: number[] = []; + const starts: InsertValueSource[] = []; let expressionStart = index; for (; index < tokens.length; index += 1) { const item = tokens[index]; @@ -247,15 +261,37 @@ function selectProjectionStarts(tokens: readonly SqlSemanticToken[], selectIndex if (isSelectClauseBoundary(item, depth)) break; if (item.text !== "," || item.depth !== depth) continue; if (index <= expressionStart || isWildcardProjection(tokens, expressionStart, index, depth)) return []; - starts.push(tokens[expressionStart]!.span.start); + starts.push(selectProjectionSource(tokens, expressionStart, index, depth)); expressionStart = index + 1; } if (index <= expressionStart || isWildcardProjection(tokens, expressionStart, index, depth)) return []; - starts.push(tokens[expressionStart]!.span.start); + starts.push(selectProjectionSource(tokens, expressionStart, index, depth)); return starts; } +/** + * Projection source expression start, plus the alias that ends the projection when it has one. + * Only the *trailing name* of a projection is an alias, so the token in front of it decides: + * `AS` or another identifier (`x total`, `f(x) total`) introduces an alias, while a dotted name + * (`t.col`), an operator (`x::text`) or an open expression does not. + */ +function selectProjectionSource(tokens: readonly SqlSemanticToken[], from: number, to: number, depth: number): InsertValueSource { + const source: InsertValueSource = { from: tokens[from]!.span.start }; + const alias = selectProjectionAlias(tokens, to, depth); + if (alias !== undefined) source.alias = alias; + return source; +} + +function selectProjectionAlias(tokens: readonly SqlSemanticToken[], to: number, depth: number): string | undefined { + const last = tokens[to - 1]; + if (!tokenIsIdentifier(last) || last.depth !== depth) return undefined; + const previous = tokens[to - 2]; + if (!previous) return undefined; + const introducesAlias = previous.kind === "word" || previous.kind === "quoted_identifier" || previous.text === ")"; + return introducesAlias ? unquoteSqlSemanticIdentifier(last) : undefined; +} + function parseInsertClause(tokens: readonly SqlSemanticToken[], span: SqlSemanticSpan): InsertValuesClause | null { const insertIndex = findWordIndex(tokens, "insert"); if (insertIndex < 0) return null; @@ -316,7 +352,7 @@ function shiftClause(clause: InsertValuesClause, offset: number): InsertValuesCl return { ...clause, span: { start: clause.span.start + offset, end: clause.span.end + offset }, - rows: clause.rows.map((row) => row.map((from) => from + offset)), + rows: clause.rows.map((row) => row.map((source) => ({ ...source, from: source.from + offset }))), }; } @@ -671,16 +707,23 @@ export function buildInsertValueHints(clauses: readonly InsertValuesClause[], op for (const row of clause.rows) { const count = Math.min(row.length, columns.length); for (let index = 0; index < count; index += 1) { - const from = row[index]; + const source = row[index]; const column = columns[index]; - if (from === undefined || !column) continue; - hints.push({ from, column }); + if (source === undefined || !column) continue; + // A projection that already ends with the target column name states the mapping by + // itself; repeating it as an inlay pill only duplicates the alias the user typed. + if (source.alias !== undefined && sameSqlIdentifier(source.alias, column)) continue; + hints.push({ from: source.from, column }); } } } return hints; } +function sameSqlIdentifier(left: string, right: string): boolean { + return left.toLowerCase() === right.toLowerCase(); +} + /** Parse SQL and return insert-value inlay hints. */ export function parseInsertValueHints(sql: string, options: ParseInsertValueHintsOptions = {}): InsertValueHint[] { return buildInsertValueHints(parseInsertValuesClauses(sql), options); diff --git a/docs/screenshots/issue-9277-after.png b/docs/screenshots/issue-9277-after.png new file mode 100644 index 0000000000..bc5f20c5c2 Binary files /dev/null and b/docs/screenshots/issue-9277-after.png differ diff --git a/docs/screenshots/issue-9277-before.png b/docs/screenshots/issue-9277-before.png new file mode 100644 index 0000000000..de24784409 Binary files /dev/null and b/docs/screenshots/issue-9277-before.png differ diff --git a/packages/app-tests/insertValueHints.test.ts b/packages/app-tests/insertValueHints.test.ts index 77a646271e..0bbe5b6f24 100644 --- a/packages/app-tests/insertValueHints.test.ts +++ b/packages/app-tests/insertValueHints.test.ts @@ -233,6 +233,52 @@ test("returns no positional hints for wildcard INSERT ... SELECT projections", ( } }); +test("skips the hint when the SELECT projection already aliases its target column", () => { + // Reported as duplicated aliases that cannot be selected or deleted: the inlay pill repeated + // the `AS ` name the statement already spells out. + const sql = ["INSERT INTO t_demo (operation_apply_id, pacu_enter_time, pacu_status)", "SELECT", " noprid.ruid as operation_apply_id,", " noprid.enter_date as pacu_enter_time,", " noprid.flag as unrelated_alias", "FROM noprid"].join("\n"); + const hints = parseInsertValueHints(sql); + assert.deepEqual( + hints.map((hint) => ({ column: hint.column, text: sql.slice(hint.from).split(/[ ,]/u, 1)[0] })), + [{ column: "pacu_status", text: "noprid.flag" }], + ); +}); + +test("keeps positional mapping when only some projections repeat their target column", () => { + const sql = "INSERT INTO t (a, b, c) SELECT x AS a, y, z AS c FROM s"; + const hints = parseInsertValueHints(sql); + assert.deepEqual( + hints.map((hint) => ({ column: hint.column, text: sql.slice(hint.from).split(/[ ,]/u, 1)[0] })), + [{ column: "b", text: "y" }], + ); +}); + +test("matches projection aliases to target columns regardless of case or quoting", () => { + assert.deepEqual(parseInsertValueHints("INSERT INTO t (operation_apply_id) SELECT noprid.ruid AS Operation_Apply_ID FROM noprid"), []); + assert.deepEqual(parseInsertValueHints("INSERT INTO t (`pacu_status`) SELECT noprid.flag AS `pacu_status` FROM noprid"), []); + assert.deepEqual(parseInsertValueHints('INSERT INTO t (total) SELECT npl.additive AS "TOTAL" FROM npl'), []); +}); + +test("recognises implicit projection aliases without the AS keyword", () => { + assert.deepEqual(parseInsertValueHints("INSERT INTO t (cnt, name) SELECT COUNT(*) cnt, src.name name FROM s"), []); + assert.deepEqual( + parseInsertValueHints("INSERT INTO t (name) SELECT src.name FROM s").map((hint) => hint.column), + ["name"], + ); + assert.deepEqual( + parseInsertValueHints("INSERT INTO t (cnt) SELECT COUNT(*) FROM s").map((hint) => hint.column), + ["cnt"], + ); +}); + +test("keeps the hint for a projection whose alias names a different column", () => { + const sql = "INSERT INTO dbo.users (id, name) SELECT source_id AS old_id, source_name AS display_name FROM staging"; + assert.deepEqual( + parseInsertValueHints(sql).map((hint) => hint.column), + ["id", "name"], + ); +}); + test("caps INSERT ... SELECT hints to the smaller target or projection count", () => { assert.deepEqual( parseInsertValueHints("INSERT INTO t (a, b) SELECT x, y, z FROM source").map((hint) => hint.column),