SERVER-50504 $facet+$match on positional path does not seem to pass results to certain subsequent stages

4 files changed, 66 insertions, 0 deletions

diff --git a/jstests/aggregation/sources/facet/use_cases.js b/jstests/aggregation/sources/facet/use_cases.js
index c3ced6a289d..1a2d662d437 100644
--- a/jstests/aggregation/sources/facet/use_cases.js
+++ b/jstests/aggregation/sources/facet/use_cases.js
@@ -106,4 +106,21 @@ const facetAutoBucketedPrices = facetResult[0].autoBucketedPrices;
 assert.sameMembers(facetManufacturers, mostCommonManufacturers);
 assert.sameMembers(facetBucketedPrices, numTVsBucketedByPriceRange);
 assert.sameMembers(facetAutoBucketedPrices, numTVsAutomaticallyBucketedByPriceRange);
+
+/**
+ * A simple case using $facet + $match. This also tests the bug found in SERVER-50504 is fixed.
+ */
+
+coll.drop();
+assert.commandWorked(coll.insert({"_id": 1, "quizzes": [{"score": 100}]}));
+assert.commandWorked(coll.insert({"_id": 2, "quizzes": [{"score": 200}]}));
+
+const facetPipeline =
+    [{$facet: {scoreRank: [{$match: {'quizzes.0.score': {$gt: 0}}}, {$count: 'count'}]}}];
+
+const facetRes = coll.aggregate(facetPipeline).toArray();
+assert.eq(facetRes.length, 1);
+const scoreRank = facetRes[0]['scoreRank'];
+assert.eq(scoreRank.length, 1);
+assert.eq(scoreRank[0]['count'], 2);
 }());
diff --git a/jstests/aggregation/sources/match/dotted_numeric_path.js b/jstests/aggregation/sources/match/dotted_numeric_path.js
new file mode 100644
index 00000000000..e2e77147979
--- /dev/null
+++ b/jstests/aggregation/sources/match/dotted_numeric_path.js
@@ -0,0 +1,20 @@
+/**
+ * Tests that $match works correctly with dotted numeric path.
+ *
+ * @tags: [sbe_incompatible]
+ */
+(function() {
+"use strict";
+
+const collName = "dotted_numeric_path";
+const coll = db.getCollection(collName);
+coll.drop();
+
+assert.commandWorked(coll.insert({"_id": 1, "quizzes": [{"score": 100}]}));
+assert.commandWorked(coll.insert({"_id": 2, "quizzes": [{"score": 200}]}));
+
+const res = coll.aggregate([{$match: {'quizzes.0.score': {$gt: 0}}}, {$count: 'count'}]).toArray();
+
+assert.eq(res.length, 1);
+assert.eq(res[0]['count'], 2);
+}());
diff --git a/src/mongo/db/matcher/expression_path.h b/src/mongo/db/matcher/expression_path.h
index a9b6ec6ca80..191e0356121 100644
--- a/src/mongo/db/matcher/expression_path.h
+++ b/src/mongo/db/matcher/expression_path.h
@@ -29,6 +29,7 @@
 
 #pragma once
 
+#include "mongo/db/field_ref.h"
 #include "mongo/db/matcher/expression.h"
 
 namespace mongo {
@@ -138,6 +139,20 @@ public:
 protected:
     void _doAddDependencies(DepsTracker* deps) const final {
         if (!path().empty()) {
+            // If a path contains a numeric component then it should not be naively added to the
+            // projection, since we do not support projecting specific array indices. Instead we add
+            // the prefix of the path up to the numeric path component. Note that we start at path
+            // component 1 rather than 0, because a numeric path component at the root of the
+            // document can only ever be a field name, never an array index.
+            FieldRef fieldRef(path());
+            for (size_t i = 1; i < fieldRef.numParts(); ++i) {
+                if (fieldRef.isNumericPathComponentStrict(i)) {
+                    auto prefix = fieldRef.dottedSubstring(0, i);
+                    deps->fields.insert(prefix.toString());
+                    return;
+                }
+            }
+
             deps->fields.insert(path().toString());
         }
     }
diff --git a/src/mongo/db/pipeline/document_source_match_test.cpp b/src/mongo/db/pipeline/document_source_match_test.cpp
index 634129596c9..703512557d6 100644
--- a/src/mongo/db/pipeline/document_source_match_test.cpp
+++ b/src/mongo/db/pipeline/document_source_match_test.cpp
@@ -226,6 +226,20 @@ TEST_F(DocumentSourceMatchTest, ShouldAddDependenciesOfAllBranchesOfOrClause) {
     ASSERT_EQUALS(false, dependencies.getNeedsMetadata(DocumentMetadataFields::kTextScore));
 }
 
+TEST_F(DocumentSourceMatchTest, ShouldNotAddPotentialArrayIndexToDependencies) {
+    auto match = DocumentSourceMatch::create(
+        fromjson("{$or: [{'a.0': 1, '3': 1, 'd.01': 1}, {'b.c.d.1': {$gt: 1}}]}"), getExpCtx());
+    DepsTracker dependencies;
+    ASSERT_EQUALS(DepsTracker::State::SEE_NEXT, match->getDependencies(&dependencies));
+    // Here we add "a" instead of "a.0". Since we do not support projecting specific array indices,
+    // we add the prefix of the path up to the numeric path component instead.
+    ASSERT_EQUALS(1U, dependencies.fields.count("a"));
+    ASSERT_EQUALS(1U, dependencies.fields.count("b.c.d"));
+    ASSERT_EQUALS(1U, dependencies.fields.count("3"));
+    ASSERT_EQUALS(1U, dependencies.fields.count("d.01"));
+    ASSERT_EQUALS(4U, dependencies.fields.size());
+}
+
 TEST_F(DocumentSourceMatchTest, TextSearchShouldRequireWholeDocumentAndTextScore) {
     auto match = DocumentSourceMatch::create(fromjson("{$text: {$search: 'hello'} }"), getExpCtx());
     DepsTracker dependencies(DepsTracker::kAllMetadata & ~DepsTracker::kOnlyTextScore);