SERVER-38949 ban index usage for {$ne: <array>} queries

jstests/core/ne_array.js

@@ -0,0 +1,65 @@
+// This is a test for the query correctness bug described in SERVER-38949. A {$ne: <array>} query
+// cannot "naively" use an index. That is, it cannot use an index by simply generating bounds for
+// {$eq: <array>} query and then complementing them. This test checks that the correct results are
+// returned for this type of query when an index is present.
+// @tags: [requires_non_retryable_writes]
+(function() {
+    const coll = db.ne_array;
+    coll.drop();
+    assert.commandWorked(coll.createIndex({a: 1}));
+
+    assert.commandWorked(coll.insert({_id: 0, a: [1]}));
+    assert.commandWorked(coll.insert({_id: 1, a: [1, 3]}));
+
+    assert.eq(coll.find({a: {$ne: [1, 3]}}, {_id: 1}).toArray(), [{_id: 0}]);
+    assert.eq(coll.find({a: {$ne: [1]}}, {_id: 1}).toArray(), [{_id: 1}]);
+
+    assert.eq(coll.find({a: {$not: {$in: [[1]]}}}, {_id: 1}).toArray(), [{_id: 1}]);
+    assert.eq(coll.find({a: {$not: {$in: [[1, 3]]}}}, {_id: 1}).toArray(), [{_id: 0}]);
+    assert.eq(coll.find({a: {$not: {$in: [[1], [1, 3]]}}}, {_id: 1}).toArray(), []);
+    assert.eq(coll.find({a: {$not: {$in: ["scalar value", [1, 3]]}}}, {_id: 1}).toArray(),
+              [{_id: 0}]);
+
+    // Insert some documents which have nested arrays so we can test $elemMatch value.
+    assert.commandWorked(coll.remove({}));
+    assert.commandWorked(coll.insert({_id: 0, a: [[123]]}));
+    assert.commandWorked(coll.insert({_id: 1, a: [4, 5, [123]]}));
+    assert.commandWorked(coll.insert({_id: 2, a: [7, 8]}));
+
+    // sort by _id in case we run on a sharded cluster which puts the documents on different
+    // shards (and thus, might return them in any order).
+    assert.eq(coll.find({a: {$elemMatch: {$not: {$eq: [123]}}}}, {_id: 1}).sort({_id: 1}).toArray(),
+              [{_id: 1}, {_id: 2}]);
+
+    assert.eq(
+        coll.find({a: {$elemMatch: {$not: {$in: [[123]]}}}}, {_id: 1}).sort({_id: 1}).toArray(),
+        [{_id: 1}, {_id: 2}]);
+
+    assert.eq(coll.find({a: {$not: {$elemMatch: {$eq: [123]}}}}, {_id: 1}).toArray(), [{_id: 2}]);
+    assert.eq(coll.find({a: {$not: {$elemMatch: {$in: [[123]]}}}}, {_id: 1}).toArray(), [{_id: 2}]);
+
+    // Test $elemMatch object.
+    assert.commandWorked(coll.remove({}));
+    coll.dropIndexes();
+    assert.commandWorked(coll.createIndex({"a.b": 1}));
+    assert.commandWorked(coll.insert({_id: 0, a: [[123]]}));
+    assert.commandWorked(coll.insert({_id: 1, a: [{b: 123}]}));
+    assert.commandWorked(coll.insert({_id: 2, a: [{b: [4, [123]]}]}));
+    assert.commandWorked(coll.insert({_id: 3, a: [{b: [[123]]}]}));
+
+    // Remember that $ne with an array will match arrays where _none_ of the elements match.
+    assert.eq(coll.find({a: {$elemMatch: {b: {$ne: [123]}}}}, {_id: 1}).sort({_id: 1}).toArray(),
+              [{_id: 0}, {_id: 1}]);
+    assert.eq(coll.find({a: {$elemMatch: {b: {$not: {$in: [[123]]}}}}}, {_id: 1})
+                  .sort({_id: 1})
+                  .toArray(),
+              [{_id: 0}, {_id: 1}]);
+
+    assert.eq(coll.find({a: {$not: {$elemMatch: {b: [123]}}}}, {_id: 1}).sort({_id: 1}).toArray(),
+              [{_id: 0}, {_id: 1}]);
+    assert.eq(coll.find({a: {$not: {$elemMatch: {b: {$in: [[123]]}}}}}, {_id: 1})
+                  .sort({_id: 1})
+                  .toArray(),
+              [{_id: 0}, {_id: 1}]);
+
+})();

src/mongo/db/query/index_bounds_builder.cpp

@@ -426,6 +426,8 @@ void IndexBoundsBuilder::_translatePredicate(const MatchExpression* expr,
             *tightnessOut = IndexBoundsBuilder::EXACT;
         }
 
+        invariant(*tightnessOut == IndexBoundsBuilder::EXACT);
+
         // If the index is multikey on this path, it doesn't matter what the tightness of the child
         // is, we must return INEXACT_FETCH. Consider a multikey index on 'a' with document
         // {a: [1, 2, 3]} and query {a: {$ne: 3}}. If we treated the bounds [MinKey, 3), (3, MaxKey]

src/mongo/db/query/planner_ixselect.cpp

@@ -55,6 +55,21 @@ namespace {
 
 namespace wcp = ::mongo::wildcard_planning;
 
+bool isNotEqualsArrayOrNotInArray(const MatchExpression* me) {
+    const auto type = me->matchType();
+    if (type == MatchExpression::EQ) {
+        return static_cast<const ComparisonMatchExpression*>(me)->getData().type() ==
+            BSONType::Array;
+    } else if (type == MatchExpression::MATCH_IN) {
+        const auto& equalities = static_cast<const InMatchExpression*>(me)->getEqualities();
+        return std::any_of(equalities.begin(), equalities.end(), [](BSONElement elt) {
+            return elt.type() == BSONType::Array;
+        });
+    }
+
+    return false;
+}
+
 std::size_t numPathComponents(StringData path) {
     return FieldRef{path}.numParts();
 }
@@ -391,7 +406,7 @@ bool QueryPlannerIXSelect::_compatible(const BSONElement& keyPatternElt,
             }
 
             const auto* child = node->getChild(0);
-            MatchExpression::MatchType childtype = child->matchType();
+            const MatchExpression::MatchType childtype = child->matchType();
             const bool isNotEqualsNull =
                 (childtype == MatchExpression::EQ &&
                  static_cast<const ComparisonMatchExpression*>(child)->getData().type() ==
@@ -418,6 +433,12 @@ bool QueryPlannerIXSelect::_compatible(const BSONElement& keyPatternElt,
                 return false;
             }
 
+            // Can't index negations of {$eq: <Array>} or {$in: [<Array>, ...]}. Note that we could
+            // use the index in principle, though we would need to generate special bounds.
+            if (isNotEqualsArrayOrNotInArray(child)) {
+                return false;
+            }
+
             // Most of the time we can't use a multikey index for a $ne: null query, however there
             // are a few exceptions around $elemMatch.
             if (isNotEqualsNull &&

src/mongo/db/query/query_planner_test.cpp

@@ -3017,6 +3017,71 @@ TEST_F(QueryPlannerTest, NegationElemMatchObject2) {
     assertSolutionExists("{cscan: {dir: 1}}");
 }
 
+// Negated $eq: <Array> won't use the index.
+TEST_F(QueryPlannerTest, NegationEqArray) {
+    addIndex(BSON("i" << 1));
+    runQuery(fromjson("{i: {$not: {$eq: [1, 2]}}}"));
+
+    assertHasOnlyCollscan();
+}
+
+// If we negate a $in and any of the members of the $in equalities
+// is an array, we don't use the index.
+TEST_F(QueryPlannerTest, NegationInArray) {
+    addIndex(BSON("i" << 1));
+    runQuery(fromjson("{i: {$not: {$in: [1, [1, 2]]}}}"));
+
+    assertHasOnlyCollscan();
+}
+
+TEST_F(QueryPlannerTest, ElemMatchValueNegationEqArray) {
+    addIndex(BSON("i" << 1));
+    runQuery(fromjson("{i: {$elemMatch: {$not: {$eq: [1]}}}}"));
+    assertHasOnlyCollscan();
+}
+
+TEST_F(QueryPlannerTest, ElemMatchValueNegationInArray) {
+    addIndex(BSON("i" << 1));
+    runQuery(fromjson("{i: {$elemMatch: {$not: {$in: [[1]]}}}}"));
+    assertHasOnlyCollscan();
+}
+
+TEST_F(QueryPlannerTest, NegatedElemMatchValueEqArray) {
+    addIndex(BSON("i" << 1));
+    runQuery(fromjson("{i: {$not: {$elemMatch: {$eq: [1]}}}}"));
+    assertHasOnlyCollscan();
+}
+
+TEST_F(QueryPlannerTest, NegatedElemMatchValueInArray) {
+    addIndex(BSON("i" << 1));
+    runQuery(fromjson("{i: {$not: {$elemMatch: {$in: [[1]]}}}}"));
+    assertHasOnlyCollscan();
+}
+
+TEST_F(QueryPlannerTest, ElemMatchObjectNegationEqArray) {
+    addIndex(BSON("i.j" << 1));
+    runQuery(fromjson("{i: {$elemMatch: {j: {$ne: [1]}}}}"));
+    assertHasOnlyCollscan();
+}
+
+TEST_F(QueryPlannerTest, ElemMatchObjectNegationInArray) {
+    addIndex(BSON("i.j" << 1));
+    runQuery(fromjson("{i: {$elemMatch: {j: {$not: {$in: [[1]]}}}}}"));
+    assertHasOnlyCollscan();
+}
+
+TEST_F(QueryPlannerTest, NegatedElemMatchObjectEqArray) {
+    addIndex(BSON("i.j" << 1));
+    runQuery(fromjson("{i: {$not: {$elemMatch: {j: [1]}}}}"));
+    assertHasOnlyCollscan();
+}
+
+TEST_F(QueryPlannerTest, NegatedElemMatchObjectInArray) {
+    addIndex(BSON("i.j" << 1));
+    runQuery(fromjson("{i: {$not: {$elemMatch: {j: {$in: [[1]]}}}}}"));
+    assertHasOnlyCollscan();
+}
+
 // If there is a negation that can't use the index,
 // ANDed with a predicate that can use the index, then
 // we can still use the index for the latter predicate.