Skip to content

Commit 8dd814d

Browse files
os-zhuangclaude
andauthored
docs(driver-sql): say why applyFilterCondition keeps logicalOp after #3776 (#3799)
#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: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent 4cca74c commit 8dd814d

2 files changed

Lines changed: 42 additions & 0 deletions

File tree

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
---
2+
---
3+
4+
docs(driver-sql): record why `applyFilterCondition` keeps its `logicalOp` parameter
5+
6+
Comment-only; no behavior change, and this changeset releases nothing.
7+
8+
#3776 made `logicalOp === 'or'` unreachable. All four call sites now pass
9+
`'and'` — the sole external caller plus the `$and`/`$or`/`$not` arms — because
10+
every key inside one filter object AND-s at every depth, and the `orWhere` that
11+
OR-s `$or`'s branches is applied to each branch's own sub-builder rather than
12+
handed down as `logicalOp`. Handing it down was the #3774 miscompile that
13+
widened every `$or` filter.
14+
15+
That leaves ~15 `logicalOp === 'or'` ternaries that nothing in the repo can
16+
reach. They are kept deliberately: `applyFilterCondition` is `protected`, so it
17+
is subclass API (`SqliteWasmDriver` here, `TursoDriver` downstream), and the
18+
flag is the seam an override needs to attach a condition into an OR group.
19+
Removing it would be source-breaking for any subclass that overrides the
20+
four-parameter signature, which in this repo's lockstep versioning costs a
21+
whole-stack major — a steep price for deleting a flag that changes no behavior.
22+
23+
The method-level TSDoc now states both halves, so the arms read as unreachable
24+
*by design* instead of as dead code inviting a "fix" that makes a branch
25+
propagate `'or'` again. `sql-driver-or-filter.test.ts` (19 cases) pins the
26+
semantics that made the flag dead.

packages/plugins/driver-sql/src/sql-driver.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3804,6 +3804,22 @@ export class SqlDriver implements IDataDriver {
38043804
}
38053805
}
38063806

3807+
/**
3808+
* Compiles a Filter Protocol condition onto `builder`.
3809+
*
3810+
* `logicalOp` controls only how this condition attaches to `builder`
3811+
* (`where` vs `orWhere`) — never how its contents combine. It is never
3812+
* `'or'` on any in-repo path: the sole caller passes `'and'` and every
3813+
* combinator recurses with `'and'`, because all keys in one filter object
3814+
* AND at every depth. The `orWhere` that OR-s `$or`'s branches is applied
3815+
* to each branch's own sub-builder; handing `'or'` down instead is exactly
3816+
* what widened every `$or` filter (#3774 — see sql-driver-or-filter.test.ts).
3817+
*
3818+
* So the `logicalOp === 'or'` arms below are unreachable **by design**, not
3819+
* dead weight to prune: the method is `protected`, i.e. subclass API, and
3820+
* the flag is the seam an override needs to attach a condition into an OR
3821+
* group. Do not "fix" them by making a branch propagate `'or'` again.
3822+
*/
38073823
protected applyFilterCondition(builder: Knex.QueryBuilder, condition: any, logicalOp: 'and' | 'or' = 'and', tableHint?: string | null) {
38083824
if (!condition || typeof condition !== 'object') return;
38093825
const table = tableHint ?? this.coercionKey(builder);

0 commit comments

Comments
 (0)