[SERVER-63018] Have ExpressionAdd::evaluate() return intermediate result for pipeline optimization constant folding Created: 26/Jan/22 Updated: 27/Oct/23 Resolved: 27/Jun/22 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | None |
| Affects Version/s: | 5.2.0, 4.2.17, 5.0.5, 5.1.1, 4.4.12 |
| Fix Version/s: | None |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Yoon Soo Kim | Assignee: | Davis Haupt (Inactive) |
| Resolution: | Gone away | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
|||||||||||||||||||||||||||||||||||
| Operating System: | ALL | |||||||||||||||||||||||||||||||||||
| Steps To Reproduce: | Starts mongod with --setParameter enableTestCommands=1
|
|||||||||||||||||||||||||||||||||||
| Participants: | ||||||||||||||||||||||||||||||||||||
| Description |
|
The proposed fix is to make ExpressionAdd::evaluate() return intermediate result in the form of decimalTotal and nonDecimalTotal for pipeline optimization constant folding. For example, {decimalTotal: val1, nonDecimalTotal: val2} |
| Comments |
| Comment by Davis Haupt (Inactive) [ 27/Jun/22 ] |
|
After committing |
| Comment by Davis Haupt (Inactive) [ 14/Feb/22 ] |
|
On the immediate issue of resolving the BF that spawned this ticket, the difference between optimized and non-optimized pipelines will be addressed by The premise of this ticket is that the more precise intermediate value produced by $add should also be considered the more correct value. However, this seems to be an active area of discussion around type conversions and type promotions that is being addressed in the spec tracked by WRITING-10039, so I'm moving this ticket to blocked for now. I also think there's a good chance that passing along the intermediate value would require a more extensive refactor of the constant folding logic. Right now, ExpressionNary::evaluate() is called during optimization as well as during the execution of the pipeline. We don't want the functionality during execution to change, just optimization, and so we probably would need a separate method that would expose the intermediate value rather than changing the evaluate() method. |
| Comment by Rushan Chen [ 11/Feb/22 ] |
|
davis.haupt assigning this to you as you are investigating the solution for related issue. |
| Comment by Rushan Chen [ 01/Feb/22 ] |
|
Leaving this in the queue to be included as part of decimal precision semantics improvement. |