From 89fd55ac8577818a70f9f43d51d70841c699560d Mon Sep 17 00:00:00 2001 From: Tiago Lauer Date: Sun, 2 Aug 2026 09:47:20 -0300 Subject: [PATCH] fix(parse): check JOIN ON inside UPDATE FROM and DELETE USING StrictRow // was Record - r.nope accepted silently The joined sources were registered - returning `r.id` from the valid variant worked - so only the ON expressions went unchecked. Both write branches hardcoded `fromText: ''`, so ApplyWhereCheck's ExtractJoinOnText never saw their join text. The README says strict mode checks JOIN ... ON conditions and makes no carve-out for write statements. Both now pass the real FROM/USING clause text, taken with the same TakeFromClause a SELECT uses, and kept as raw text for the same reason: the scan only runs in strict mode. Fixes #281 Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 4 ++ src/parse.ts | 18 ++++- tests/dml-join-on-strict.test-d.ts | 104 +++++++++++++++++++++++++++++ 3 files changed, 124 insertions(+), 2 deletions(-) create mode 100644 tests/dml-join-on-strict.test-d.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 684fed8..8395b84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ Notable changes to this project, following [Keep a Changelog](https://keepachang ## [Unreleased] +### Fixed + +- Strict mode validates the `JOIN ... ON` conditions inside `UPDATE ... FROM` and `DELETE ... USING`. The joined sources were registered, so selecting from them worked, but the ON expressions were never checked because both branches passed an empty FROM text to the check - `... join refunds r on r.nope = o.id` was accepted silently ([#281](https://github.com/tiagolauer/OwlSQL/issues/281)). + ## [0.2.0] - 2026-07-29 ### Changed diff --git a/src/parse.ts b/src/parse.ts index 8f3ee9f..03de3f9 100644 --- a/src/parse.ts +++ b/src/parse.ts @@ -290,6 +290,20 @@ type ExtraSourcesAfterKeyword = [ ? ParseFromClause : []; +// The FROM/USING clause text of a write, for the same JOIN ... ON check a +// SELECT gets. Both branches hardcoded an empty string, so a mistyped column +// in `update users set ... from orders o join refunds r on r.nope = o.id` was +// accepted silently even though the joined sources themselves were registered +// (issue #281). Mirrors ParseSelectBody, which keeps the clause as raw text +// and lets ExtractJoinOnText scan it only in strict mode. +type ExtraFromTextAfterKeyword = [ + SplitAtTopLevelKeyword, +] extends [never] + ? '' + : SplitAtTopLevelKeyword extends { after: infer AfterClause extends string } + ? TakeFromClause + : ''; + export interface ParsedStatement { columns: string; sources: Source[]; @@ -346,7 +360,7 @@ type ParseStatementNormalized = Trim extends `(${infer Afte ...ExtraSourcesAfterKeyword, ]; whereText: ExtractUpdateDeleteWhereText; - fromText: ''; + fromText: ExtraFromTextAfterKeyword; } : IsKeyword extends true ? { @@ -356,7 +370,7 @@ type ParseStatementNormalized = Trim extends `(${infer Afte ...ExtraSourcesAfterKeyword, ]; whereText: ExtractUpdateDeleteWhereText; - fromText: ''; + fromText: ExtraFromTextAfterKeyword; } : IsKeyword extends true ? { diff --git a/tests/dml-join-on-strict.test-d.ts b/tests/dml-join-on-strict.test-d.ts new file mode 100644 index 0000000..1feef96 --- /dev/null +++ b/tests/dml-join-on-strict.test-d.ts @@ -0,0 +1,104 @@ +import type { QueryTypeError, Row, StrictRow } from '../src/index.js'; + +type Equal = + (() => T extends A ? 1 : 2) extends (() => T extends B ? 1 : 2) ? true : false; + +type Expect = T; + +interface DB { + users: { id: number; name: string }; + orders: { id: number; user_id: number; total: number }; + refunds: { id: number; order_id: number }; +} + +type EmptyRow = Record; + +// The joined sources were registered - returning `r.id` worked - but the ON +// expressions were never seen, because both write branches passed an empty +// FROM text to the check (issue #281). The README says strict mode checks +// JOIN ... ON conditions and carves out nothing for writes. +type UpdateFromJoinOnIsChecked = Expect< + Equal< + StrictRow< + DB, + 'update users set name = $1 from orders o join refunds r on r.nope = o.id where users.id = o.user_id' + >, + QueryTypeError<'unknown column: nope'> + > +>; + +type DeleteUsingJoinOnIsChecked = Expect< + Equal< + StrictRow< + DB, + 'delete from users using orders o join refunds r on r.nope = o.id where users.id = o.user_id' + >, + QueryTypeError<'unknown column: nope'> + > +>; + +type UpdateFromJoinOnUnknownAliasIsChecked = Expect< + Equal< + StrictRow< + DB, + 'update users set name = $1 from orders o join refunds r on q.order_id = o.id where users.id = o.user_id' + >, + QueryTypeError<'unknown alias: q'> + > +>; + +// Controls: the valid versions of the same statements still pass. +type ValidUpdateFromJoinOn = Expect< + Equal< + StrictRow< + DB, + 'update users set name = $1 from orders o join refunds r on r.order_id = o.id where users.id = o.user_id' + >, + EmptyRow + > +>; + +type ValidDeleteUsingJoinOn = Expect< + Equal< + StrictRow< + DB, + 'delete from users using orders o join refunds r on r.order_id = o.id where users.id = o.user_id' + >, + EmptyRow + > +>; + +type UpdateWithoutFromIsUnaffected = Expect< + Equal, EmptyRow> +>; + +type UpdateFromWithReturningIsUnaffected = Expect< + Equal< + StrictRow< + DB, + 'update users set name = $1 from orders o where users.id = o.user_id returning users.id' + >, + { id: number } + > +>; + +type LooseModeIsUnchanged = Expect< + Equal< + Row< + DB, + 'update users set name = $1 from orders o join refunds r on r.nope = o.id where users.id = o.user_id' + >, + EmptyRow + > +>; + +export type DmlJoinOnStrictLock = [ + UpdateFromJoinOnIsChecked, + DeleteUsingJoinOnIsChecked, + UpdateFromJoinOnUnknownAliasIsChecked, + ValidUpdateFromJoinOn, + ValidDeleteUsingJoinOn, + UpdateWithoutFromIsUnaffected, + UpdateFromWithReturningIsUnaffected, + LooseModeIsUnchanged, +];