SERVER-71525: Removed failOnPoisonedFieldLookup fail point and associated tests relying on it.

Performs an invalid operation (divide-by-zero) instead of a "POISON" field lookup for short-circuit testing

5 files changed, 55 insertions, 87 deletions

diff --git a/jstests/aggregation/expressions/filter.js b/jstests/aggregation/expressions/filter.js
index 2e7abe6b0cb..2f7014a6706 100644
--- a/jstests/aggregation/expressions/filter.js
+++ b/jstests/aggregation/expressions/filter.js
@@ -1,9 +1,4 @@
 // Test $filter aggregation expression.
-//
-// @tags: [
-//   # Can't set the 'failOnPoisonedFieldLookup' failpoint on mongos.
-//   assumes_against_mongod_not_mongos,
-// ]
 
 load('jstests/aggregation/extras/utils.js');        // For assertErrorCode.
 load("jstests/libs/sbe_assert_error_override.js");  // Override error-code-checking APIs.
@@ -405,29 +400,24 @@ runAndAssert(filterDoc, expectedResults);
 
 // Test short-circuiting in $and and $or inside $filter expression.
 coll.drop();
-assert.commandWorked(coll.insert({_id: 0, a: [-1, -2, -3, -4]}));
-try {
-    // Lookup of '$POISON' field will always fail with this fail point enabled.
-    assert.commandWorked(
-        db.adminCommand({configureFailPoint: "failOnPoisonedFieldLookup", mode: "alwaysOn"}));
-
-    // Create filter with $and expression containing '$POISON' in it.
-    expectedResults = [
-        {_id: 0, b: []},
-    ];
-    filterDoc = {$filter: {input: '$a', cond: {$and: [{$gt: ['$$this', 0]}, '$POISON']}}};
-    runAndAssert(filterDoc, expectedResults);
-
-    // Create filter with $or expression containing '$POISON' in it.
-    expectedResults = [
-        {_id: 0, b: [-1, -2, -3, -4]},
-    ];
-    filterDoc = {$filter: {input: '$a', cond: {$or: [{$lt: ['$$this', 0]}, '$POISON']}}};
-    runAndAssert(filterDoc, expectedResults);
-} finally {
-    assert.commandWorked(
-        db.adminCommand({configureFailPoint: "failOnPoisonedFieldLookup", mode: "off"}));
-}
+assert.commandWorked(coll.insert({_id: 0, a: [-1, -2, -3, -4], zero: 0}));
+// Create filter with $and expression containing $divide by zero operation in it.
+expectedResults = [
+    {_id: 0, b: []},
+];
+filterDoc = {
+    $filter: {input: '$a', cond: {$and: [{$gt: ['$$this', 0]}, {$divide: [1, '$zero']}]}}
+};
+runAndAssert(filterDoc, expectedResults);
+
+// Create filter with $or expression containing $divide by zero operation in it.
+expectedResults = [
+    {_id: 0, b: [-1, -2, -3, -4]},
+];
+filterDoc = {
+    $filter: {input: '$a', cond: {$or: [{$lte: ['$$this', 0]}, {$divide: [1, '$zero']}]}}
+};
+runAndAssert(filterDoc, expectedResults);
 
 // Create filter with $and expression containing invalid call to $ln in it.
 expectedResults = [
diff --git a/jstests/core/computed_projections.js b/jstests/core/computed_projections.js
index fd432771f44..9dd5eb7cab5 100644
--- a/jstests/core/computed_projections.js
+++ b/jstests/core/computed_projections.js
@@ -4,21 +4,10 @@
 load("jstests/aggregation/extras/utils.js");  // For arrayEq and orderedArrayEq.
 load("jstests/libs/sbe_assert_error_override.js");
 
-// It is safe for other tests to run while this failpoint is active, so long as those tests do not
-// use documents containing a field with "POISON" as their name. Note that this command can fail.
-// This failpoint does not exist on mongos and on older version of mongod, which we may be talking
-// to in sharding and/or multi-version passthrough suites. The test still functions but with reduced
-// coverage.
-const isPoisonFailPointEnabled =
-    db.adminCommand({configureFailPoint: "failOnPoisonedFieldLookup", mode: "alwaysOn"}).ok;
-if (!isPoisonFailPointEnabled) {
-    print("Unable to set 'failOnPoisonedFieldLookup' failpoint.");
-}
-
 const coll = db.computed_projection;
 coll.drop();
 const documents = [
-    {_id: 0, a: 1, b: "x", c: 10},
+    {_id: 0, a: 1, b: "x", c: 10, zero: 0},
     {_id: 1, a: 2, b: "y", c: 11},
     {_id: 2, a: 3, b: "z", c: 12},
     {_id: 3, x: {y: 1}},
@@ -40,7 +29,17 @@ const documents = [
     {_id: 19, i: {j: 5}, k: {l: 10}},
     {_id: 20, x: [[{y: 1}, {y: 2}], {y: 3}, {y: 4}, [[[{y: 5}]]], {y: 6}]},
     {_id: 21, x: [[{y: {z: 1}}, {y: 2}], {y: 3}, {y: {z: 2}}, [[[{y: 5}, {y: {z: 3}}]]], {y: 6}]},
-    {_id: 22, tf: [true, false], ff: [false, false], t: true, f: false, n: null, a: 1, b: 0},
+    {
+        _id: 22,
+        tf: [true, false],
+        ff: [false, false],
+        t: true,
+        f: false,
+        n: null,
+        a: 1,
+        b: 0,
+        zero: 0
+    },
     {_id: 23, i1: NumberInt(1), i2: NumberInt(-1), i3: NumberInt(-2147483648)},
     {_id: 24, l1: NumberLong("12345678900"), l2: NumberLong("-12345678900")},
     {_id: 25, s: "string", l: NumberLong("-9223372036854775808"), n: null},
@@ -646,35 +645,37 @@ const testCases = [
     },
     //
     // Test that short-circuited branches do not do any work, such as traversing an array. The
-    // 'failOnPoisonedFieldLookup' failpoint ensures that none of the "$POISON" lookups, which are
-    // in short-circuited branches, ever execute. If they were to execute, it would result in an
-    // error from the query that would cause the test to fail.
+    // short-circuited branches cointain divide by zero operation which if it were to execute
+    // would result in an error from the query that would cause the test to fail.
     //
     {
-        desc: "$POISON in short-circuited $and/$or branches",
+        desc: "$divide by zero in short-circuited $and/$or branches",
         expected: [{_id: 22, foo: false, bar: true}],
         query: {_id: 22, a: 1},
-        proj: {foo: {$and: ["$f", "$POISON"]}, bar: {$or: ["$t", "$POISON"]}}
+        proj: {
+            foo: {$and: ["$f", {$divide: [1, "$zero"]}]},
+            bar: {$or: ["$t", {$divide: [1, "$zero"]}]}
+        }
     },
     {
-        desc: "$POISON in nested short-circuited $or branches",
+        desc: "$divide by zero in nested short-circuited $or branches",
         expected: [{_id: 22, foo: false, bar: true}],
         query: {_id: 22, a: 1},
         proj: {
-            foo: {$and: ["$f", {$or: ["$f", "$POISON"]}, {$eq: ["$a", 1]}]},
-            bar: {$and: ["$t", {$or: ["$t", "$POISON"]}, {$eq: ["$a", 1]}]}
+            foo: {$and: ["$f", {$or: ["$f", {$divide: [1, "$zero"]}]}, {$eq: ["$a", 1]}]},
+            bar: {$and: ["$t", {$or: ["$t", {$divide: [1, "$zero"]}]}, {$eq: ["$a", 1]}]}
         }
     },
     {
-        desc: "$POISON in untaken $switch cases",
+        desc: "$divide by zero in untaken $switch cases",
         expected: [{_id: 0, foo: "x"}, {_id: 22, foo: 0}],
         query: {a: 1},
         proj: {
             foo: {
                 $switch: {
                     branches: [
-                        {case: {$eq: ["$a", 2]}, then: "$POISON"},
-                        {case: {$eq: ["$a", 3]}, then: "$POISON"}
+                        {case: {$eq: ["$a", 2]}, then: {$divide: [1, "$zero"]}},
+                        {case: {$eq: ["$a", 3]}, then: {$divide: [1, "$zero"]}}
                     ],
                     default: "$b"
                 }
@@ -682,30 +683,32 @@ const testCases = [
         }
     },
     {
-        desc: "$POISON in unevaluated case condition and untaken $switch default branch",
+        desc: "$divide by zero in unevaluated case condition and untaken $switch default branch",
         expected: [{_id: 0, foo: "x"}, {_id: 22, foo: 0}],
         query: {a: 1},
         proj: {
             foo: {
                 $switch: {
-                    branches:
-                        [{case: {$eq: ["$a", 1]}, then: "$b"}, {case: "$POISON", then: "$POISON"}],
-                    default: "$POISON"
+                    branches: [
+                        {case: {$eq: ["$a", 1]}, then: "$b"},
+                        {case: {$divide: [1, "$zero"]}, then: {$divide: [1, "$zero"]}}
+                    ],
+                    default: {$divide: [1, "$zero"]}
                 }
             }
         }
     },
     {
-        desc: "$POISON in untaken $cond branch (else)",
+        desc: "$divide by zero in untaken $cond branch (else)",
         expected: [{_id: 0, foo: "x"}, {_id: 22, foo: 0}],
         query: {a: 1},
-        proj: {foo: {$cond: {if: {$eq: ["$a", 1]}, then: "$b", else: "$POISON"}}}
+        proj: {foo: {$cond: {if: {$eq: ["$a", 1]}, then: "$b", else: {$divide: [1, "$zero"]}}}}
     },
     {
-        desc: "$POISON in untaken $cond branch (then)",
+        desc: "$divide by zero in untaken $cond branch (then)",
         expected: [{_id: 0, foo: "x"}, {_id: 22, foo: 0}],
         query: {a: 1},
-        proj: {foo: {$cond: {if: {$eq: ["$a", 2]}, then: "$POISON", else: "$b"}}}
+        proj: {foo: {$cond: {if: {$eq: ["$a", 2]}, then: {$divide: [1, "$zero"]}, else: "$b"}}}
     },
     //
     // $let tests.
@@ -1115,8 +1118,4 @@ testCases.forEach(function(test) {
             result, test.expectedErrorCode, Object.assign({result: result}, test));
     }
 });
-
-if (isPoisonFailPointEnabled) {
-    db.adminCommand({configureFailPoint: "failOnPoisonedFieldLookup", mode: "off"});
-}
 }());
diff --git a/src/mongo/db/exec/sbe/vm/vm.cpp b/src/mongo/db/exec/sbe/vm/vm.cpp
index 35dab757964..99464310d2f 100644
--- a/src/mongo/db/exec/sbe/vm/vm.cpp
+++ b/src/mongo/db/exec/sbe/vm/vm.cpp
@@ -64,9 +64,6 @@
 
 #define MONGO_LOGV2_DEFAULT_COMPONENT ::mongo::logv2::LogComponent::kQuery
 
-
-MONGO_FAIL_POINT_DEFINE(failOnPoisonedFieldLookup);
-
 namespace mongo {
 namespace sbe {
 namespace vm {
@@ -962,10 +959,6 @@ FastTuple<bool, value::TypeTags, value::Value> ByteCode::getField(value::TypeTag
 FastTuple<bool, value::TypeTags, value::Value> ByteCode::getField(value::TypeTags objTag,
                                                                   value::Value objValue,
                                                                   StringData fieldStr) {
-    if (MONGO_unlikely(failOnPoisonedFieldLookup.shouldFail())) {
-        uassert(4623399, "Lookup of $POISON", fieldStr != "POISON");
-    }
-
     if (objTag == value::TypeTags::Object) {
         auto [tag, val] = value::getObjectView(objValue)->getField(fieldStr);
         return {false, tag, val};
diff --git a/src/mongo/db/query/sbe_stage_builder.cpp b/src/mongo/db/query/sbe_stage_builder.cpp
index 8fe87acf72b..cc8b4c7fa7c 100644
--- a/src/mongo/db/query/sbe_stage_builder.cpp
+++ b/src/mongo/db/query/sbe_stage_builder.cpp
@@ -2438,9 +2438,8 @@ std::pair<std::unique_ptr<sbe::PlanStage>, PlanStageSlots> SlotBasedStageBuilder
     }
 
     if (!groupNode->needWholeDocument) {
-        // Tracks whether we need to request kResult. One such case is lookup of the '$$POISON'
-        // field.
-        bool rootDocIsNeeded = containsPoisonTopLevelField(groupNode->requiredFields);
+        // Tracks whether we need to request kResult.
+        bool rootDocIsNeeded = false;
         auto referencesRoot = [&](const ExpressionFieldPath* fieldExpr) {
             rootDocIsNeeded = rootDocIsNeeded || fieldExpr->isROOT();
         };
diff --git a/src/mongo/db/query/sbe_stage_builder_helpers.h b/src/mongo/db/query/sbe_stage_builder_helpers.h
index acf111d5675..3ee2d1a2b35 100644
--- a/src/mongo/db/query/sbe_stage_builder_helpers.h
+++ b/src/mongo/db/query/sbe_stage_builder_helpers.h
@@ -1113,18 +1113,11 @@ inline StringData getTopLevelField(const T& path) {
 
 template <typename T>
 inline std::vector<std::string> getTopLevelFields(const T& setOfPaths) {
-    const bool testCommandsEnabled = getTestCommandsEnabled();
     std::vector<std::string> topLevelFields;
     StringSet topLevelFieldsSet;
 
     for (const auto& path : setOfPaths) {
         auto field = getTopLevelField(path);
-        // If test commands are enabled, then we need to skip any top-level field named
-        // "POISON" so that the "failOnPoisonedFieldLookup" fail point works correctly.
-        if (testCommandsEnabled && field == "POISON") {
-            continue;
-        }
-
         if (!topLevelFieldsSet.count(field)) {
             topLevelFields.emplace_back(std::string(field));
             topLevelFieldsSet.emplace(std::string(field));
@@ -1134,12 +1127,6 @@ inline std::vector<std::string> getTopLevelFields(const T& setOfPaths) {
     return topLevelFields;
 }
 
-template <typename T>
-inline bool containsPoisonTopLevelField(const T& setOfPaths) {
-    auto it = setOfPaths.lower_bound("POISON");
-    return getTestCommandsEnabled() && it != setOfPaths.end() && getTopLevelField(*it) == "POISON";
-}
-
 template <typename T, typename FuncT>
 inline std::vector<T> filterVector(std::vector<T> vec, FuncT fn) {
     std::vector<T> result;