fix: audit #24 — coalesce arity, MSSQL OUTPUT star, MySQL/SQLite JSONPath#89
Merged
Conversation
…Path HIGH #1: coalesce() silently accepted zero arguments `COALESCE()` is invalid on every dialect (PG / MySQL / SQLite / MSSQL all reject). The variadic signature had no min-args check; the runtime driver error ("requires at least one argument") pointed at the DB instead of the caller. Added a builder-time throw. HIGH #2: MSSQL OUTPUT emitted invalid three-part name `returning(star("orders"))` compiled to `OUTPUT INSERTED.[orders].*` on MSSQL — a three-part reference to a non-existent object. The INSERTED/DELETED pseudo-tables have all the target columns already and cannot be qualified by the base table name. Fix: strip the table qualifier when a star hits the MSSQL OUTPUT path; emit `INSERTED.*` / `DELETED.*`. HIGH #4: MySQL / SQLite `->` / `->>` required JSONPath, not bare keys Both engines' JSON operators expect the RHS to be a JSONPath string starting with `$` (e.g. `data->'$.name'`, `data->'$[0]'`). The base printer emitted PG's bare-key form (`data->'name'`) which MySQL rejects with ER_INVALID_JSON_PATH and SQLite with "JSON path error". Overrode `printJsonAccess` on both MysqlPrinter and SqlitePrinter to rewrite single-segment paths to `$.name` / `$[N]`, with standard single-quote escaping of embedded quotes. Updated audit5 MSSQL test expectations to match the new pseudo-table behavior and added audit24 regression tests for each fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Review of PR #89 noted that MySQL/SQLite's JSONPath rewrite treats `at("a.b")` as a two-level path (`$.a.b`) while PG's printer treats the same string as a literal single key. Add a docblock note explaining the cross-dialect semantic scope: `.at(path)` takes a single JSON selector string, and chained `.at("a").at("b")` is the recommended portable form. Users who need path traversal on PG should use `#>` with a segment array. No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three correctness issues from audit round #24:
coalesce()silently accepted zero arguments (src/builder/eb.ts): every dialect rejectsCOALESCE(); the runtime driver error didn't point at the caller. Builder now throws.INSERTED.[tbl].*(src/printer/mssql.ts):returning(star("orders"))on MSSQL produced a reference SQL Server rejects at parse. The INSERTED/DELETED pseudo-tables cannot be qualified by a base-table name. Strip the qualifier; emitINSERTED.*/DELETED.*.->/->>required JSONPath, not bare keys (src/printer/{mysql,sqlite}.ts): base printer emitted PG's bare-key form (data->'name'); MySQL rejects withER_INVALID_JSON_PATH. Rewrite single-segment paths to$.name(or$[N]for numeric indices) with proper single-quote escaping.Test plan
pnpm fmt && pnpm lint && pnpm typecheckcleanpnpm test— 1247 pass (+8 new)test/audit24-regressions.test.ts🤖 Generated with Claude Code