[SERVER-66548] $lookup sequential cache can incorrectly treat a $facet as non-correlated Created: 18/May/22  Updated: 29/Oct/23  Resolved: 25/Jul/22

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: 4.4.13, 5.0.7, 4.2.20, 6.0.0-rc6
Fix Version/s: 6.0.1, 4.4.16, 5.0.11

Type: Bug Priority: Major - P3
Reporter: Nicholas Zolnierz Assignee: Nicholas Zolnierz
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Backports
Depends
Related
related to SERVER-63141 Difference in $lookup/$redact/$let be... Closed
related to SERVER-63845 Separate interface to get set of refe... Closed
Backwards Compatibility: Fully Compatible
Operating System: ALL
Backport Requested:
v5.3, v5.0, v4.4
Sprint: QO 2022-05-16, QO 2022-05-30, QO 2022-06-13, QO 2022-06-27, QO 2022-07-11, QO 2022-07-25, QO 2022-08-08
Participants:
Linked BF Score: 135

 Description   

For a pipeline of the following shape:

[{$lookup: {let: {a: ...}, pipeline: [{$facet: {array: [{$redact: {...$$a...}}]}}]}]

We can get incorrect results with optimizations enabled if the following occurs:

1. $lookup creates the sequential cache and slaps it at the end of the sub-pipeline
2. $lookup then attempts to re-order the cache to the optimal position with a non-correlated prefix. It relies on the dependency tracker to achieve this.
3. The $facet within the $lookup reports it's dependencies, (incorrectly?) indicating that it does not depend on any of the let variables even though it has a sub-pipeline which refers to $$a. This happens because the $redact stage within the $facet does not support dep tracking, so the variable reference is not found.
4. The cache does not swap with the facet since it believes it to be non-correlated, which can lead to incorrect query results.

Note that in step 3, if a stage within the $facet is "not supported" for dep analysis, it is not propagated back to the facet but instead ends up setting the needWholeDocument flag.



 Comments   
Comment by Githook User [ 26/Jul/22 ]

Author:

{'name': 'Nicholas Zolnierz', 'email': 'nicholas.zolnierz@mongodb.com', 'username': 'nzolnierzmdb'}

Message: SERVER-66548 Add support for dependency tracking to $redact

(cherry picked from commit cc4968771221658d66ff42ed2fb861656c5683ca)
Branch: v4.4
https://github.com/mongodb/mongo/commit/80349ba179b6f87f78d5a9f787ec8848dbd4cc22

Comment by Githook User [ 25/Jul/22 ]

Author:

{'name': 'Nicholas Zolnierz', 'email': 'nicholas.zolnierz@mongodb.com', 'username': 'nzolnierzmdb'}

Message: SERVER-66548 Add support for dependency tracking to $redact

(cherry picked from commit cc4968771221658d66ff42ed2fb861656c5683ca)
Branch: v5.0
https://github.com/mongodb/mongo/commit/56666ada8172906b106bbbf6ba9ace1b73e50e05

Comment by Githook User [ 25/Jul/22 ]

Author:

{'name': 'Nicholas Zolnierz', 'email': 'nicholas.zolnierz@mongodb.com', 'username': 'nzolnierzmdb'}

Message: SERVER-66548 Add support for dependency tracking to $redact
Branch: v6.0
https://github.com/mongodb/mongo/commit/cc4968771221658d66ff42ed2fb861656c5683ca

Comment by Steve Tarzia [ 06/Jul/22 ]

This is ready to merge into the v6.0 branch, but we're waiting for the 6.0 code freeze to end.

Comment by Steve Tarzia [ 26/May/22 ]

Sounds good, david.storch@mongodb.com. nicholas.zolnierz@mongodb.com can look at both of these, since he has the context.

Comment by David Storch [ 25/May/22 ]

Yes, I think we should definitely schedule SERVER-63845. We've had several bugs like this pop up, so I think it's worth doing on the master branch. I'll put it back into the triage queue.

It makes sense to avoid backporting SERVER-63845. Perhaps we can fix this via SERVER-63845 in master and then have a separate fix tracked by this ticket to target 6.0 and older?

Comment by Nicholas Zolnierz [ 24/May/22 ]

david.storch@mongodb.com yeah absolutely, that sounds like a more elaborate option that would be better long term. As long as we require all sources to report their variable references then it should fix this BF. It looks like that ticket is in the backlog void? Should we retriage?

Since this issue spans back to 4.x, we may want to go with a simpler fix to backport. As part of this ticket I also want to look for other stages that do not implement the getDependencies API but still may refer to variables.

Comment by David Storch [ 23/May/22 ]

nicholas.zolnierz@mongodb.com would implementing SERVER-63845 help with fixing this?

Comment by Nicholas Zolnierz [ 18/May/22 ]

There are a few different ways I can think of fixing this:

1. Update $redact to report its dependencies
2. Update sequential cache optimization to consider `needWholeDocument` dependencies as correlated and thus the cache should move ahead of such a stage
3. Add a new concept in the dependency tracker such as `hasUnsupportedStage` then use this flag in the sequential cache optimization

I confirmed that (2) at least fixes the issue locally but I need to investigate what the downsides would be. Also (1) would likely solve the BF but I suspect there are other stages that fall in a similar bucket.

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