fix(client): orWhere now groups, so an earlier where keeps applying (#1083)

Closes #1083.

The bug

orWhere appended a bare, ungrouped OR. SQL binds AND tighter than OR, so:

.where('id', '<=', 10)
.where('migration', 'like', '%create%')
.orWhere('migration', 'like', '%table%')
// SELECT * FROM migrations WHERE id <= ? AND migration like ? OR migration like ?

parses as (id <= 10 AND create) OR table. id <= 10 silently stops applying: 10 rows → 118.

The query is valid SQL, nothing warns, and the result is a superset of what was asked for. That last part is what made it dangerous — it fails toward MORE rows, so it looks like data rather than an error. The reporter hit it on a moderation queue: a "Pending" tab search returned 18 rows instead of 3, 15 of them already published, and because the list and count queries were built identically the wrong total agreed with the wrong rows.

The rule now

Each maximal run of OR terms is bracketed with the term that opens it, so OR binds tighter than AND in chain order:

.where(a).where(b).orWhere(c)   ->  a AND (b OR c)
.where(a).orWhere(b).where(c)   ->  (a OR b) AND c

That is the inverse of raw SQL, deliberately. It is how the chain reads, and it is already what whereAny/whereAll/whereNone and the ORM's whereGroup do — orWhere was the odd one out.

Before / after, measured

Against the 118-row fixture from the issue:

Verified on SQLite and on live Postgres, including the placeholder renumbering for nested groups.

Why there was nothing to fix in orWhere itself

The select builder had no WHERE representation. Clauses were appended to a text string — alongside a whereConditions array with 25 pushes and zero reads, a structured form nobody ever wired to the emit path. That is why this shipped. That array is now the real term list, rendered at emit time by renderWhereTerms().

The ORM had the same defect by a different route: it did keep terms with an and/or flag, and joined them flat. Having the right data structure is not the same as using it.

Both now share src/sql-fragments.ts. Two independent copies of one rule is how they drifted — they also had two copies of the IN () bug.

Fixed along the way

All consequences of the same missing structure:

• A leading orWhere/orWhereIn/orWhereNotIn/orWhereColumn/orWhereNested emitted SELECT … FROM t OR … — a syntax error on every dialect.
• .where(a).orderBy(c).orWhere(b) emitted … ORDER BY c ASC OR b. The WHERE is now spliced ahead of the trailing clauses instead of appended, so clause ordering is structural rather than regex-managed. (.where(x).join(y) benefits from the same change.)
• The dynamic orWhereX proxy mis-grouped through a second path: it also assigned built = sql`${ensureBuilt()} OR …` , which was the object actually executed. whereHas/whereDoesntHave and the soft-delete filter carried the same double-write with an unconditional WHERE keyword.
• An object condition came apart: .where({a, b}).orWhere(c) dropped a. An object is one term now, bracketed when it has more than one key.
• wherePivotIn(rel, col, []) dropped its predicate and widened the query.
• andWhere and orWhere were two near-identical 75-line copies differing only in the connector — precisely how the OR variant came to be the broken one. One implementation now.

⚠️ This is a behaviour change

Emitted SQL changes for any chain of 3+ where-terms containing an orWhere, on both the select builder and the ORM. Two-term chains, all-OR chains, and chains with no orWhere are byte-identical — asserted with toBe, not toContain, so the no-churn guarantee is pinned.

Who is affected:

1. Anyone who wrote .where(x).where(y).orWhere(z) meaning (x AND y) OR z and got it by accident. They now get x AND (y OR z) and should switch to whereGroup.
2. Anyone who was bitten (the reporter) gets correct rows with no code change.
3. Anyone snapshot-testing emitted SQL for 3+-term OR chains.

The migration path ships in this PR: whereGroup(cb) / orWhereGroup(cb), plus orWhereNull, orWhereNotNull, orWhereBetween, orWhereExists, orWhereRaw.

// the old reading, said explicitly
.whereGroup(qb => qb.where(x).where(y)).orWhere(z)   // (x AND y) OR z

An e2e test asserts whereGroup reproduces the pre-fix row count exactly (80), so the old behaviour is provably still reachable.

where(callback) also builds a real group now instead of throwing — the form docs/guide/where.md has documented since before the builder rejected it. A callback that adds no conditions still throws, so the fails-closed guarantee that test file exists to protect is unchanged.

No compatibility flag: two emit paths would double the surface of the code that just shipped a silent wrong-rows bug.

Tests

43 new tests across four files:

• client.or-where-precedence.test.ts — SQL shape, incl. the byte-identical guarantees and the sub-select builder
• client.or-where-precedence.e2e.test.ts — row counts, each carrying the pre-fix number. Asserting the string proves the shape changed; only rows prove it was wrong.
• orm.or-where-precedence.test.ts — the ORM mirror, incl. delete() (which shares buildWhereClauses, so the mis-grouping removed rows the filter was meant to protect)
• Strengthened four existing tests that had vacuous assertions — types-and-inference.test.ts:888 checked toContain('WHERE'), toContain('OR'), toContain('AND') and passed on the broken output. That is the test that should have caught this.

1776 pass / 4 skip / 0 fail, typecheck clean, 0 lint errors.

Noted, not fixed here

createTableFromModel maps type: 'integer' to a TEXT column, so integer attributes read back as strings. Unrelated to this issue — worth its own report.

🤖 Generated with Claude Code