mirrors/mongodb
mirrors/mongodb
0
0
Fork 0
mirror of https://github.com/mongodb/mongo.git synced 2024-12-01 09:32:32 +01:00
Code Issues Releases Activity

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

Browse Source 
Performs an invalid operation (divide-by-zero) instead of a "POISON" field lookup for short-circuit testing
This commit is contained in:
Projjal Chanda 2023-02-09 11:57:09 +00:00 committed by Evergreen Agent
parent c136914675
commit a579f1d19e
5 changed files with 58 additions and 90 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
jstests/aggregation/expressions/filter.js
View File

@ -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"}));
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 $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"}));
}
// 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 = [

79
jstests/core/computed_projections.js
View File

@ -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",
expected: [{_id: 22, foo: false, bar: true}],
query: {_id: 22, a: 1},
proj: {foo: {$and: ["$f", "$POISON"]}, bar: {$or: ["$t", "$POISON"]}}
},
{
desc: "$POISON in nested short-circuited $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", {$or: ["$f", "$POISON"]}, {$eq: ["$a", 1]}]},
bar: {$and: ["$t", {$or: ["$t", "$POISON"]}, {$eq: ["$a", 1]}]}
foo: {$and: ["$f", {$divide: [1, "$zero"]}]},
bar: {$or: ["$t", {$divide: [1, "$zero"]}]}
}
},
{
desc: "$POISON in untaken $switch cases",
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", {$divide: [1, "$zero"]}]}, {$eq: ["$a", 1]}]},
bar: {$and: ["$t", {$or: ["$t", {$divide: [1, "$zero"]}]}, {$eq: ["$a", 1]}]}
}
},
{
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"});
}
}());

7
src/mongo/db/exec/sbe/vm/vm.cpp
View File

@ -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};

5
src/mongo/db/query/sbe_stage_builder.cpp
View File

@ -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();
};

13
src/mongo/db/query/sbe_stage_builder_helpers.h
View File

@ -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;