SERVER-21929 Make $in with null always match undefined

(cherry picked from commit d75f3a8bb66ef1ef41bfd16eecd42c31f91c818b)

8 files changed, 287 insertions, 104 deletions

diff --git a/etc/backports_required_for_multiversion_tests.yml b/etc/backports_required_for_multiversion_tests.yml
index f61e63811fa..63cc000aab2 100644
--- a/etc/backports_required_for_multiversion_tests.yml
+++ b/etc/backports_required_for_multiversion_tests.yml
@@ -150,6 +150,12 @@ all:
     test_file: jstests/sharding/txn_writes_during_movechunk.js
   - ticket: SERVER-57476
     test_file: jstests/replsets/assert_on_prepare_conflict_with_hole.js
+  - ticket: SERVER-21929
+    test_file: jstests/core/null_query_semantics.js
+  - ticket: SERVER-21929
+    test_file: jstests/core/or_to_in.js
+  - ticket: SERVER-21929
+    test_file: jstests/aggregation/sources/lookup/lookup_null_semantics.js
 
 # Tests that should only be excluded from particular suites should be listed under that suite.
 suites:
diff --git a/jstests/aggregation/sources/lookup/lookup_null_semantics.js b/jstests/aggregation/sources/lookup/lookup_null_semantics.js
new file mode 100644
index 00000000000..5ea50382558
--- /dev/null
+++ b/jstests/aggregation/sources/lookup/lookup_null_semantics.js
@@ -0,0 +1,111 @@
+/**
+ * Test that $lookup behaves correctly for null values, as well as "missing" and undefined.
+ *
+ * @tags: [
+ *   # We don't yet support lookup into a sharded collection.
+ *   assumes_unsharded_collection,
+ * ]
+ */
+(function() {
+"use strict";
+
+load("jstests/aggregation/extras/utils.js");
+
+const localColl = db.lookup_null_semantics_local;
+const foreignColl = db.lookup_null_semantics_foreign;
+
+localColl.drop();
+foreignColl.drop();
+
+assert.commandWorked(localColl.insert([
+    {_id: 0},
+    {_id: 1, a: null},
+    {_id: 2, a: 9},
+    {_id: 3, a: [null, 9]},
+]));
+assert.commandWorked(foreignColl.insert([
+    {_id: 0},
+    {_id: 1, b: null},
+    {_id: 2, b: undefined},
+    {_id: 3, b: 9},
+    {_id: 4, b: [null, 9]},
+    {_id: 5, b: 42},
+    {_id: 6, b: [undefined, 9]},
+    {_id: 7, b: [9, 10]},
+]));
+
+const actualResults = localColl.aggregate([{
+    $lookup: {from: foreignColl.getName(), localField: "a", foreignField: "b", as: "lookupResults"}
+}]).toArray();
+
+const expectedResults = [
+    // Missing on the left-hand side results in a lookup with $eq:null semantics on the left-hand
+    // side. Namely, we expect this document to join with null, missing, and undefined, or arrays
+    // thereof.
+    {
+        _id: 0,
+        lookupResults: [
+            {_id: 0},
+            {_id: 1, b: null},
+            {_id: 2, b: undefined},
+            {_id: 4, b: [null, 9]},
+            {_id: 6, b: [undefined, 9]}
+        ]
+    },
+    // Null on the left-hand side is the same as the missing case above.
+    {
+        _id: 1,
+        a: null,
+        lookupResults: [
+            {_id: 0},
+            {_id: 1, b: null},
+            {_id: 2, b: undefined},
+            {_id: 4, b: [null, 9]},
+            {_id: 6, b: [undefined, 9]}
+        ]
+    },
+    // A "negative" test-case where the value being looked up is not nullish.
+    {
+        _id: 2,
+        a: 9,
+        lookupResults: [
+            {_id: 3, b: 9},
+            {_id: 4, b: [null, 9]},
+            {_id: 6, b: [undefined, 9]},
+            {_id: 7, b: [9, 10]},
+        ]
+    },
+    // Here we are looking up both null and a scalar. We expected missing, null, and undefined to
+    // match in addition to the matches due to the scalar.
+    {
+        _id: 3,
+        a: [null, 9],
+        lookupResults: [
+            {_id: 0},
+            {_id: 1, b: null},
+            {_id: 2, b: undefined},
+            {_id: 3, b: 9},
+            {_id: 4, b: [null, 9]},
+            {_id: 6, b: [undefined, 9]},
+            {_id: 7, b: [9, 10]},
+        ]
+    },
+];
+
+assertArrayEq({actual: actualResults, expected: expectedResults});
+
+// The results should not change there is an index available on the right-hand side.
+assert.commandWorked(foreignColl.createIndex({b: 1}));
+assertArrayEq({actual: actualResults, expected: expectedResults});
+
+// If the left-hand side collection has a value of undefined for "localField", then the query will
+// fail. This is a consequence of the fact that queries which explicitly compare to undefined, such
+// as {$eq:undefined}, are banned. Arguably this behavior could be improved, but we are unlikely to
+// change it given that the undefined BSON type has been deprecated for many years.
+assert.commandWorked(localColl.insert({a: undefined}));
+assert.throws(() => {
+    localColl.aggregate([{
+    $lookup: {from: foreignColl.getName(), localField: "a", foreignField: "b", as: "lookupResults"}
+}]).toArray();
+});
+}());
diff --git a/jstests/core/null_query_semantics.js b/jstests/core/null_query_semantics.js
index 2a5437ada9d..4ac6064a2fa 100644
--- a/jstests/core/null_query_semantics.js
+++ b/jstests/core/null_query_semantics.js
@@ -16,7 +16,7 @@ function extractAValues(results) {
     });
 }
 
-function testNotEqualsNullSemantics() {
+function testNullSemantics() {
     // For the first portion of the test, only insert documents without arrays. This will avoid
     // making the indexes multi-key, which may allow an index to be used to answer the queries.
     assert.commandWorked(coll.insert([
@@ -63,6 +63,40 @@ function testNotEqualsNullSemantics() {
         assert(resultsEq(projectResults, extractAValues(expected)), tojson(projectResults));
     }());
 
+    // Test the semantics of the query {a: {$in: [null, <number>]}}.
+    (function testInNullQuery() {
+        const query = {a: {$in: [null, 4]}};
+        const noProjectResults = coll.find(query).toArray();
+        const expected = [
+            {_id: "a_null", a: null},
+            {_id: "a_number", a: 4},
+            {_id: "a_undefined", a: undefined},
+            {_id: "no_a"},
+        ];
+
+        assert(resultsEq(noProjectResults, expected), noProjectResults);
+
+        const projectResults = coll.find(query, projectToOnlyA).toArray();
+        assert(resultsEq(projectResults, extractAValues(expected)), projectResults);
+    }());
+
+    // Test the semantics of the query {$or: [{a: null}, {a: <number>}]}.
+    (function testOrNullQuery() {
+        const query = {$or: [{a: null}, {a: 4}]};
+        const noProjectResults = coll.find(query).toArray();
+        const expected = [
+            {_id: "a_null", a: null},
+            {_id: "a_number", a: 4},
+            {_id: "a_undefined", a: undefined},
+            {_id: "no_a"},
+        ];
+
+        assert(resultsEq(noProjectResults, expected), noProjectResults);
+
+        const projectResults = coll.find(query, projectToOnlyA).toArray();
+        assert(resultsEq(projectResults, extractAValues(expected)), projectResults);
+    }());
+
     // Test the semantics of the query {a: {$nin: [null, <number>]}}.
     (function testNotInNullQuery() {
         const query = {a: {$nin: [null, 4]}};
@@ -74,18 +108,10 @@ function testNotEqualsNullSemantics() {
             {_id: "a_subobject_b_undefined", a: {b: undefined}},
         ];
 
-        // TODO: SERVER-21929: $in may (or may not) miss fields with value "undefined".
-        const expectedWithUndefined = expected.concat([
-            {_id: "a_undefined", a: undefined},
-        ]);
-        assert(resultsEq(noProjectResults, expected) ||
-                   resultsEq(noProjectResults, expectedWithUndefined),
-               noProjectResults);
+        assert(resultsEq(noProjectResults, expected), noProjectResults);
 
         const projectResults = coll.find(query, projectToOnlyA).toArray();
-        assert(resultsEq(projectResults, extractAValues(expected)) ||
-                   resultsEq(projectResults, extractAValues(expectedWithUndefined)),
-               projectResults);
+        assert(resultsEq(projectResults, extractAValues(expected)), projectResults);
     }());
 
     (function testNotInNullAndRegexQuery() {
@@ -100,18 +126,10 @@ function testNotEqualsNullSemantics() {
             {_id: "a_subobject_b_null", a: {b: null}},
             {_id: "a_subobject_b_undefined", a: {b: undefined}},
         ];
-        // TODO: SERVER-21929: $in may (or may not) miss fields with value "undefined".
-        const expectedWithUndefined = expected.concat([
-            {_id: "a_undefined", a: undefined},
-        ]);
-        assert(resultsEq(noProjectResults, expected) ||
-                   resultsEq(noProjectResults, expectedWithUndefined),
-               tojson(noProjectResults));
+        assert(resultsEq(noProjectResults, expected), tojson(noProjectResults));
 
         const projectResults = coll.find(query, projectToOnlyA).toArray();
-        assert(resultsEq(projectResults, extractAValues(expected)) ||
-                   resultsEq(projectResults, extractAValues(expectedWithUndefined)),
-               projectResults);
+        assert(resultsEq(projectResults, extractAValues(expected)), projectResults);
     }());
 
     (function testExistsFalse() {
@@ -283,6 +301,44 @@ function testNotEqualsNullSemantics() {
         assert(resultsEq(noProjectResults, expected), tojson(noProjectResults));
     }());
 
+    // Test the semantics of the query {a: {$in: [null, <number>]}}.
+    (function testInNullQuery() {
+        const query = {a: {$in: [null, 75]}};
+        const noProjectResults = coll.find(query).toArray();
+        const expected = [
+            {_id: "a_null", a: null},
+            {_id: "a_undefined", a: undefined},
+            {_id: "no_a"},
+            {_id: "a_value_array_all_nulls", a: [null, null]},
+            {_id: "a_value_array_with_null", a: [1, "string", null, 4]},
+            {_id: "a_value_array_with_undefined", a: [1, "string", undefined, 4]},
+        ];
+
+        assert(resultsEq(noProjectResults, expected), noProjectResults);
+
+        const projectResults = coll.find(query, projectToOnlyA).toArray();
+        assert(resultsEq(projectResults, extractAValues(expected)), projectResults);
+    }());
+
+    // Test the semantics of the query {$or: [{a: null}, {a: <number>}]}.
+    (function testOrNullQuery() {
+        const query = {$or: [{a: null}, {a: 75}]};
+        const noProjectResults = coll.find(query).toArray();
+        const expected = [
+            {_id: "a_null", a: null},
+            {_id: "a_undefined", a: undefined},
+            {_id: "no_a"},
+            {_id: "a_value_array_all_nulls", a: [null, null]},
+            {_id: "a_value_array_with_null", a: [1, "string", null, 4]},
+            {_id: "a_value_array_with_undefined", a: [1, "string", undefined, 4]},
+        ];
+
+        assert(resultsEq(noProjectResults, expected), noProjectResults);
+
+        const projectResults = coll.find(query, projectToOnlyA).toArray();
+        assert(resultsEq(projectResults, extractAValues(expected)), projectResults);
+    }());
+
     // Test the semantics of the query {a: {$nin: [null, <number>]}}.
     (function testNotInNullQuery() {
         const query = {a: {$nin: [null, 75]}};
@@ -304,19 +360,10 @@ function testNotEqualsNullSemantics() {
             {_id: "a_value_array_no_nulls", a: [1, "string", 4]},
         ];
 
-        // TODO: SERVER-21929: $in may (or may not) miss fields with value "undefined".
-        const expectedWithUndefined = expected.concat([
-            {_id: "a_undefined", a: undefined},
-            {_id: "a_value_array_with_undefined", a: [1, "string", undefined, 4]},
-        ]);
-        assert(resultsEq(noProjectResults, expected) ||
-                   resultsEq(noProjectResults, expectedWithUndefined),
-               noProjectResults);
+        assert(resultsEq(noProjectResults, expected), noProjectResults);
 
         const projectResults = coll.find(query, projectToOnlyA).toArray();
-        assert(resultsEq(projectResults, extractAValues(expected)) ||
-                   resultsEq(projectResults, extractAValues(expectedWithUndefined)),
-               projectResults);
+        assert(resultsEq(projectResults, extractAValues(expected)), projectResults);
     }());
 
     (function testNotInNullAndRegexQuery() {
@@ -338,19 +385,10 @@ function testNotEqualsNullSemantics() {
             {_id: "a_object_array_some_b_missing", a: [{b: 3}, {}]},
         ];
 
-        // TODO: SERVER-21929: $in may (or may not) miss fields with value "undefined".
-        const expectedWithUndefined = expected.concat([
-            {_id: "a_undefined", a: undefined},
-        ]);
-
-        assert(resultsEq(noProjectResults, expected) ||
-                   resultsEq(noProjectResults, expectedWithUndefined),
-               noProjectResults);
+        assert(resultsEq(noProjectResults, expected), noProjectResults);
 
         const projectResults = coll.find(query, projectToOnlyA).toArray();
-        assert(resultsEq(projectResults, extractAValues(expected)) ||
-                   resultsEq(projectResults, extractAValues(expectedWithUndefined)),
-               projectResults);
+        assert(resultsEq(projectResults, extractAValues(expected)), projectResults);
     }());
 
     // Test the results of similar queries with an $elemMatch.
@@ -465,6 +503,48 @@ function testNotEqualsNullSemantics() {
                tojson(projectResults));
     }());
 
+    // Test the semantics of the query {a.b: {$in: [null, <number>]}}.
+    (function testDottedInNullQuery() {
+        const query = {"a.b": {$in: [null, 75]}};
+        const noProjectResults = coll.find(query).toArray();
+        const expected = [
+            {_id: "a_empty_subobject", a: {}},
+            {_id: "a_null", a: null},
+            {_id: "a_number", a: 4},
+            {_id: "a_subobject_b_null", a: {b: null}},
+            {_id: "a_subobject_b_undefined", a: {b: undefined}},
+            {_id: "a_undefined", a: undefined},
+            {_id: "no_a"},
+            {_id: "a_object_array_all_b_nulls", a: [{b: null}, {b: undefined}, {b: null}, {}]},
+            {_id: "a_object_array_some_b_nulls", a: [{b: null}, {b: 3}, {b: null}]},
+            {_id: "a_object_array_some_b_undefined", a: [{b: undefined}, {b: 3}]},
+            {_id: "a_object_array_some_b_missing", a: [{b: 3}, {}]},
+        ];
+
+        assert(resultsEq(noProjectResults, expected), noProjectResults);
+    }());
+
+    // Test the semantics of the query {$or: [{a.b: null}, {a.b: <number>}]}.
+    (function testDottedOrNullQuery() {
+        const query = {$or: [{"a.b": null}, {"a.b": 75}]};
+        const noProjectResults = coll.find(query).toArray();
+        const expected = [
+            {_id: "a_empty_subobject", a: {}},
+            {_id: "a_null", a: null},
+            {_id: "a_number", a: 4},
+            {_id: "a_subobject_b_null", a: {b: null}},
+            {_id: "a_subobject_b_undefined", a: {b: undefined}},
+            {_id: "a_undefined", a: undefined},
+            {_id: "no_a"},
+            {_id: "a_object_array_all_b_nulls", a: [{b: null}, {b: undefined}, {b: null}, {}]},
+            {_id: "a_object_array_some_b_nulls", a: [{b: null}, {b: 3}, {b: null}]},
+            {_id: "a_object_array_some_b_undefined", a: [{b: undefined}, {b: 3}]},
+            {_id: "a_object_array_some_b_missing", a: [{b: 3}, {}]},
+        ];
+
+        assert(resultsEq(noProjectResults, expected), noProjectResults);
+    }());
+
     // Test the semantics of the query {a.b: {$nin: [null, <number>]}}.
     (function testDottedNotInNullQuery() {
         const query = {"a.b": {$nin: [null, 75]}};
@@ -480,19 +560,10 @@ function testNotEqualsNullSemantics() {
             {_id: "a_value_array_with_undefined", a: [1, "string", undefined, 4]},
         ];
 
-        // TODO: SERVER-21929: $in may (or may not) miss fields with value "undefined".
-        const expectedWithUndefined = expected.concat([
-            {_id: "a_object_array_some_b_undefined", a: [{b: undefined}, {b: 3}]},
-            {_id: "a_subobject_b_undefined", a: {b: undefined}},
-        ]);
-        assert(resultsEq(noProjectResults, expected) ||
-                   resultsEq(noProjectResults, expectedWithUndefined),
-               noProjectResults);
+        assert(resultsEq(noProjectResults, expected), noProjectResults);
 
         const projectResults = coll.find(query, projectToOnlyA).toArray();
-        assert(resultsEq(projectResults, extractAValues(expected)) ||
-                   resultsEq(projectResults, extractAValues(expectedWithUndefined)),
-               projectResults);
+        assert(resultsEq(projectResults, extractAValues(expected)), projectResults);
     }());
 
     // Test the semantics of the query {a.b: {$nin: [null, <regex>]}}.
@@ -509,19 +580,10 @@ function testNotEqualsNullSemantics() {
             {_id: "a_value_array_with_undefined", a: [1, "string", undefined, 4]},
         ];
 
-        // TODO: SERVER-21929: $in may (or may not) miss fields with value "undefined".
-        const expectedWithUndefined = expected.concat([
-            {_id: "a_object_array_some_b_undefined", a: [{b: undefined}, {b: 3}]},
-            {_id: "a_subobject_b_undefined", a: {b: undefined}},
-        ]);
-        assert(resultsEq(noProjectResults, expected) ||
-                   resultsEq(noProjectResults, expectedWithUndefined),
-               noProjectResults);
+        assert(resultsEq(noProjectResults, expected), noProjectResults);
 
         const projectResults = coll.find(query, projectToOnlyA).toArray();
-        assert(resultsEq(projectResults, extractAValues(expected)) ||
-                   resultsEq(projectResults, extractAValues(expectedWithUndefined)),
-               projectResults);
+        assert(resultsEq(projectResults, extractAValues(expected)), projectResults);
     }());
 
     // Test the results of similar dotted queries with an $elemMatch. These should have no
@@ -583,7 +645,7 @@ function testNotEqualsNullSemantics() {
 }
 
 // Test without any indexes.
-testNotEqualsNullSemantics(coll);
+testNullSemantics(coll);
 
 const keyPatterns = [
     {keyPattern: {a: 1}},
@@ -605,7 +667,7 @@ for (let indexSpec of keyPatterns) {
     coll.drop();
     jsTestLog(`Index spec: ${tojson(indexSpec)}`);
     assert.commandWorked(coll.createIndex(indexSpec.keyPattern, indexSpec.options));
-    testNotEqualsNullSemantics(coll);
+    testNullSemantics(coll);
 }
 
 // Test that you cannot use a $ne: null predicate in a partial filter expression.
diff --git a/jstests/core/or_to_in.js b/jstests/core/or_to_in.js
index 3774c88ebe3..2dfb893776f 100644
--- a/jstests/core/or_to_in.js
+++ b/jstests/core/or_to_in.js
@@ -79,7 +79,7 @@ const positiveTestQueries = [
     [{$or: [{f1: 42}, {f1: NaN}, {f1: 99}]}, {f1: {$in: [42, NaN, 99]}}],
     [{$or: [{f1: /^x/}, {f1: "ab"}]}, {f1: {$in: [/^x/, "ab"]}}],
     [{$or: [{f1: /^x/}, {f1: "^a"}]}, {f1: {$in: [/^x/, "^a"]}}],
-    [{$or: [{f1: 42}, {f1: null}, {f1: 99}]}, {$or: [{f1: {$in: [42, 99]}}, {f1: null}]}],
+    [{$or: [{f1: 42}, {f1: null}, {f1: 99}]}, {f1: {$in: [42, 99, null]}}],
     [{$or: [{f1: 1}, {f2: 9}, {f1: 99}]}, {$or: [{f2: 9}, {f1: {$in: [1, 99]}}]}],
     [{$or: [{f1: {$regex: /^x/}}, {f1: {$regex: /ab/}}]}, {f1: {$in: [/^x/, /ab/]}}],
     [
@@ -96,15 +96,15 @@ const positiveTestQueries = [
     ],
     [{$or: [{f2: [32, 52]}, {f2: [42, [13, 11]]}]}, {f2: {$in: [[32, 52], [42, [13, 11]]]}}],
     [{$or: [{f2: 52}, {f2: 13}]}, {f2: {$in: [52, 13]}}],
-    [{$or: [{f2: [11]}, {f2: [23]}]}, {f2: {$in: [[11], [23]]}}]
+    [{$or: [{f2: [11]}, {f2: [23]}]}, {f2: {$in: [[11], [23]]}}],
+    {$or: [{f1: 42}, {f1: null}]},
 ];
 
 // These $or queries should not be rewritten into $in because of different semantics.
 const negativeTestQueries = [
     {$or: [{f2: 9}, {f1: 1}, {f1: 99}]},
-    {$or: [{f1: 42}, {f1: null}]},
     {$or: [{f1: {$eq: /^x/}}, {f1: {$eq: /ab/}}]},
-    {$or: [{f1: {$lt: 2}}, {f1: {$gt: 3}}]}
+    {$or: [{f1: {$lt: 2}}, {f1: {$gt: 3}}]},
 ];
 
 for (const query of negativeTestQueries) {
diff --git a/src/mongo/db/matcher/expression_leaf.cpp b/src/mongo/db/matcher/expression_leaf.cpp
index 25280408af2..66abea54ccf 100644
--- a/src/mongo/db/matcher/expression_leaf.cpp
+++ b/src/mongo/db/matcher/expression_leaf.cpp
@@ -403,7 +403,9 @@ bool InMatchExpression::contains(const BSONElement& e) const {
 }
 
 bool InMatchExpression::matchesSingleElement(const BSONElement& e, MatchDetails* details) const {
-    if (_hasNull && e.eoo()) {
+    // When an $in has a null, it adopts the same semantics as {$eq:null}. Namely, in addition to
+    // matching literal null values, the $in should match missing and undefined.
+    if (_hasNull && (e.eoo() || e.type() == BSONType::Undefined)) {
         return true;
     }
     if (contains(e)) {
diff --git a/src/mongo/db/matcher/expression_leaf_test.cpp b/src/mongo/db/matcher/expression_leaf_test.cpp
index 8ee1d50cb1b..209a6b80225 100644
--- a/src/mongo/db/matcher/expression_leaf_test.cpp
+++ b/src/mongo/db/matcher/expression_leaf_test.cpp
@@ -134,11 +134,14 @@ TEST(EqOp, MatchesReferencedArrayValue) {
 TEST(EqOp, MatchesNull) {
     BSONObj operand = BSON("a" << BSONNULL);
     EqualityMatchExpression eq("a", operand["a"]);
-    ASSERT(eq.matchesBSON(BSONObj(), nullptr));
-    ASSERT(eq.matchesBSON(BSON("a" << BSONNULL), nullptr));
-    ASSERT(!eq.matchesBSON(BSON("a" << 4), nullptr));
-    // A non-existent field is treated same way as an empty bson object
-    ASSERT(eq.matchesBSON(BSON("b" << 4), nullptr));
+    ASSERT_TRUE(eq.matchesBSON(BSONObj(), nullptr));
+    ASSERT_TRUE(eq.matchesBSON(BSON("a" << BSONNULL), nullptr));
+    ASSERT_FALSE(eq.matchesBSON(BSON("a" << 4), nullptr));
+
+    // {$eq:null} has special semantics which say that both missing and undefined match, in addition
+    // to literal nulls.
+    ASSERT_TRUE(eq.matchesBSON(BSON("b" << 4), nullptr));
+    ASSERT_TRUE(eq.matchesBSON(BSON("a" << BSONUndefined), nullptr));
 }
 
 // This test documents how the matcher currently works,
@@ -1192,11 +1195,14 @@ TEST(InMatchExpression, MatchesNull) {
     std::vector<BSONElement> equalities{operand.firstElement()};
     ASSERT_OK(in.setEqualities(std::move(equalities)));
 
-    ASSERT(in.matchesBSON(BSONObj(), nullptr));
-    ASSERT(in.matchesBSON(BSON("a" << BSONNULL), nullptr));
-    ASSERT(!in.matchesBSON(BSON("a" << 4), nullptr));
-    // A non-existent field is treated same way as an empty bson object
-    ASSERT(in.matchesBSON(BSON("b" << 4), nullptr));
+    ASSERT_TRUE(in.matchesBSON(BSONObj(), nullptr));
+    ASSERT_TRUE(in.matchesBSON(BSON("a" << BSONNULL), nullptr));
+    ASSERT_FALSE(in.matchesBSON(BSON("a" << 4), nullptr));
+
+    // When null appears inside an $in, it has the same special semantics as an {$eq:null}
+    // predicate. In particular, we expect it to match both missing and undefined.
+    ASSERT_TRUE(in.matchesBSON(BSON("b" << 4), nullptr));
+    ASSERT_TRUE(in.matchesBSON(BSON("a" << BSONUndefined), nullptr));
 }
 
 TEST(InMatchExpression, MatchesUndefined) {
diff --git a/src/mongo/db/matcher/expression_optimize_test.cpp b/src/mongo/db/matcher/expression_optimize_test.cpp
index ab9e91cb30f..eaa606e06df 100644
--- a/src/mongo/db/matcher/expression_optimize_test.cpp
+++ b/src/mongo/db/matcher/expression_optimize_test.cpp
@@ -460,8 +460,7 @@ TEST(ExpressionOptimizeTest, OrRewrittenToIn) {
         {"{$or: [{f1: 42}, {f1: NaN}, {f1: 99}]}", "{ f1: { $in: [ NaN, 42, 99 ] } }"},
         {"{$or: [{f1: /^x/}, {f1:'ab'}]}", "{ f1: { $in: [ \"ab\", /^x/ ] } }"},
         {"{$or: [{f1: /^x/}, {f1:'^a'}]}", "{ f1: { $in: [ \"^a\", /^x/ ] } }"},
-        {"{$or: [{f1: 42}, {f1: null}, {f1: 99}]}",
-         "{ $or: [ { f1: { $in: [ 42, 99 ] } }, { f1: { $eq: null } } ] }"},
+        {"{$or: [{f1: 42}, {f1: null}, {f1: 99}]}", "{ f1: { $in: [ null, 42, 99 ] } }"},
         {"{$or: [{f1: 1}, {f2: 9}, {f1: 99}]}",
          "{ $or: [ { f1: { $in: [ 1, 99 ] } }, { f2: { $eq: 9 } } ] }"},
         {"{$and: [{$or: [{f1: 7}, {f1: 3}, {f1: 5}]}, {$or: [{f1: 1}, {f1: 2}, {f1: 3}]}]}",
@@ -471,6 +470,7 @@ TEST(ExpressionOptimizeTest, OrRewrittenToIn) {
         {"{$or: [{$and: [{f1: 7}, {f2: 7}, {f1: 5}]}, {$or: [{f1: 1}, {f1: 2}, {f1: 3}]}]}",
          "{ $or: [ { $and: [ { f1: { $eq: 7 } }, { f2: { $eq: 7 } }, { f1: { $eq: 5 } } ] },"
          " { f1: { $in: [ 1, 2, 3 ] } } ] }"},
+        {"{$or: [{$and: [ { f1: null } ] } ] }", "{ f1: { $eq: null } }"},
     };
 
     auto optimizeExpr = [](std::string exprStr) {
@@ -492,6 +492,7 @@ TEST(ExpressionOptimizeTest, OrRewrittenToIn) {
     ASSERT_BSONOBJ_EQ(optimizeExpr(queries[7].first), fromjson(queries[7].second));
     ASSERT_BSONOBJ_EQ(optimizeExpr(queries[8].first), fromjson(queries[8].second));
     ASSERT_BSONOBJ_EQ(optimizeExpr(queries[9].first), fromjson(queries[9].second));
+    ASSERT_BSONOBJ_EQ(optimizeExpr(queries[10].first), fromjson(queries[10].second));
 }
 
 }  // namespace
diff --git a/src/mongo/db/matcher/expression_tree.cpp b/src/mongo/db/matcher/expression_tree.cpp
index ec7a5e08dd0..c9cdf026700 100644
--- a/src/mongo/db/matcher/expression_tree.cpp
+++ b/src/mongo/db/matcher/expression_tree.cpp
@@ -162,9 +162,7 @@ MatchExpression::ExpressionOptimizerFunc ListOfMatchExpression::getOptimizer() c
             boost::optional<std::string> childPath;
             const CollatorInterface* eqCollator = nullptr;
 
-            auto isNullOrRegEx = [](const BSONElement& elm) {
-                return (elm.isNull() || elm.type() == BSONType::RegEx);
-            };
+            auto isRegEx = [](const BSONElement& elm) { return elm.type() == BSONType::RegEx; };
 
             // Check if all children are equality conditions or regular expressions with the
             // same path argument, and same collation.
@@ -175,22 +173,19 @@ MatchExpression::ExpressionOptimizerFunc ListOfMatchExpression::getOptimizer() c
                     continue;
                 }
 
-                // Disjunctions of equalities use $eq comparison, which has different semantics
-                // from $in equality comparison in two cases:
-                // (1) ('null' $eq 'undefined' = true), while ('null' $in 'undefined' = false),
-                //     that is, when comparing a 'null' argument to an 'undefined' value, the
-                //     result is different for $eq vs $in.
-                // (2) the regex under the equality is matched literally as a string constant,
-                //     while a regex inside $in is matched as a regular expression.
-                //     $lookup processing explicitly depends on this different semantics.
-                // Both these cases should not be rewritten into $in because of the different
-                // comparison semantics.
+                // Disjunctions of equalities use $eq comparison, which has different semantics from
+                // $in for regular expressions. The regex under the equality is matched literally as
+                // a string constant, while a regex inside $in is matched as a regular expression.
+                // Furthermore, $lookup processing explicitly depends on these different semantics.
+                //
+                // We should not attempt to rewrite an $eq:<regex> into $in because of these
+                // different comparison semantics.
                 const CollatorInterface* curCollator = nullptr;
                 if (childExpression->matchType() == MatchExpression::EQ) {
                     auto eqExpression =
                         static_cast<EqualityMatchExpression*>(childExpression.get());
                     curCollator = eqExpression->getCollator();
-                    if (isNullOrRegEx(eqExpression->getData())) {
+                    if (isRegEx(eqExpression->getData())) {
                         ++countNonEquivExpr;
                         continue;
                     }
@@ -211,14 +206,14 @@ MatchExpression::ExpressionOptimizerFunc ListOfMatchExpression::getOptimizer() c
                 }
             }
             tassert(3401201,
-                    "All expressions must be classified as either eq-equiv or non-eq-equiv.",
+                    "All expressions must be classified as either eq-equiv or non-eq-equiv",
                     countEquivEqPaths + countNonEquivExpr == children.size());
 
             // The condition above checks that there are at least two equalities that can be
-            // rewritten to an $in, and the we have classified all $or conditions into two disjunct
+            // rewritten to an $in, and the we have classified all $or conditions into two disjoint
             // groups.
             if (countEquivEqPaths > 1) {
-                tassert(3401202, "There must be a common path.", childPath);
+                tassert(3401202, "There must be a common path", childPath);
                 auto inExpression = std::make_unique<InMatchExpression>(StringData(*childPath));
                 auto nonEquivOrExpr =
                     (countNonEquivExpr > 0) ? std::make_unique<OrMatchExpression>() : nullptr;
@@ -230,7 +225,7 @@ MatchExpression::ExpressionOptimizerFunc ListOfMatchExpression::getOptimizer() c
                     } else if (childExpression->matchType() == MatchExpression::EQ) {
                         std::unique_ptr<EqualityMatchExpression> eqExpressionPtr{
                             static_cast<EqualityMatchExpression*>(childExpression.release())};
-                        if (isNullOrRegEx(eqExpressionPtr->getData()) ||
+                        if (isRegEx(eqExpressionPtr->getData()) ||
                             eqExpressionPtr->getCollator() != eqCollator) {
                             nonEquivOrExpr->add(std::move(eqExpressionPtr));
                         } else {
@@ -244,7 +239,7 @@ MatchExpression::ExpressionOptimizerFunc ListOfMatchExpression::getOptimizer() c
                         regexExpressionPtr->setPath({});
                         auto status = inExpression->addRegex(std::move(regexExpressionPtr));
                         tassert(3401203,  // TODO SERVER-53380 convert to tassertStatusOK.
-                                "Conversion from OR to IN should always succeed.",
+                                "Conversion from OR to IN should always succeed",
                                 status == Status::OK());
                     } else {
                         nonEquivOrExpr->add(std::move(childExpression));
@@ -266,7 +261,7 @@ MatchExpression::ExpressionOptimizerFunc ListOfMatchExpression::getOptimizer() c
 
                 auto status = inExpression->setEqualities(std::move(inEqualities));
                 tassert(3401206,  // TODO SERVER-53380 convert to tassertStatusOK.
-                        "Conversion from OR to IN should always succeed.",
+                        "Conversion from OR to IN should always succeed",
                         status == Status::OK());
 
                 inExpression->setBackingBSON(std::move(backingArr));