PHPORM-90 Fix whereNot to use $nor
#2624
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
| if (str_ends_with($where['boolean'], 'not')) { | ||
| $result = ['$not' => $result]; | ||
| $result = ['$nor' => [$result]]; |
There was a problem hiding this comment.
I don't see PHPORM-90 linked to anything. This this bug predate us taking over the library, or was it introduced during some recent query refactoring? If the latter, we should link it up.
There was a problem hiding this comment.
I implemented whereNot in GromNaN/laravel-mongodb#13. The issue PHPORM-49 is now linked in Jira.
| @@ -1053,8 +1053,9 @@ protected function compileWheres(): array | |||
| $method = 'compileWhere' . $where['type']; | |||
| $result = $this->{$method}($where); | |||
|
|
|||
| // Negate the expression | |||
There was a problem hiding this comment.
Is $result always going to be an <expression> at this point, and never an <operator-expression> as it expected by $not?
This is related to my other comment, in that I'm curious how this wasn't discovered sooner unless it was only recently introduced. This also hints at a gap in functional test coverage running generated queries on a live server.
There was a problem hiding this comment.
Yes, compileWhere* function always return expressions with $and, $or or a field name, and now $nor.
| { | ||
| // implicit equality operator | ||
| $users = User::whereNot('title', 'admin')->get(); | ||
| $this->assertCount(6, $users); |
There was a problem hiding this comment.
This test is failing if I run it on version 3.9.3: the generated query was { title: 'admin' }, which is wrong because it ignore the negation.
In the current master, the query is { $not: { title: 'admin' } }, which is invalid and rejected by the server.
With this patch, the working query is { $nor: [ { title: 'admin' } ] }.
There was a problem hiding this comment.
reviewing the generated query was { title: 'admin' }, which is wrong because it ignore the negation.
I'm not sure if I realized this when originally reviewing PHPORM-49, but the fact that earlier versions completely ignored negations with no feedback to the user sounds like pretty disastrous bug.
There was a problem hiding this comment.
It looks like someone asked for this feature: #2418
| @@ -1053,8 +1053,9 @@ protected function compileWheres(): array | |||
| $method = 'compileWhere' . $where['type']; | |||
| $result = $this->{$method}($where); | |||
|
|
|||
| // Negate the expression | |||
There was a problem hiding this comment.
Yes, compileWhere* function always return expressions with $and, $or or a field name, and now $nor.
| if (str_ends_with($where['boolean'], 'not')) { | ||
| $result = ['$not' => $result]; | ||
| $result = ['$nor' => [$result]]; |
There was a problem hiding this comment.
I implemented whereNot in GromNaN/laravel-mongodb#13. The issue PHPORM-49 is now linked in Jira.
Fix #2623 #2418 PHPORM-90
$notcannot be used with an expression. I switched to$nor, that negates a list of expressions.The query could be optimized, but I expect the mongo server to be smart enough to do it.
Example of optimization I chose not to do because that would require to detect when there is a single field expression (related to GromNaN/laravel-mongodb#13 (comment))
This expression:
Is negated with:
While it could also be: