[SERVER-78596] SBE $project aggregation $sqrt: {NaN} asserts but should return NaN Created: 30/Jun/23  Updated: 29/Oct/23  Resolved: 04/Aug/23

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: 6.3.0-rc0, 7.0.0-rc0, 7.1.0-rc0
Fix Version/s: 7.1.0-rc0

Type: Bug Priority: Major - P3
Reporter: Kevin Cherkauer Assignee: Adi Agrawal
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Depends
depends on SERVER-78246 Add $project and $addFields to the se... Closed
Assigned Teams:
Query Execution
Backwards Compatibility: Fully Compatible
Operating System: ALL
Participants:

 Description   

Expected behavior of a query like this

db.a.aggregate([{$project: {_id: 0, result: {$sqrt: [NaN]}}}]) 

is to return the following for each document in the collection

{ "result" : NaN }

This $project does not get pushed down to SBE until the SBE Pushdown feature (PM-3162), which is still in progress. Working on this feature, the above $project does get pushed down to SBE and fails with this assertion:

uncaught exception: Error: command failed: {
   "ok" : 0,
   "errmsg" : "PlanExecutor error during aggregation :: caused by :: $sqrt's argument must be greater than or equal to 0",
   "code" : 7157710,
  "codeName" : "Location7157710"
} with original command request: {
   "aggregate" : "sqrt",
   "pipeline" : [
           {
                   "$project" : {
                           "_id" : 0,
                           "result" : {
                                   "$sqrt" : [
                                           NaN
                                   ]
                           }
                   }
           }
   ],
   "cursor" : {
   },
   "lsid" : {
           "id" : UUID("5494ede8-1225-47e6-9396-7975688eaf2a")
   }
}

This assertion was introduced in 6.3.0-rc0 by SERVER-71577.



 Comments   
Comment by Githook User [ 07/Aug/23 ]

Author:

{'name': 'Adityavardhan Agrawal', 'email': 'adi.agrawal@mongodb.com', 'username': 'Adityav369'}

Message: SERVER-78596 SBE $project aggregation $sqrt:

{NaN}

asserts but should return NaN
Branch: minh.luu-no_compile_sys-perf
https://github.com/mongodb/mongo/commit/c4fee3d918dd98c3cf07eee62fb8049cb4de4e21

Comment by Githook User [ 04/Aug/23 ]

Author:

{'name': 'Adityavardhan Agrawal', 'email': 'adi.agrawal@mongodb.com', 'username': 'Adityav369'}

Message: SERVER-78596 SBE $project aggregation $sqrt:

{NaN}

asserts but should return NaN
Branch: master
https://github.com/mongodb/mongo/commit/c4fee3d918dd98c3cf07eee62fb8049cb4de4e21

Comment by Matt Boros [ 18/Jul/23 ]

Would we want someone from QO to review as well? Since this will change the Bonsai constant folder.

Comment by Kevin Cherkauer [ 11/Jul/23 ]

kyle.suarez@mongodb.com This works for me as long as we don't discover any cases where the bug can be exposed without the SBE Pushdown feature. Currently I don't know of any and it seems likely it is not possible to trigger it without the feature. If we discover some case that can trigger it without this feature, then it should be fixed as a separate bug and backported.

Comment by Kyle Suarez [ 11/Jul/23 ]

Given that this would be uncovered by the prefix project, we are adding this as another work item to that project. justin.seyster@mongodb.com, kevin.cherkauer@mongodb.com, amr.elhelw@mongodb.com let us know if you have an objection.

Comment by Justin Seyster [ 06/Jul/23 ]

amr.elhelw@mongodb.com, my understanding is that one of the copies is specifically for the stage builders. That way any changes that are just for the stage builders but that won't be useful in the long term can go in the temporary copy without the need for the optimizer team to review and maintain them.

Comment by Amr Elhelw [ 06/Jul/23 ]

justin.seyster@mongodb.com Thanks for the analysis. Do you know if there is a good reason why we have two copies of this code, rather than a common function that can be reused?

Comment by Justin Seyster [ 05/Jul/23 ]

It looks like the cause is a bug in the `ExpressionConstEval` ABT pass:
https://github.com/mongodb/mongo/blob/ba6406caabe61d58bd621f96d983acea75a33634/src/mongo/db/query/sbe_stage_builder_const_eval.cpp#L315-L415

The constant folding doesn't have the same NaN comparison semantics as the VM implementation. Normally, `NaN < 0` evaluates to `false` in the VM, allowing the `sqrt` computation to continue and correctly produce a NaN result.

Normally, the AST-level optimizer performs constant folding before the `ExpressionConstEval` pass executes, but with `disablePipelineOptimization` set (as in the `aggregation_disabled_optimization` passthrough), the AST optimizer gets turned off, allowing the newer ABT optimizer to incorrectly fold the expression to the `EFail` case.

Note that there are temporarily two copies of the ABT constant folder, and any fix should be applied to both. The second constant folder is here:
https://github.com/mongodb/mongo/blob/ba6406caabe61d58bd621f96d983acea75a33634/src/mongo/db/query/optimizer/rewrites/const_eval.cpp#L331-L430

Generated at Thu Feb 08 06:38:45 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.