diff --git a/src/builder/eb.ts b/src/builder/eb.ts index 97a532f..e5329c6 100644 --- a/src/builder/eb.ts +++ b/src/builder/eb.ts @@ -454,6 +454,13 @@ export function max(expr: Expression): Expression { /** COALESCE(a, b, c, ...) — returns first non-null value */ export function coalesce(...args: Expression[]): Expression { + if (args.length === 0) { + // `COALESCE()` is invalid on every dialect (PG, MySQL, SQLite, + // MSSQL all reject zero-arg COALESCE). Catch at build time — the + // runtime driver error ("COALESCE requires at least one argument") + // doesn't point at the caller. + throw new Error("coalesce() requires at least one argument") + } return wrap( rawFn( "COALESCE", diff --git a/src/printer/mssql.ts b/src/printer/mssql.ts index 32f2639..ebcbd5d 100644 --- a/src/printer/mssql.ts +++ b/src/printer/mssql.ts @@ -361,8 +361,11 @@ export class MssqlPrinter extends BasePrinter { /** * Render a RETURNING list as MSSQL `OUTPUT` columns under the given * pseudo-table (`INSERTED` or `DELETED`). Handles `StarNode` bare and - * table-qualified — previously a `printed === "*"` string check missed - * the `"t".*` form and emitted invalid `OUTPUT INSERTED."t".*`. + * table-qualified — the pseudo-tables are fixed names, so a user's + * `returning(star("orders"))` (meaning "all columns of orders") maps + * to `INSERTED.*` (the pseudo-table has every column of the target). + * Emitting `INSERTED.[orders].*` produces an invalid three-part name + * that SQL Server rejects at parse. */ private _outputCols( returning: readonly import("../ast/nodes.ts").ExpressionNode[], @@ -371,7 +374,9 @@ export class MssqlPrinter extends BasePrinter { return returning .map((r) => { if (r.type === "star") { - return r.table ? `${prefix}.${quoteIdentifier(r.table, this.dialect)}.*` : `${prefix}.*` + // Drop any user-supplied table qualifier: OUTPUT targets the + // INSERTED/DELETED pseudo-table, never the base table directly. + return `${prefix}.*` } return `${prefix}.${this.printExpression(r)}` }) diff --git a/src/printer/mysql.ts b/src/printer/mysql.ts index 53dc0ef..7a2754d 100644 --- a/src/printer/mysql.ts +++ b/src/printer/mysql.ts @@ -13,6 +13,7 @@ import type { } from "../ast/nodes.ts" import { UnsupportedDialectFeatureError } from "../errors.ts" import { quoteIdentifier } from "../utils/identifier.ts" +import { escapeStringLiteral } from "../utils/security.ts" import { BasePrinter } from "./base.ts" export class MysqlPrinter extends BasePrinter { @@ -197,9 +198,21 @@ export class MysqlPrinter extends BasePrinter { } /** - * MySQL supports `->` / `->>` (single key) but has no path operators - * `#>` / `#>>`; those are PG-specific. Reject the path variants with - * a pointer at the JSON_EXTRACT equivalent. + * MySQL `->` / `->>` require the RHS to be a JSONPath starting with + * `$` (e.g. `data->'$.name'`, `data->'$[0]'`). The base printer emits + * PG's bare-key form (`data->'name'`), which MySQL rejects with + * `ER_INVALID_JSON_PATH`. Rewrite the path literal here. + * + * Note on cross-dialect semantics: if a caller passes `at("a.b")` the + * MySQL form becomes `$.a.b` — a two-level JSONPath. On PG the same + * node emits `data->'a.b'` — a literal single key. Sumak treats + * `.at(path)` as "the user-supplied JSON selector string"; dotted + * keys are rare and chaining (`.at("a").at("b")`) is the + * recommended portable form. Use `#>` with a segment array for + * path traversal (PG only). + * + * `#>` / `#>>` path operators are PG-specific — reject with a pointer + * at JSON_EXTRACT. */ protected override printJsonAccess(node: import("../ast/nodes.ts").JsonAccessNode): string { if (node.operator === "#>" || node.operator === "#>>") { @@ -208,7 +221,13 @@ export class MysqlPrinter extends BasePrinter { `${node.operator} JSON path operator — use JSON_EXTRACT(expr, '$.a.b') or chained '->' on MySQL`, ) } - return super.printJsonAccess(node) + // Rewrite single-segment path to JSONPath. `at("0")` → `$[0]`, + // `at("name")` → `$.name`. Escape embedded quotes with the usual + // single-quote doubling. + const seg = /^\d+$/.test(node.path) ? `[${node.path}]` : `.${node.path}` + const pathLiteral = `'$${escapeStringLiteral(seg)}'` + const result = `${this.printExpression(node.expr)}${node.operator}${pathLiteral}` + return node.alias ? `${result} AS ${quoteIdentifier(node.alias, this.dialect)}` : result } /** MySQL does not support `DELETE … RETURNING` — PG / SQLite 3.35+ only. */ diff --git a/src/printer/sqlite.ts b/src/printer/sqlite.ts index 78df3d7..7fc1d78 100644 --- a/src/printer/sqlite.ts +++ b/src/printer/sqlite.ts @@ -10,6 +10,7 @@ import type { } from "../ast/nodes.ts" import { UnsupportedDialectFeatureError } from "../errors.ts" import { quoteIdentifier } from "../utils/identifier.ts" +import { escapeStringLiteral } from "../utils/security.ts" import { BasePrinter } from "./base.ts" export class SqlitePrinter extends BasePrinter { @@ -94,9 +95,10 @@ export class SqlitePrinter extends BasePrinter { } /** - * SQLite supports `->` / `->>` (3.38+) but has no path operators - * `#>` / `#>>`; those are PG-specific. Reject with a pointer at - * json_extract / chained `->`. + * SQLite `->` / `->>` (3.38+) require the RHS to be a JSONPath + * starting with `$` (`data->'$.name'`, `data->'$[0]'`). The base + * printer emits PG's bare-key form, which SQLite rejects. Rewrite + * the path literal here. `#>` / `#>>` are PG-specific — reject. */ protected override printJsonAccess(node: import("../ast/nodes.ts").JsonAccessNode): string { if (node.operator === "#>" || node.operator === "#>>") { @@ -105,7 +107,10 @@ export class SqlitePrinter extends BasePrinter { `${node.operator} JSON path operator — use json_extract(expr, '$.a.b') or chained '->' on SQLite`, ) } - return super.printJsonAccess(node) + const seg = /^\d+$/.test(node.path) ? `[${node.path}]` : `.${node.path}` + const pathLiteral = `'$${escapeStringLiteral(seg)}'` + const result = `${this.printExpression(node.expr)}${node.operator}${pathLiteral}` + return node.alias ? `${result} AS ${quoteIdentifier(node.alias, this.dialect)}` : result } /** diff --git a/test/audit24-regressions.test.ts b/test/audit24-regressions.test.ts new file mode 100644 index 0000000..8f4d633 --- /dev/null +++ b/test/audit24-regressions.test.ts @@ -0,0 +1,88 @@ +import { describe, expect, it } from "vitest" + +import type { InsertNode, JsonAccessNode, SelectNode } from "../src/ast/nodes.ts" +import { createInsertNode, createSelectNode } from "../src/ast/nodes.ts" +import { coalesce } from "../src/builder/eb.ts" +import { MssqlPrinter } from "../src/printer/mssql.ts" +import { MysqlPrinter } from "../src/printer/mysql.ts" +import { SqlitePrinter } from "../src/printer/sqlite.ts" + +const selectJsonAccess = (op: "->" | "->>", path: string): SelectNode => ({ + ...createSelectNode(), + columns: [ + { + type: "json_access", + expr: { type: "column_ref", column: "data" }, + operator: op, + path, + } as JsonAccessNode, + ], + from: { type: "table_ref", name: "t" }, +}) + +describe("Audit #24 regressions", () => { + describe("coalesce() requires at least one argument", () => { + it("coalesce() with zero args throws at builder time", () => { + expect(() => coalesce()).toThrow(/requires at least one argument/) + }) + + it("coalesce(x) with one arg still works", () => { + const r = coalesce({ node: { type: "literal", value: 1 } } as any) + expect((r as any).node).toEqual({ + type: "function_call", + name: "COALESCE", + args: [{ type: "literal", value: 1 }], + }) + }) + }) + + describe("MSSQL OUTPUT drops table qualifier on star (pseudo-tables only)", () => { + it("INSERT ... RETURNING star('orders') → OUTPUT INSERTED.*", () => { + const node: InsertNode = { + ...createInsertNode({ type: "table_ref", name: "orders" }), + columns: ["name"], + values: [[{ type: "literal", value: "x" }]], + returning: [{ type: "star", table: "orders" }], + } + const r = new MssqlPrinter().print(node) + expect(r.sql).toContain("OUTPUT INSERTED.*") + expect(r.sql).not.toContain("INSERTED.[orders].*") + }) + + it("bare RETURNING * → OUTPUT INSERTED.*", () => { + const node: InsertNode = { + ...createInsertNode({ type: "table_ref", name: "orders" }), + columns: ["name"], + values: [[{ type: "literal", value: "x" }]], + returning: [{ type: "star" }], + } + const r = new MssqlPrinter().print(node) + expect(r.sql).toContain("OUTPUT INSERTED.*") + }) + }) + + describe("MySQL / SQLite JSON `->` uses JSONPath ($.path / $[n])", () => { + it("MySQL at('name') → `data`->'$.name'", () => { + const r = new MysqlPrinter().print(selectJsonAccess("->", "name")) + expect(r.sql).toContain("->'$.name'") + expect(r.sql).not.toMatch(/->'name'/) + }) + + it("MySQL at('0') → `data`->'$[0]' (array index)", () => { + const r = new MysqlPrinter().print(selectJsonAccess("->", "0")) + expect(r.sql).toContain("->'$[0]'") + expect(r.sql).not.toContain("->'0'") + }) + + it("SQLite ->> name → \"data\"->>'$.name'", () => { + const r = new SqlitePrinter().print(selectJsonAccess("->>", "name")) + expect(r.sql).toContain("->>'$.name'") + }) + + it("embedded single quote in path key is escape-doubled", () => { + // e.g. `at("a'b")` should still land as a well-formed literal. + const r = new MysqlPrinter().print(selectJsonAccess("->", "a'b")) + expect(r.sql).toContain("->'$.a''b'") + }) + }) +}) diff --git a/test/audit5-regressions.test.ts b/test/audit5-regressions.test.ts index ef07862..680edf0 100644 --- a/test/audit5-regressions.test.ts +++ b/test/audit5-regressions.test.ts @@ -36,18 +36,22 @@ describe("Audit #5 regressions", () => { returning: [{ type: "star", table: "users" }], } const r = printer.print(node) - // Previously emitted `INSERTED."users".*` (wrong quote style + bug) - expect(r.sql).toContain("OUTPUT INSERTED.[users].*") + // Audit #24: MSSQL pseudo-tables (INSERTED/DELETED) cannot be + // qualified by a base table name — three-part `INSERTED..*` + // is a SQL Server parse error. Drop the caller's qualifier. + expect(r.sql).toContain("OUTPUT INSERTED.*") + expect(r.sql).not.toContain("INSERTED.[users].*") }) - it("DELETE … OUTPUT DELETED.[t].* for table-qualified star", () => { + it("DELETE … OUTPUT DELETED.* for table-qualified star", () => { const node: DeleteNode = { ...createDeleteNode({ type: "table_ref", name: "users" }), returning: [{ type: "star", table: "users" }], where: eq(col("id"), param(0, 1)), } const r = printer.print(node) - expect(r.sql).toContain("OUTPUT DELETED.[users].*") + expect(r.sql).toContain("OUTPUT DELETED.*") + expect(r.sql).not.toContain("DELETED.[users].*") }) it("INSERT OUTPUT with non-star column nodes still prefixes correctly", () => {