jcoglan commented on code in PR #6080:
URL: https://github.com/apache/couchdb/pull/6080#discussion_r3949739125


##########
src/mango/src/mango_selector.erl:
##########
@@ -299,9 +321,16 @@ norm_negations({[{<<"$or">>, Args}]}) ->
 norm_negations({[{<<"$elemMatch">>, Arg}]}) ->
     {[{<<"$elemMatch">>, norm_negations(Arg)}]};
 norm_negations({[{<<"$allMatch">>, Arg}]}) ->
-    {[{<<"$allMatch">>, norm_negations(Arg)}]};
+    Out = norm_negations(Arg),
+    {[{<<"$allMatch">>, Out}]};
 norm_negations({[{<<"$keyMapMatch">>, Arg}]}) ->
     {[{<<"$keyMapMatch">>, norm_negations(Arg)}]};
+% Sometimes a top-level key will be a doc field because it
+% can't be pushed down through its corresponding operator,
+% e.g. {x: {$allMatch: S}}. Negation inside the $allMatch
+% should still be normalized.
+norm_negations({[{Field, Cond}]}) when is_list(Field) ->

Review Comment:
   My understanding is this should strictly be considered a bug fix, i.e. 
normalisation should not in general alter the meaning of an expression, but it 
does remove certain forms that `match` would otherwise have to consider. One 
such effect is that `$nor` was expected to be fully removed from the expression 
and `match` does not have to handle it at all, but this was not the case if 
`$nor` appeared inside an `$allMatch` expression since normalisation did not 
recurse into it.
   
   So the behaviour change is that expressions that did not work at all should 
now work. I'm not aware of any kind of expression that previously worked, and 
now has a different effect because of this change, such that applications might 
be relying on the old behaviour and would break after this.
   
   Am I missing something?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to