[SERVER-59785] [SBE] Change $concatArrays to not short-circuit on null Created: 03/Sep/21  Updated: 29/Oct/23  Resolved: 01/Oct/21

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: 5.1.0-rc0

Type: Improvement Priority: Major - P3
Reporter: Drew Paroski Assignee: Mihai Andrei
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Documented
is documented by DOCS-14835 DOCS-14835: Investigate changes in SE... Backlog
Related
is related to SERVER-60222 Consider changing all variadic expres... Backlog
Backwards Compatibility: Minor Change
Sprint: QE 2021-09-20, QE 2021-10-04
Participants:

 Description   

In SBE, for almost all aggregate operators, all of the operator's arguments are evaluated first before doing typechecking or anything else. This was an intentional design decision.

From what I can tell, the only aggregate operators that intentionally have "short-circuit" style behavior are as follows:

  • $and - Short-circuits evaluation of the remaining arguments when an argument evaluates to false (or something that when coerced to boolean will produce "false")
  • $or - Short-circuits evaluation of the remaining arguments when an argument evaluates to true (or something that when coerced to boolean will produce "true")
  • $switch - Short-circuits evaluation of the remaining case/then's when a case evaluates to true (or something that when coerced to boolean will produce "true")
  • $ifNull - Short-circuits evaluation of the remaining arguments when an argument evaluates to a value that is not null/missing

Aside from the operators listed above, unless there is a specific reason, each aggregate operator's arguments should be evaluated first before doing anything else.

I noticed that the SBE implementation of $concatArrays seems to exhibit "short-circuit on null" behavior. Here is an example that demonstrates this:

> db.adminCommand({setParameter: 1, internalQueryEnableSlotBasedExecutionEngine: true});
{ "was" : false, "ok" : 1 }
 
> db.c.find()
{_id: 1, a: null, b: "foo"}
 
> db.c.aggregate([{$project: {x: {$concatArrays: [[1,2], "$a", {$add: [1,"$b"]}]}}}])
{_id: 1, x: null}

The goal of this task is to determine if $concatArrays in SBE should evaluate all of its arguments first before doing typechecking or anything else, and if so, to update the SBE implementation of $concatArrays accordingly.



 Comments   
Comment by Githook User [ 08/Oct/21 ]

Author:

{'name': 'Mihai Andrei', 'email': 'mihai.andrei@10gen.com', 'username': 'mtandrei'}

Message: SERVER-59785 Ignore fuzzer failures due to short circuiting behavior … (#540)
Branch: gregorynoma/TIG-3250
https://github.com/10gen/jstestfuzz/commit/ca1fe935922c58461cec3aef19bc90382d0b035c

Comment by Vivian Ge (Inactive) [ 06/Oct/21 ]

Updating the fixversion since branching activities occurred yesterday. This ticket will be in rc0 when it’s been triggered. For more active release information, please keep an eye on #server-release. Thank you!

Comment by Githook User [ 30/Sep/21 ]

Author:

{'name': 'Mihai Andrei', 'email': 'mihai.andrei@10gen.com', 'username': 'mtandrei'}

Message: SERVER-59785 [SBE] Change $concatArrays to not short-circuit on null
Branch: master
https://github.com/mongodb/mongo/commit/7d15f0c0b536716a0996d17ca7768f2ae1a3d739

Comment by Githook User [ 27/Sep/21 ]

Author:

{'name': 'Mihai Andrei', 'email': 'mihai.andrei@10gen.com', 'username': 'mtandrei'}

Message: SERVER-59785 Ignore fuzzer failures due to short circuiting behavior … (#540)
Branch: master
https://github.com/10gen/jstestfuzz/commit/ca1fe935922c58461cec3aef19bc90382d0b035c

Comment by Kyle Suarez [ 24/Sep/21 ]

To summarize our discussion and final decision on this:

  • We will do this ticket for the sake of consistency and make $concatArrays not short-circuit.
  • We've filed SERVER-60222 to consider making all variadic expressions short circuit in SBE (and if we do it, doing them all together to maintain internal consistency).
Generated at Thu Feb 08 05:48:07 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.