From 61ed4a147bc2ffb673fa291321b98f81b15ecab6 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Tue, 28 Jul 2026 13:57:19 +0800 Subject: [PATCH] docs(driver-sql): say why `applyFilterCondition` keeps `logicalOp` after #3776 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #3776 left `logicalOp === 'or'` unreachable: all four call sites pass 'and' — the sole external caller plus the `$and`/`$or`/`$not` arms — so the ~15 `logicalOp === 'or'` ternaries are dead code. Keep the parameter rather than prune it. `applyFilterCondition` is `protected`, i.e. subclass API (`SqliteWasmDriver` here, `TursoDriver` downstream), and the flag is the seam an override needs to attach a condition into an OR group. Removing it is source-breaking for any subclass overriding the four-parameter signature, which under this repo's lockstep versioning costs a whole-stack major — steep for deleting a flag that changes no behavior. Document both halves at the method so the arms read as unreachable by design, not as dead code inviting a "fix" that makes a branch propagate 'or' again — precisely the #3774 miscompile. Comment-only. sql-driver-or-filter.test.ts (19 cases) and the full driver-sql suite (386) stay green; empty changeset since this releases nothing. Co-Authored-By: Claude --- .../driver-sql-logicalop-retention-note.md | 26 +++++++++++++++++++ packages/plugins/driver-sql/src/sql-driver.ts | 16 ++++++++++++ 2 files changed, 42 insertions(+) create mode 100644 .changeset/driver-sql-logicalop-retention-note.md diff --git a/.changeset/driver-sql-logicalop-retention-note.md b/.changeset/driver-sql-logicalop-retention-note.md new file mode 100644 index 0000000000..ad5d83ea51 --- /dev/null +++ b/.changeset/driver-sql-logicalop-retention-note.md @@ -0,0 +1,26 @@ +--- +--- + +docs(driver-sql): record why `applyFilterCondition` keeps its `logicalOp` parameter + +Comment-only; no behavior change, and this changeset releases nothing. + +#3776 made `logicalOp === 'or'` unreachable. All four call sites now pass +`'and'` — the sole external caller plus the `$and`/`$or`/`$not` arms — because +every key inside one filter object AND-s at every depth, and the `orWhere` that +OR-s `$or`'s branches is applied to each branch's own sub-builder rather than +handed down as `logicalOp`. Handing it down was the #3774 miscompile that +widened every `$or` filter. + +That leaves ~15 `logicalOp === 'or'` ternaries that nothing in the repo can +reach. They are kept deliberately: `applyFilterCondition` is `protected`, so it +is subclass API (`SqliteWasmDriver` here, `TursoDriver` downstream), and the +flag is the seam an override needs to attach a condition into an OR group. +Removing it would be source-breaking for any subclass that overrides the +four-parameter signature, which in this repo's lockstep versioning costs a +whole-stack major — a steep price for deleting a flag that changes no behavior. + +The method-level TSDoc now states both halves, so the arms read as unreachable +*by design* instead of as dead code inviting a "fix" that makes a branch +propagate `'or'` again. `sql-driver-or-filter.test.ts` (19 cases) pins the +semantics that made the flag dead. diff --git a/packages/plugins/driver-sql/src/sql-driver.ts b/packages/plugins/driver-sql/src/sql-driver.ts index ef99e5b2fb..413d00ddab 100644 --- a/packages/plugins/driver-sql/src/sql-driver.ts +++ b/packages/plugins/driver-sql/src/sql-driver.ts @@ -3721,6 +3721,22 @@ export class SqlDriver implements IDataDriver { } } + /** + * Compiles a Filter Protocol condition onto `builder`. + * + * `logicalOp` controls only how this condition attaches to `builder` + * (`where` vs `orWhere`) — never how its contents combine. It is never + * `'or'` on any in-repo path: the sole caller passes `'and'` and every + * combinator recurses with `'and'`, because all keys in one filter object + * AND at every depth. The `orWhere` that OR-s `$or`'s branches is applied + * to each branch's own sub-builder; handing `'or'` down instead is exactly + * what widened every `$or` filter (#3774 — see sql-driver-or-filter.test.ts). + * + * So the `logicalOp === 'or'` arms below are unreachable **by design**, not + * dead weight to prune: the method is `protected`, i.e. subclass API, and + * the flag is the seam an override needs to attach a condition into an OR + * group. Do not "fix" them by making a branch propagate `'or'` again. + */ protected applyFilterCondition(builder: Knex.QueryBuilder, condition: any, logicalOp: 'and' | 'or' = 'and', tableHint?: string | null) { if (!condition || typeof condition !== 'object') return; const table = tableHint ?? this.coercionKey(builder);