Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions .changeset/driver-sql-logicalop-retention-note.md
Original file line numberDiff line numberDiff line change
@@ -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.
16 changes: 16 additions & 0 deletions packages/plugins/driver-sql/src/sql-driver.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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);
Expand Down
Loading