PHPLARA-268 Fix AND/OR precedence in compileWheres()

[zigzagdev]

Fixes PHPLARA-268

compileWheres() bucketed all "and"-connected wheres and all "or"-connected wheres separately, then merged them as sibling $and and $or keys at the top level of the query. Since MongoDB implicitly ANDs sibling top-level keys, a chain like:

Model::where('a', 1)->where('b', 2)->orWhere('c', 3)

compiled to {'$and': [{a:1},{b:2}], '$or': [{c:3}]}, which MongoDB evaluates as a AND b AND c, silently dropping documents that should match through the c branch. Laravel generates where "a" = ? and "b" = ? or "c" = ? for the same chain, and SQL puts AND before OR, so the expected meaning is (a AND b) OR c.

The bug was hidden because only the trivial two-clause case was covered by tests. The special case that rewrote the boolean of the first where based on the second one happened to produce the right result for two clauses only.

Changes

• Group consecutive "and"-connected wheres together and start a new group at each "or", then combine the groups with $or. This mirrors the precedence of the SQL generated by Laravel.
• Remove the special case that rewrote the boolean of the first where, superseded by the general grouping.
• The compilation loop no longer takes each where by reference, so compileWheres() no longer mutates $this->wheres. Nothing else in the package relies on that mutation, and the method becomes idempotent.
• Add regression tests: two-clause and three-clause chains in both directions, a long alternating chain, a leading orWhere, an orWhereNot in the middle of a chain, and a nested closure next to an orWhere.

Backwards compatibility

The generated query changes for any chain of three clauses or more that mixes where and orWhere. compileWheres() is also used by update() and delete(), so an application that relied on the old, more restrictive result will now match a wider set of documents, including on writes:

Post::where('a', 1)->orWhere('b', 2)->where('owner_id', $me->id)->delete();
// before: (a OR b) AND owner  -> the owner filter applied to both branches
// after:  a OR (b AND owner)  -> the a = 1 branch is no longer filtered

This has a security dimension for applications that hand-write an authorization filter next to an orWhere: the old behavior was fail-closed by accident, not by contract, and the new one matches Laravel and SQL. Applications that want the old grouping should make it explicit with a closure, which is stable across both versions:

->where('owner_id', $me->id)->where(fn ($q) => $q->where('a', 1)->orWhere('b', 2))

Eloquent global scopes are not affected. callScope() goes through addNewWheresWithinGroup(), so SoftDeletes and multi-tenant scopes stay in their own nested group connected with "and", before and after this change.

Because this changes the result of existing queries, it targets the next minor version and must appear as a behavioral change in the release notes.

[GromNaN]

Thank you for your contribution and the explanations.

The logic seems correct to me. Here's what I'll be reviewing:

• Backward compatibility: it’s possible that some projects rely on the previous behavior
• Consistency with Eloquent SQL, but based on the comment, this seems to be an improvement

[GromNaN]

Thanks for this fix @zigzagdev, and for the clear write-up of the root cause. The analysis is right: the old code produced a logical expression that MongoDB evaluated as an implicit AND across the sibling top-level keys, and only the two-clause case ever came out correct.

I reviewed it and pushed two changes to your branch:

• compileWheres() now builds the groups directly in the compilation loop. The intermediate array that paired each compiled where with its connector was only used by the following loop, so it could go away along with that second pass. The generated MQL is unchanged.
• Four more test cases, each of which fails without the fix: a long alternating chain a or b and c or d and e, a leading orWhere followed by a where, an orWhereNot in the middle of a chain to pin down where the $nor lands, and a nested closure next to an orWhere.

I also verified that a leading whereNot still behaves as before, which is the case the removed special case used to handle, and that a chain of several orWhere still compiles to a single flat $or.

One consequence worth recording, so I expanded the description with it: compileWheres() is also used by update() and delete(), so an application that hand-writes an authorization filter next to an orWhere will now match more documents than before, on writes too. The old behavior was more restrictive by accident rather than by contract, and the new one matches what Laravel generates in SQL, so this is the right fix. It just needs to ship as a behavioral change in the release notes.

I rebased the branch on 5.x. Waiting for CI, then this is good to merge.

[GromNaN]

I'm hesitant to merge this PR. I'm worried about breaking changes.

[GromNaN]

Inline notes showing the query generated for each new test case before the fix, so the change in meaning is easy to follow.

[GromNaN]

->where('age', 1)->where('name', 'bar')->orWhere('email', 'foo')

Before this PR:

{"$and":[{"age":1},{"name":"bar"}],"$or":[{"email":"foo"}]}

MongoDB ANDs the sibling top-level keys, so this was evaluated as age AND name AND email.

With this PR:

{"$or":[{"$and":[{"age":1},{"name":"bar"}]},{"email":"foo"}]}

which is (age AND name) OR email, the meaning of the SQL that Laravel generates for the same chain.

Documents matching only on email were dropped.

[GromNaN]

->where('age', 1)->orWhere('email', 'foo')->where('name', 'bar')

Before this PR:

{"$or":[{"age":1},{"email":"foo"}],"$and":[{"name":"bar"}]}

MongoDB ANDs the sibling top-level keys, so this was evaluated as (age OR email) AND name.

With this PR:

{"$or":[{"age":1},{"$and":[{"email":"foo"},{"name":"bar"}]}]}

which is age OR (email AND name), the meaning of the SQL that Laravel generates for the same chain.

The symmetric case. Here the old query was too restrictive instead of too permissive: the trailing where was applied to both branches.

[GromNaN]

->where('a', 1)->orWhere('b', 2)->where('c', 3)->orWhere('d', 4)->where('e', 5)

Before this PR:

{"$or":[{"a":1},{"b":2},{"d":4}],"$and":[{"c":3},{"e":5}]}

MongoDB ANDs the sibling top-level keys, so this was evaluated as (a OR b OR d) AND c AND e.

With this PR:

{"$or":[{"a":1},{"$and":[{"b":2},{"c":3}]},{"$and":[{"d":4},{"e":5}]}]}

which is a OR (b AND c) OR (d AND e), the meaning of the SQL that Laravel generates for the same chain.

This is the case that shows the old bucketing most clearly: every orWhere landed in one flat $or and every where in one flat $and, losing all the grouping.

[GromNaN]

->orWhere('a', 1)->where('b', 2)

Before this PR:

{"$or":[{"a":1}],"$and":[{"b":2}]}

MongoDB ANDs the sibling top-level keys, so this was evaluated as a AND b.

With this PR:

{"$and":[{"a":1},{"b":2}]}

which is a AND b, the meaning of the SQL that Laravel generates for the same chain.

Semantics were already correct here, a leading orWhere behaves like a where. The test guards the $i > 0 condition in the new grouping, and the emitted query is now a plain $and instead of a single-element $or next to an $and.

[GromNaN]

->where('a', 1)->orWhereNot('b', 2)->where('c', 3)

Before this PR:

{"$or":[{"a":1},{"$nor":[{"b":2}]}],"$and":[{"c":3}]}

MongoDB ANDs the sibling top-level keys, so this was evaluated as (a OR NOT b) AND c.

With this PR:

{"$or":[{"a":1},{"$and":[{"$nor":[{"b":2}]},{"c":3}]}]}

which is a OR (NOT b AND c), the meaning of the SQL that Laravel generates for the same chain.

Checks that the $nor produced by the negation stays inside the group opened by the or, and is not hoisted out of it.

[GromNaN]

->where('a', 1)->orWhere(fn ($q) => $q->where('b', 2)->orWhere('c', 3))->where('d', 4)

Before this PR:

{"$or":[{"a":1},{"$or":[{"b":2},{"c":3}]}],"$and":[{"d":4}]}

MongoDB ANDs the sibling top-level keys, so this was evaluated as (a OR b OR c) AND d.

With this PR:

{"$or":[{"a":1},{"$and":[{"$or":[{"b":2},{"c":3}]},{"d":4}]}]}

which is a OR ((b OR c) AND d), the meaning of the SQL that Laravel generates for the same chain.

Nested groups built by a closure compose with the new grouping, and an explicit closure keeps behaving as its own unit. This is the pattern to recommend to anyone who wants the old grouping back.