diff options
59 files changed, 1438 insertions, 947 deletions
diff --git a/jstests/noPassthrough/telemetry/application_name_find.js b/jstests/noPassthrough/telemetry/application_name_find.js index e3c8ca30e60..31a04ca195c 100644 --- a/jstests/noPassthrough/telemetry/application_name_find.js +++ b/jstests/noPassthrough/telemetry/application_name_find.js @@ -45,7 +45,7 @@ assert.eq(1, telemetry.length, telemetry); assert.eq({ cmdNs: {db: testDB.getName(), coll: coll.getName()}, find: coll.getName(), - filter: {v: {"$eq": "?"}}, + filter: {v: {"$eq": "?number"}}, applicationName: kApplicationName }, telemetry[0].key, @@ -57,7 +57,7 @@ const hashedColl = "tU+RtrEU9QHrWsxNIL8OUDvfpUdavYkcuw7evPKfxdU="; assert.eq({ cmdNs: {db: "Q7DO+ZJl+eNMEOqdNQGSbSezn1fG1nRWHYuiNueoGfs=", coll: hashedColl}, find: hashedColl, - filter: {"ksdi13D4gc1BJ0Es4yX6QtG6MAwIeNLsCgeGRePOvFE=": {"$eq": "?"}}, + filter: {"ksdi13D4gc1BJ0Es4yX6QtG6MAwIeNLsCgeGRePOvFE=": {"$eq": "?number"}}, applicationName: kHashedApplicationName }, telemetry[0].key, diff --git a/jstests/noPassthrough/telemetry/telemetry_collect_on_mongos.js b/jstests/noPassthrough/telemetry/telemetry_collect_on_mongos.js index 5882ab529f0..1a68a7d43e3 100644 --- a/jstests/noPassthrough/telemetry/telemetry_collect_on_mongos.js +++ b/jstests/noPassthrough/telemetry/telemetry_collect_on_mongos.js @@ -10,8 +10,6 @@ load('jstests/libs/telemetry_utils.js'); // Redacted literal replacement string. This may change in the future, so it's factored out. const aggRedactString = "###"; -const findRedactString = "?"; - const setup = () => { const st = new ShardingTest({ mongos: 1, @@ -75,8 +73,8 @@ const assertExpectedResults = (results, const telemetryKey = { cmdNs: {db: "test", coll: "coll"}, find: collName, - filter: {$and: [{v: {$gt: findRedactString}}, {v: {$lt: findRedactString}}]}, - batchSize: findRedactString, + filter: {$and: [{v: {$gt: "?number"}}, {v: {$lt: "?number"}}]}, + batchSize: "?number", readConcern: {level: "local", provenance: "implicitDefault"}, applicationName: "MongoDB Shell", }; @@ -208,8 +206,8 @@ const assertExpectedResults = (results, const telemetryKey = { cmdNs: {db: "test", coll: "coll"}, find: collName, - filter: {$and: [{v: {$gt: findRedactString}}, {v: {$lt: findRedactString}}]}, - batchSize: findRedactString, + filter: {$and: [{v: {$gt: "?number"}}, {v: {$lt: "?number"}}]}, + batchSize: "?number", readConcern: {level: "local", provenance: "implicitDefault"}, applicationName: "MongoDB Shell" }; diff --git a/jstests/noPassthrough/telemetry/telemetry_metrics_across_getMore_calls.js b/jstests/noPassthrough/telemetry/telemetry_metrics_across_getMore_calls.js index 87fc54a3360..4543b96edf7 100644 --- a/jstests/noPassthrough/telemetry/telemetry_metrics_across_getMore_calls.js +++ b/jstests/noPassthrough/telemetry/telemetry_metrics_across_getMore_calls.js @@ -122,7 +122,7 @@ const fooNeBatchSize = 3; // This filters to just the telemetry for query coll.find({foo: {$eq: // 1}}).limit(query2Limit).batchSize(2). telemetryResults = testDB.getSiblingDB("admin") - .aggregate([{$telemetry: {}}, {$match: {"key.limit": '?'}}]) + .aggregate([{$telemetry: {}}, {$match: {"key.limit": '?number'}}]) .toArray(); assert.eq(telemetryResults.length, 1, telemetryResults); assert.eq(telemetryResults[0].key.cmdNs.db, "test"); @@ -137,7 +137,7 @@ const fooNeBatchSize = 3; {$telemetry: {}}, { $match: { - "key.filter.foo": {$eq: {$eq: "?"}}, + "key.filter.foo": {$eq: {$eq: "?number"}}, "key.limit": {$exists: false}, "key.sort": {$exists: false} } diff --git a/jstests/noPassthrough/telemetry/telemetry_redact_find_cmd.js b/jstests/noPassthrough/telemetry/telemetry_redact_find_cmd.js index 140dfd97418..9a0f847d9ad 100644 --- a/jstests/noPassthrough/telemetry/telemetry_redact_find_cmd.js +++ b/jstests/noPassthrough/telemetry/telemetry_redact_find_cmd.js @@ -22,7 +22,7 @@ function runTest(conn) { assert.eq(1, telemetry.length); assert.eq(kHashedCollName, telemetry[0].key.find); - assert.eq({[kHashedFieldName]: {$eq: "?"}}, telemetry[0].key.filter); + assert.eq({[kHashedFieldName]: {$eq: "?number"}}, telemetry[0].key.filter); db.test.insert({v: 2}); @@ -37,7 +37,9 @@ function runTest(conn) { telemetry = getTelemetryRedacted(admin); assert.eq(2, telemetry.length); assert.eq(kHashedCollName, telemetry[1].key.find); - assert.eq({"$and": [{[kHashedFieldName]: {"$gt": "?"}}, {[kHashedFieldName]: {"$lt": "?"}}]}, + assert.eq({ + "$and": [{[kHashedFieldName]: {"$gt": "?number"}}, {[kHashedFieldName]: {"$lt": "?number"}}] + }, telemetry[1].key.filter); } diff --git a/src/mongo/db/SConscript b/src/mongo/db/SConscript index 0f014dd8848..29ba3357afd 100644 --- a/src/mongo/db/SConscript +++ b/src/mongo/db/SConscript @@ -1609,6 +1609,7 @@ env.Library( 'pipeline/make_js_function.cpp', 'pipeline/monotonic_expression.cpp', 'pipeline/variables.cpp', + 'query/serialization_options.cpp', ], LIBDEPS=[ '$BUILD_DIR/mongo/bson/util/bson_extract', diff --git a/src/mongo/db/exec/document_value/value.h b/src/mongo/db/exec/document_value/value.h index ab2e2c02934..c3e3dc78546 100644 --- a/src/mongo/db/exec/document_value/value.h +++ b/src/mongo/db/exec/document_value/value.h @@ -435,13 +435,16 @@ public: ImplicitValue(T&& arg) : Value(std::forward<T>(arg)) {} ImplicitValue(std::initializer_list<ImplicitValue> values) : Value(convertToValues(values)) {} + ImplicitValue(std::vector<ImplicitValue> values) : Value(convertToValues(values)) {} - ImplicitValue(std::vector<int> values) : Value(convertToValues(values)) {} + template <typename T> + ImplicitValue(std::vector<T> values) : Value(convertToValues(values)) {} - static std::vector<Value> convertToValues(const std::vector<int>& vec) { + template <typename T> + static std::vector<Value> convertToValues(const std::vector<T>& vec) { std::vector<Value> values; values.reserve(vec.size()); - for_each(vec.begin(), vec.end(), ([&](const int& val) { values.emplace_back(val); })); + for_each(vec.begin(), vec.end(), ([&](const T& val) { values.emplace_back(val); })); return values; } diff --git a/src/mongo/db/exec/projection_executor_redaction_test.cpp b/src/mongo/db/exec/projection_executor_redaction_test.cpp index b7ff9f1059c..ed4fd5a7793 100644 --- a/src/mongo/db/exec/projection_executor_redaction_test.cpp +++ b/src/mongo/db/exec/projection_executor_redaction_test.cpp @@ -65,6 +65,7 @@ std::string redactFieldNameForTest(StringData s) { TEST(Redaction, ProjectionTest) { SerializationOptions options; options.replacementForLiteralArgs = "?"; + options.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; options.redactIdentifiers = true; options.identifierRedactionPolicy = redactFieldNameForTest; @@ -169,7 +170,7 @@ TEST(Redaction, ProjectionTest) { /// Add fields projection actual = redactProj("{a: \"hi\"}"); ASSERT_DOCUMENT_EQ_AUTO( // NOLINT - R"({"HASH<_id>":true,"HASH<a>":{"$const":"?"}})", + R"({"HASH<_id>":true,"HASH<a>":"?string"})", actual); actual = redactProj("{a: '$field'}"); @@ -180,55 +181,38 @@ TEST(Redaction, ProjectionTest) { // Dotted path actual = redactProj("{\"a.b\": \"hi\"}"); ASSERT_DOCUMENT_EQ_AUTO( // NOLINT - R"({"HASH<_id>":true,"HASH<a>":{"HASH<b>":{"$const":"?"}}})", + R"({"HASH<_id>":true,"HASH<a>":{"HASH<b>":"?string"}})", actual); // Two fields actual = redactProj("{a: \"hi\", b: \"hello\"}"); ASSERT_DOCUMENT_EQ_AUTO( // NOLINT - R"({"HASH<_id>":true,"HASH<a>":{"$const":"?"},"HASH<b>":{"$const":"?"}})", + R"({"HASH<_id>":true,"HASH<a>":"?string","HASH<b>":"?string"})", actual); // Explicit _id: 0 actual = redactProj("{b: \"hi\", _id: \"hey\"}"); ASSERT_DOCUMENT_EQ_AUTO( // NOLINT - R"({"HASH<b>":{"$const":"?"},"HASH<_id>":{"$const":"?"}})", + R"({"HASH<b>":"?string","HASH<_id>":"?string"})", actual); // Two nested fields actual = redactProj("{\"b.d\": \"hello\", \"b.c\": \"world\"}"); ASSERT_DOCUMENT_EQ_AUTO( // NOLINT - R"({ - "HASH<_id>": true, - "HASH<b>": { - "HASH<d>": { - "$const": "?" - }, - "HASH<c>": { - "$const": "?" - } - } - })", + R"({"HASH<_id>":true,"HASH<b>":{"HASH<d>":"?string","HASH<c>":"?string"}})", actual); actual = redactProj("{\"b.d\": \"hello\", a: \"world\", \"b.c\": \"mongodb\"}"); ASSERT_DOCUMENT_EQ_AUTO( // NOLINT R"({ - "HASH<_id>": true, - "HASH<b>": { - "HASH<d>": { - "$const": "?" - }, - "HASH<c>": { - "$const": "?" - } - }, - "HASH<a>": { - "$const": "?" - } - })", + "HASH<_id>": true, + "HASH<b>": { + "HASH<d>": "?string", + "HASH<c>": "?string" + }, + "HASH<a>": "?string" + })", actual); } - } // namespace } // namespace mongo diff --git a/src/mongo/db/matcher/expression.h b/src/mongo/db/matcher/expression.h index 0976404646d..4f376d78ce2 100644 --- a/src/mongo/db/matcher/expression.h +++ b/src/mongo/db/matcher/expression.h @@ -488,9 +488,9 @@ public: * parsed, produces a logically equivalent MatchExpression. However, if special options are set, * this no longer holds. * - * If 'options.replacementForLiteralArgs' is set, the result is no longer expected to re-parse, - * since we will put strings in places where strings may not be accpeted syntactically (e.g. a - * number is always expected, as in with the $mod expression). + * If 'options.literalPolicy' is set to 'kToDebugTypeString', the result is no longer expected + * to re-parse, since we will put strings in places where strings may not be accpeted + * syntactically (e.g. a number is always expected, as in with the $mod expression). */ virtual void serialize(BSONObjBuilder* out, SerializationOptions options) const = 0; diff --git a/src/mongo/db/matcher/expression_always_boolean.h b/src/mongo/db/matcher/expression_always_boolean.h index b82d582a682..5b272d9e555 100644 --- a/src/mongo/db/matcher/expression_always_boolean.h +++ b/src/mongo/db/matcher/expression_always_boolean.h @@ -64,11 +64,7 @@ public: } void serialize(BSONObjBuilder* out, SerializationOptions opts) const final { - if (opts.replacementForLiteralArgs) { - out->append(name(), *opts.replacementForLiteralArgs); - } else { - out->append(name(), 1); - } + opts.appendLiteral(out, name(), 1); } bool equivalent(const MatchExpression* other) const final { diff --git a/src/mongo/db/matcher/expression_array.cpp b/src/mongo/db/matcher/expression_array.cpp index b0fd8d05a71..64b627f04a5 100644 --- a/src/mongo/db/matcher/expression_array.cpp +++ b/src/mongo/db/matcher/expression_array.cpp @@ -206,11 +206,7 @@ void SizeMatchExpression::debugString(StringBuilder& debug, int indentationLevel } BSONObj SizeMatchExpression::getSerializedRightHandSide(SerializationOptions opts) const { - const char* opName = "$size"; - if (opts.replacementForLiteralArgs) { - return BSON(opName << *opts.replacementForLiteralArgs); - } - return BSON(opName << _size); + return BSON("$size" << opts.serializeLiteral(_size)); } bool SizeMatchExpression::equivalent(const MatchExpression* other) const { diff --git a/src/mongo/db/matcher/expression_expr_test.cpp b/src/mongo/db/matcher/expression_expr_test.cpp index bb008a536c1..9c294ddab88 100644 --- a/src/mongo/db/matcher/expression_expr_test.cpp +++ b/src/mongo/db/matcher/expression_expr_test.cpp @@ -812,9 +812,9 @@ TEST_F(ExprMatchTest, ExprRedactsCorrectly) { createMatcher(fromjson("{$expr: {$sum: [\"$a\", \"$b\"]}}")); SerializationOptions opts; + opts.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; opts.identifierRedactionPolicy = redactFieldNameForTest; opts.redactIdentifiers = true; - opts.replacementForLiteralArgs = "?"; ASSERT_BSONOBJ_EQ_AUTO( // NOLINT R"({"$expr":{"$sum":["$HASH<a>","$HASH<b>"]}})", @@ -822,7 +822,7 @@ TEST_F(ExprMatchTest, ExprRedactsCorrectly) { createMatcher(fromjson("{$expr: {$sum: [\"$a\", \"b\"]}}")); ASSERT_BSONOBJ_EQ_AUTO( // NOLINT - R"({"$expr":{"$sum":["$HASH<a>",{"$const":"?"}]}})", + R"({"$expr":{"$sum":["$HASH<a>","?string"]}})", serialize(opts)); createMatcher(fromjson("{$expr: {$sum: [\"$a.b\", \"$b\"]}}")); @@ -842,7 +842,7 @@ TEST_F(ExprMatchTest, ExprRedactsCorrectly) { createMatcher(fromjson("{$expr: {$getField: {field: \"b\", input: {a: 1, b: 2}}}}")); ASSERT_BSONOBJ_EQ_AUTO( // NOLINT - R"({"$expr":{"$getField":{"field":"HASH<b>","input":{"$const":"?"}}}})", + R"({"$expr":{"$getField":{"field":"HASH<b>","input":"?object"}}})", serialize(opts)); createMatcher(fromjson("{$expr: {$getField: {field: \"b\", input: \"$a\"}}}")); @@ -857,9 +857,7 @@ TEST_F(ExprMatchTest, ExprRedactsCorrectly) { "$getField": { "field": "HASH<b>", "input": { - "HASH<a>": { - "$const": "?" - }, + "HASH<a>": "?number", "HASH<b>": "$HASH<c>" } } @@ -874,9 +872,7 @@ TEST_F(ExprMatchTest, ExprRedactsCorrectly) { "$getField": { "field": "HASH<b>.HASH<c>", "input": { - "HASH<a>": { - "$const": "?" - }, + "HASH<a>": "?number", "HASH<b>": "$HASH<c>" } } @@ -892,14 +888,10 @@ TEST_F(ExprMatchTest, ExprRedactsCorrectly) { "$setField": { "field": "HASH<b>", "input": { - "HASH<a>": { - "$const": "?" - }, + "HASH<a>": "?number", "HASH<b>": "$HASH<c>" }, - "value": { - "$const": "?" - } + "value": "?number" } } })", @@ -913,9 +905,7 @@ TEST_F(ExprMatchTest, ExprRedactsCorrectly) { "$setField": { "field": "HASH<b>.HASH<c>", "input": { - "HASH<a>": { - "$const": "?" - }, + "HASH<a>": "?number", "HASH<b>": "$HASH<c>" }, "value": "$HASH<d>" @@ -932,9 +922,7 @@ TEST_F(ExprMatchTest, ExprRedactsCorrectly) { "$setField": { "field": "HASH<b>.HASH<c>", "input": { - "HASH<a>": { - "$const": "?" - }, + "HASH<a>": "?number", "HASH<b>": "$HASH<c>" }, "value": "$HASH<d>.HASH<e>" @@ -952,14 +940,10 @@ TEST_F(ExprMatchTest, ExprRedactsCorrectly) { "$setField": { "field": "HASH<b>", "input": { - "HASH<a>": { - "$const": "?" - }, + "HASH<a>": "?number", "HASH<b>": "$HASH<c>" }, - "value": { - "$const": "?" - } + "value": "?object" } } })", @@ -974,18 +958,12 @@ TEST_F(ExprMatchTest, ExprRedactsCorrectly) { "$setField": { "field": "HASH<b>", "input": { - "HASH<a>": { - "$const": "?" - }, + "HASH<a>": "?number", "HASH<b>": "$HASH<c>" }, "value": { - "HASH<a>": { - "$const": "?" - }, - "HASH<b>": { - "$const": "?" - }, + "HASH<a>": "?number", + "HASH<b>": "?number", "HASH<c>": "$HASH<d>" } } diff --git a/src/mongo/db/matcher/expression_geo.cpp b/src/mongo/db/matcher/expression_geo.cpp index 5c496c2dca5..f9c781f24b0 100644 --- a/src/mongo/db/matcher/expression_geo.cpp +++ b/src/mongo/db/matcher/expression_geo.cpp @@ -104,8 +104,7 @@ Status GeoExpression::parseQuery(const BSONObj& obj) { return Status::OK(); } -BSONObj redactGeoExpression(const BSONObj& obj, - boost::optional<StringData> literalArgsReplacement) { +BSONObj customSerialization(const BSONObj& obj, SerializationOptions opts) { // Ideally each sub operator ($minDistance, $maxDistance, $geometry, $box) would serialize // itself, rather than GeoExpression reparse the query during serialization. However GeoMatch @@ -127,8 +126,7 @@ BSONObj redactGeoExpression(const BSONObj& obj, // Alternatively, a legacy query can have a $maxDistance suboperator to make it more // explicit. None of these values are enums so it is fine to treat them as literals // during redaction. - outerElem = it.next(); - bob.append(outerElem.fieldNameStringData(), *literalArgsReplacement); + opts.appendLiteral(&bob, it.next()); } return bob.obj(); } @@ -141,18 +139,30 @@ BSONObj redactGeoExpression(const BSONObj& obj, while (embedded_it.more()) { BSONElement argElem = embedded_it.next(); fieldName = argElem.fieldNameStringData(); - if (fieldName == "$geometry") { - BSONObjBuilder nestedSubObj = BSONObjBuilder(subObj.subobjStart(fieldName)); - BSONElement typeElt = argElem.Obj().getField("type"); - if (!typeElt.eoo()) { - nestedSubObj.append(typeElt); + if (fieldName == "$geometry"_sd) { + if (argElem.type() == BSONType::Array) { + // This would be like {$geometry: [0, 0]} which must be a point. + auto asArray = argElem.Array(); + tassert(7539807, + "Expected the point to have exactly 2 elements: an x and y.", + asArray.size() == 2UL); + subObj.appendArray(fieldName, + BSON_ARRAY(opts.serializeLiteral(asArray[0]) + << opts.serializeLiteral(asArray[1]))); + } else { + BSONObjBuilder nestedSubObj = BSONObjBuilder(subObj.subobjStart(fieldName)); + auto geometryObj = argElem.Obj(); + if (auto typeElt = geometryObj["type"]) { + nestedSubObj.append(typeElt); + } + tassert(7539800, "always expect coordinates", geometryObj["coordinates"]); + opts.appendLiteral(&nestedSubObj, geometryObj["coordinates"]); + nestedSubObj.doneFast(); } - nestedSubObj.append("coordinates", *literalArgsReplacement); - nestedSubObj.doneFast(); } - if (fieldName == "$maxDistance" || fieldName == "$box" || fieldName == "$nearSphere" || - fieldName == "$minDistance") { - subObj.append(fieldName, *literalArgsReplacement); + if (fieldName == "$maxDistance"_sd || fieldName == "$box"_sd || + fieldName == "$nearSphere"_sd || fieldName == "$minDistance"_sd) { + opts.appendLiteral(&subObj, argElem); } } subObj.doneFast(); @@ -499,8 +509,8 @@ void GeoMatchExpression::debugString(StringBuilder& debug, int indentationLevel) } BSONObj GeoMatchExpression::getSerializedRightHandSide(SerializationOptions opts) const { - if (opts.replacementForLiteralArgs) { - return redactGeoExpression(_rawObj, opts.replacementForLiteralArgs); + if (opts.literalPolicy != LiteralSerializationPolicy::kUnchanged) { + return customSerialization(_rawObj, opts); } BSONObjBuilder subobj; subobj.appendElements(_rawObj); @@ -555,8 +565,8 @@ void GeoNearMatchExpression::debugString(StringBuilder& debug, int indentationLe } BSONObj GeoNearMatchExpression::getSerializedRightHandSide(SerializationOptions opts) const { - if (opts.replacementForLiteralArgs) { - return redactGeoExpression(_rawObj, opts.replacementForLiteralArgs); + if (opts.literalPolicy != LiteralSerializationPolicy::kUnchanged) { + return customSerialization(_rawObj, opts); } BSONObjBuilder objBuilder; objBuilder.appendElements(_rawObj); diff --git a/src/mongo/db/matcher/expression_geo_test.cpp b/src/mongo/db/matcher/expression_geo_test.cpp index 92a06bf8b3b..947967cfb16 100644 --- a/src/mongo/db/matcher/expression_geo_test.cpp +++ b/src/mongo/db/matcher/expression_geo_test.cpp @@ -159,14 +159,13 @@ TEST(ExpressionGeoTest, GeoNearEquivalent) { TEST(ExpressionGeoTest, SerializeGeoExpressions) { SerializationOptions opts = {}; opts.redactIdentifiers = true; - opts.replacementForLiteralArgs = "?"; + opts.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; { BSONObj query = fromjson("{$within: {$box: [{x: 4, y: 4}, [6, 6]]}}"); std::unique_ptr<GeoMatchExpression> ge(makeGeoMatchExpression(query)); - ASSERT_VALUE_EQ_AUTO( // NOLINT - "{ $within: { $box: \"?\" } }", // NOLINT (test - // auto-update) + ASSERT_BSONOBJ_EQ_AUTO( // NOLINT + R"({"$within":{"$box":"?array<>"}})", ge->getSerializedRightHandSide(opts)); } { @@ -174,32 +173,56 @@ TEST(ExpressionGeoTest, SerializeGeoExpressions) { "{$geoWithin: {$geometry: {type: \"MultiPolygon\", coordinates: [[[[20.0, 70.0],[30.0, " "70.0],[30.0, 50.0],[20.0, 50.0],[20.0, 70.0]]]]}}}"); std::unique_ptr<GeoMatchExpression> ge(makeGeoMatchExpression(query)); - ASSERT_VALUE_EQ_AUTO( // NOLINT - "{ $geoWithin: { $geometry: { type: \"MultiPolygon\", coordinates: \"?\" } } }", // NOLINT - // (test - // auto-update) + ASSERT_BSONOBJ_EQ_AUTO( // NOLINT + R"({ + "$geoWithin": { + "$geometry": { + "type": "MultiPolygon", + "coordinates": "?array<?array>" + } + } + })", ge->getSerializedRightHandSide(opts)); } { BSONObj query = fromjson( - "{$geoIntersects: {$geometry: {type: \"MultiPolygon\",coordinates: [[[[-20.0, " - "-70.0],[-30.0, -70.0],[-30.0, -50.0],[-20.0, -50.0],[-20.0, -70.0]]]]}}}"); + R"({ + "$geoIntersects": { + "$geometry": { + "type": "MultiPolygon", + "coordinates": [[[ + [-20.0, -70.0], + [-30.0, -70.0], + [-30.0, -50.0], + [-20.0, -50.0], + [-20.0, -70.0] + ]]] + } + } + })"); std::unique_ptr<GeoMatchExpression> ge(makeGeoMatchExpression(query)); - ASSERT_VALUE_EQ_AUTO( // NOLINT - "{ $geoIntersects: { $geometry: { type: \"MultiPolygon\", coordinates: \"?\" } } }", // NOLINT - // (test - // auto-update) + ASSERT_BSONOBJ_EQ_AUTO( // NOLINT + R"({ + "$geoIntersects": { + "$geometry": { + "type": "MultiPolygon", + "coordinates": "?array<?array>" + } + } + })", ge->getSerializedRightHandSide(opts)); } { BSONObj query1 = fromjson( - "{$within: {$geometry: {type: 'Polygon'," - "coordinates: [[[0, 0], [3, 6], [6, 1], [0, 0]]]}}}"); + R"({$within: { + $geometry: { + type: 'Polygon', + coordinates: [[[0, 0], [3, 6], [6, 1], [0, 0]]] + } + }})"); std::unique_ptr<GeoMatchExpression> ge(makeGeoMatchExpression(query1)); - ASSERT_VALUE_EQ_AUTO( // NOLINT - "{ $within: { $geometry: { type: \"Polygon\", coordinates: \"?\" } } }", // NOLINT - // (test - // auto-update) + ASSERT_BSONOBJ_EQ_AUTO( // NOLINT + R"({"$within":{"$geometry":{"type":"Polygon","coordinates":"?array<?array>"}}})", ge->getSerializedRightHandSide(opts)); } { @@ -207,48 +230,66 @@ TEST(ExpressionGeoTest, SerializeGeoExpressions) { "{$near: {$maxDistance: 100, " "$geometry: {type: 'Point', coordinates: [0, 0]}}}"); std::unique_ptr<GeoNearMatchExpression> gne(makeGeoNearMatchExpression(query)); - ASSERT_VALUE_EQ_AUTO( // NOLINT - "{ $near: { $maxDistance: \"?\", $geometry: { type: \"Point\", coordinates: \"?\" } } " - "}", // NOLINT (test auto-update) + ASSERT_BSONOBJ_EQ_AUTO( // NOLINT + R"({ + "$near": { + "$maxDistance": "?number", + "$geometry": { + "type": "Point", + "coordinates": "?array<?number>" + } + } + })", gne->getSerializedRightHandSide(opts)); } { BSONObj query = fromjson("{ $nearSphere: [0,0], $minDistance: 1, $maxDistance: 3 }"); std::unique_ptr<GeoNearMatchExpression> gne(makeGeoNearMatchExpression(query)); - ASSERT_VALUE_EQ_AUTO( // NOLINT - "{ $nearSphere: \"?\", $minDistance: \"?\", $maxDistance: \"?\" }", // NOLINT (test - // auto-update) + ASSERT_BSONOBJ_EQ_AUTO( // NOLINT + R"({ + "$nearSphere": "?array<?number>", + "$minDistance": "?number", + "$maxDistance": "?number" + })", gne->getSerializedRightHandSide(opts)); } { - BSONObj query = fromjson("{$near : [0, 0, 1] } }"); + BSONObj query = fromjson("{$near : [0, 0, 1] }"); std::unique_ptr<GeoNearMatchExpression> gne(makeGeoNearMatchExpression(query)); - ASSERT_VALUE_EQ_AUTO( // NOLINT - "{ $near: \"?\" }", // NOLINT (test auto-update) + ASSERT_BSONOBJ_EQ_AUTO( // NOLINT + R"({"$near":"?array<?number>"})", gne->getSerializedRightHandSide(opts)); } { BSONObj query = fromjson("{$geoNear: [0, 0, 100]}"); std::unique_ptr<GeoNearMatchExpression> gne(makeGeoNearMatchExpression(query)); - ASSERT_VALUE_EQ_AUTO( // NOLINT - "{ $geoNear: \"?\" }", // NOLINT (test auto-update) + ASSERT_BSONOBJ_EQ_AUTO( // NOLINT + R"({"$geoNear":"?array<?number>"})", gne->getSerializedRightHandSide(opts)); } { BSONObj query = fromjson("{$geoNear: [0, 10], $maxDistance: 80 }"); std::unique_ptr<GeoNearMatchExpression> gne(makeGeoNearMatchExpression(query)); - ASSERT_VALUE_EQ_AUTO( // NOLINT - "{ $geoNear: \"?\", $maxDistance: \"?\" }", // NOLINT (test auto-update) + ASSERT_BSONOBJ_EQ_AUTO( // NOLINT + R"({"$geoNear":"?array<?number>","$maxDistance":"?number"})", gne->getSerializedRightHandSide(opts)); } { BSONObj query = fromjson("{$geoIntersects: {$geometry: [0, 0]}}"); std::unique_ptr<GeoMatchExpression> ge(makeGeoMatchExpression(query)); - ASSERT_VALUE_EQ_AUTO( // NOLINT - "{ $geoIntersects: { $geometry: { coordinates: \"?\" } } }", + ASSERT_BSONOBJ_EQ_AUTO( // NOLINT + R"({"$geoIntersects":{"$geometry":["?number","?number"]}})", ge->getSerializedRightHandSide(opts)); } + { + // Make sure we reject arrays with <2 or >2 elements. + BSONObj query = fromjson("{$geoIntersects: {$geometry: [0, 0, 1]}}"); + std::unique_ptr<GeoExpression> gq(new GeoExpression); + ASSERT_NOT_OK(gq->parseFrom(query)); + query = fromjson("{$geoIntersects: {$geometry: [0]}}"); + ASSERT_NOT_OK(gq->parseFrom(query)); + } } /** diff --git a/src/mongo/db/matcher/expression_internal_bucket_geo_within.cpp b/src/mongo/db/matcher/expression_internal_bucket_geo_within.cpp index bb1edfde18b..39f20cffde0 100644 --- a/src/mongo/db/matcher/expression_internal_bucket_geo_within.cpp +++ b/src/mongo/db/matcher/expression_internal_bucket_geo_within.cpp @@ -190,11 +190,7 @@ void InternalBucketGeoWithinMatchExpression::serialize(BSONObjBuilder* builder, // Serialize the geometry shape. BSONObjBuilder withinRegionBob( bob.subobjStart(InternalBucketGeoWithinMatchExpression::kWithinRegion)); - if (opts.replacementForLiteralArgs) { - bob.append(_geoContainer->getGeoElement().fieldName(), *opts.replacementForLiteralArgs); - } else { - withinRegionBob.append(_geoContainer->getGeoElement()); - } + opts.appendLiteral(&withinRegionBob, _geoContainer->getGeoElement()); withinRegionBob.doneFast(); // Serialize the field which is being searched over. bob.append(InternalBucketGeoWithinMatchExpression::kField, diff --git a/src/mongo/db/matcher/expression_leaf.cpp b/src/mongo/db/matcher/expression_leaf.cpp index faa1fde0555..484498ecc0f 100644 --- a/src/mongo/db/matcher/expression_leaf.cpp +++ b/src/mongo/db/matcher/expression_leaf.cpp @@ -38,6 +38,7 @@ #include "mongo/bson/bsonmisc.h" #include "mongo/bson/bsonobj.h" #include "mongo/config.h" +#include "mongo/db/exec/document_value/value.h" #include "mongo/db/field_ref.h" #include "mongo/db/jsobj.h" #include "mongo/db/matcher/expression_parser.h" @@ -88,11 +89,7 @@ void ComparisonMatchExpressionBase::debugString(StringBuilder& debug, int indent } BSONObj ComparisonMatchExpressionBase::getSerializedRightHandSide(SerializationOptions opts) const { - if (opts.replacementForLiteralArgs) { - return BSON(name() << *opts.replacementForLiteralArgs); - } else { - return BSON(name() << _rhs); - } + return BSON(name() << opts.serializeLiteral(_rhs)); } ComparisonMatchExpression::ComparisonMatchExpression(MatchType type, @@ -278,10 +275,10 @@ void RegexMatchExpression::debugString(StringBuilder& debug, int indentationLeve BSONObj RegexMatchExpression::getSerializedRightHandSide(SerializationOptions opts) const { BSONObjBuilder regexBuilder; - regexBuilder.append("$regex", opts.replacementForLiteralArgs.value_or(_regex)); + opts.appendLiteral(®exBuilder, "$regex", _regex); if (!_flags.empty()) { - regexBuilder.append("$options", opts.replacementForLiteralArgs.value_or(_flags)); + opts.appendLiteral(®exBuilder, "$options", _flags); } return regexBuilder.obj(); @@ -353,11 +350,8 @@ void ModMatchExpression::debugString(StringBuilder& debug, int indentationLevel) } BSONObj ModMatchExpression::getSerializedRightHandSide(SerializationOptions opts) const { - if (auto str = opts.replacementForLiteralArgs) { - return BSON("$mod" << *str); - } else { - return BSON("$mod" << BSON_ARRAY(_divisor << _remainder)); - } + return BSON( + "$mod" << BSON_ARRAY(opts.serializeLiteral(_divisor) << opts.serializeLiteral(_remainder))); } bool ModMatchExpression::equivalent(const MatchExpression* other) const { @@ -388,11 +382,7 @@ void ExistsMatchExpression::debugString(StringBuilder& debug, int indentationLev } BSONObj ExistsMatchExpression::getSerializedRightHandSide(SerializationOptions opts) const { - if (opts.replacementForLiteralArgs) { - return BSON("$exists" << *opts.replacementForLiteralArgs); - } else { - return BSON("$exists" << true); - } + return BSON("$exists" << opts.serializeLiteral(true)); } bool ExistsMatchExpression::equivalent(const MatchExpression* other) const { @@ -471,10 +461,38 @@ void InMatchExpression::debugString(StringBuilder& debug, int indentationLevel) _debugStringAttachTagInfo(&debug); } +namespace { +/** + * Reduces the potentially large vector of elements to just the first of each "canonical" type. + * Different types of numbers are not considered distinct. + * + * For example, collapses [2, 4, NumberInt(3), "string", "another", 3, 5] into just [2, "string"]. + */ +std::vector<Value> justFirstOfEachType(std::vector<BSONElement> elems) { + stdx::unordered_set<int> seenTypes; + std::vector<Value> result; + for (auto&& elem : elems) { + bool inserted = seenTypes.insert(canonicalizeBSONType(elem.type())).second; + if (inserted) { + // A new type. + result.emplace_back(elem); + } + } + return result; +} +} // namespace + +BSONObj InMatchExpression::serializeToShape(SerializationOptions opts) const { + std::vector<Value> firstOfEachType = justFirstOfEachType(_equalitySet); + if (hasRegex()) { + firstOfEachType.emplace_back(BSONRegEx()); + } + return BSON("$in" << opts.serializeLiteral(firstOfEachType)); +} + BSONObj InMatchExpression::getSerializedRightHandSide(SerializationOptions opts) const { - if (opts.replacementForLiteralArgs) { - // In this case, treat an '$in' with any number of arguments as equivalent. - return BSON("$in" << BSON_ARRAY(*opts.replacementForLiteralArgs)); + if (opts.literalPolicy != LiteralSerializationPolicy::kUnchanged) { + return serializeToShape(opts); } BSONObjBuilder inBob; @@ -846,17 +864,13 @@ BSONObj BitTestMatchExpression::getSerializedRightHandSide(SerializationOptions MONGO_UNREACHABLE; } - if (opts.replacementForLiteralArgs) { - return BSON(opString << *opts.replacementForLiteralArgs); - } - BSONArrayBuilder arrBob; for (auto bitPosition : _bitPositions) { arrBob.append(static_cast<int32_t>(bitPosition)); } arrBob.doneFast(); - return BSON(opString << arrBob.arr()); + return BSON(opString << opts.serializeLiteral(arrBob.arr())); } bool BitTestMatchExpression::equivalent(const MatchExpression* other) const { diff --git a/src/mongo/db/matcher/expression_leaf.h b/src/mongo/db/matcher/expression_leaf.h index 86e73637928..6b04ad09fc0 100644 --- a/src/mongo/db/matcher/expression_leaf.h +++ b/src/mongo/db/matcher/expression_leaf.h @@ -763,6 +763,12 @@ public: private: ExpressionOptimizerFunc getOptimizer() const final; + /** + * A helper to serialize to something like {$in: "?array<?number>"} or similar, depending on + * 'opts' and whether we have a mixed-type $in or not. + */ + BSONObj serializeToShape(SerializationOptions opts) const; + // Whether or not '_equalities' has a jstNULL element in it. bool _hasNull = false; diff --git a/src/mongo/db/matcher/expression_path.h b/src/mongo/db/matcher/expression_path.h index dcab87590f6..7bed80b751a 100644 --- a/src/mongo/db/matcher/expression_path.h +++ b/src/mongo/db/matcher/expression_path.h @@ -174,9 +174,9 @@ public: * PathMatchExpression. * * Serialization options should be respected for any descendent expressions. Eg, if the - * 'replacementForLiteralArgs' option is set, then any literal argument (like the number 1 in - * the example above), should be replaced with this string. 'literal' here is in contrast to - * another expression, if that is possible syntactically. + * 'literalPolicy' option is 'kToDebugTypeString', then any literal argument (like the number 1 + * in the example above), should be "shapified" (e.g. "?number"). 'literal' here is in contrast + * to another expression, if that is possible syntactically. */ virtual BSONObj getSerializedRightHandSide(SerializationOptions opts = {}) const = 0; diff --git a/src/mongo/db/matcher/expression_serialization_test.cpp b/src/mongo/db/matcher/expression_serialization_test.cpp index d0bc9801753..024a90404d3 100644 --- a/src/mongo/db/matcher/expression_serialization_test.cpp +++ b/src/mongo/db/matcher/expression_serialization_test.cpp @@ -362,7 +362,6 @@ TEST(SerializeBasic, ExpressionElemMatchValueWithTripleNotSerializesCorrectly) { ASSERT_EQ(original.matches(obj), reserialized.matches(obj)); } - TEST(SerializeBasic, ExpressionSizeSerializesCorrectly) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); Matcher original(fromjson("{x: {$size: 2}}"), @@ -1100,7 +1099,7 @@ TEST(SerializeBasic, ExpressionNotWithDirectPathExpSerializesCorrectly) { // direct path expression child, instead creating a NOT -> AND -> path expression. This test // manually constructs such an expression in case it ever turns up, since that should still be // able to serialize. - auto originalBSON = fromjson("{a: {$not: {$eq: 2}}}}"); + auto originalBSON = fromjson("{a: {$not: {$eq: 2}}}"); auto equalityRHSElem = originalBSON["a"]["$not"]["$eq"]; auto equalityExpression = std::make_unique<EqualityMatchExpression>("a"_sd, equalityRHSElem); @@ -1686,7 +1685,7 @@ TEST(SerializeInternalSchema, ExpressionInternalSchemaMaxLengthSerializesCorrect TEST(SerializeInternalSchema, ExpressionInternalSchemaCondSerializesCorrectly) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); - Matcher original(fromjson("{$_internalSchemaCond: [{a: 1}, {b: 2}, {c: 3}]}}"), + Matcher original(fromjson("{$_internalSchemaCond: [{a: 1}, {b: 2}, {c: 3}]}"), expCtx, ExtensionsCallbackNoop(), MatchExpressionParser::kAllowAllSpecialFeatures); @@ -1697,7 +1696,7 @@ TEST(SerializeInternalSchema, ExpressionInternalSchemaCondSerializesCorrectly) { BSONObjBuilder builder; ASSERT_BSONOBJ_EQ( *reserialized.getQuery(), - fromjson("{$_internalSchemaCond: [{a: {$eq: 1}}, {b: {$eq: 2}}, {c: {$eq: 3}}]}}")); + fromjson("{$_internalSchemaCond: [{a: {$eq: 1}}, {b: {$eq: 2}}, {c: {$eq: 3}}]}")); ASSERT_BSONOBJ_EQ(*reserialized.getQuery(), serialize(reserialized.getMatchExpression())); } @@ -1856,7 +1855,9 @@ TEST(SerializeInternalBinDataSubType, ExpressionBinDataSubTypeSerializesCorrectl } std::string redactFieldNameForTest(StringData s) { - return str::stream() << "HASH(" << s << ")"; + // Avoid ending in a parenthesis since the results will occur in a raw string where the )" + // sequence will accidentally terminate the string. + return str::stream() << "HASH<" << s << ">"; } TEST(SerializeInternalSchema, AllowedPropertiesRedactsCorrectly) { @@ -1870,12 +1871,21 @@ TEST(SerializeInternalSchema, AllowedPropertiesRedactsCorrectly) { SerializationOptions opts; opts.redactIdentifiers = true; opts.identifierRedactionPolicy = redactFieldNameForTest; - opts.replacementForLiteralArgs = "?"; - - ASSERT_BSONOBJ_EQ( - fromjson( - "{ $_internalSchemaAllowedProperties: { properties: \"?\", namePlaceholder: \"?\", " - "patternProperties: [], otherwise: { \"HASH(i)\": { $eq: \"?\" } } } }"), + opts.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; + + ASSERT_BSONOBJ_EQ_AUTO( // NOLINT + R"({ + "$_internalSchemaAllowedProperties": { + "properties": "?array<?string>", + "namePlaceholder": "?string", + "patternProperties": [], + "otherwise": { + "HASH<i>": { + "$eq": "?number" + } + } + } + })", objMatch.getValue()->serialize(opts)); } @@ -1905,7 +1915,7 @@ std::unique_ptr<InternalSchemaCondMatchExpression> createCondMatchExpression(BSO TEST(SerializeInternalSchema, CondMatchRedactsCorrectly) { SerializationOptions opts; opts.redactIdentifiers = true; - opts.replacementForLiteralArgs = "?"; + opts.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; opts.identifierRedactionPolicy = redactFieldNameForTest; auto conditionQuery = BSON("age" << BSON("$lt" << 18)); auto thenQuery = BSON("job" @@ -1915,25 +1925,38 @@ TEST(SerializeInternalSchema, CondMatchRedactsCorrectly) { auto cond = createCondMatchExpression(conditionQuery, thenQuery, elseQuery); BSONObjBuilder bob; cond->serialize(&bob, opts); - auto expectedResult = - BSON("$_internalSchemaCond" << BSON_ARRAY(BSON("HASH(age)" << BSON("$lt" - << "?")) - << BSON("HASH(job)" << BSON("$eq" - << "?")) - << BSON("HASH(job)" << BSON("$eq" - << "?")))); - ASSERT_BSONOBJ_EQ(expectedResult, bob.done()); + ASSERT_BSONOBJ_EQ_AUTO( // NOLINT + R"({ + "$_internalSchemaCond": [ + { + "HASH<age>": { + "$lt": "?number" + } + }, + { + "HASH<job>": { + "$eq": "?string" + } + }, + { + "HASH<job>": { + "$eq": "?string" + } + } + ] + })", + bob.done()); } TEST(SerializeInternalSchema, FmodMatchRedactsCorrectly) { InternalSchemaFmodMatchExpression m("a"_sd, Decimal128(1.7), Decimal128(2)); SerializationOptions opts; - opts.replacementForLiteralArgs = "?"; + opts.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; BSONObjBuilder bob; m.serialize(&bob, opts); - ASSERT_BSONOBJ_EQ(BSON("a" << BSON("$_internalSchemaFmod" << BSON_ARRAY("?" - << "?"))), - bob.done()); + ASSERT_BSONOBJ_EQ_AUTO( // NOLINT + R"({"a":{"$_internalSchemaFmod":["?number","?number"]}})", + bob.done()); } TEST(SerializeInternalSchema, MatchArrayIndexRedactsCorrectly) { @@ -1946,81 +1969,87 @@ TEST(SerializeInternalSchema, MatchArrayIndexRedactsCorrectly) { BSONObjBuilder bob; SerializationOptions opts; + opts.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; opts.redactIdentifiers = true; opts.identifierRedactionPolicy = redactFieldNameForTest; - opts.replacementForLiteralArgs = "?"; objMatch.getValue()->serialize(&bob, opts); - - ASSERT_BSONOBJ_EQ(bob.done(), - BSON("HASH(foo)" << BSON("$_internalSchemaMatchArrayIndex" - << BSON("index" - << "?" - << "namePlaceholder" - << "HASH(i)" - << "expression" - << BSON("HASH(i)" << BSON("$type" - << "?")))))); + ASSERT_BSONOBJ_EQ_AUTO( // NOLINT + R"({ + "HASH<foo>": { + "$_internalSchemaMatchArrayIndex": { + "index": "?number", + "namePlaceholder": "HASH<i>", + "expression": { + "HASH<i>": { + "$type": "?array<?string>" + } + } + } + } + })", + bob.done()); } TEST(SerializeInternalSchema, MaxItemsRedactsCorrectly) { InternalSchemaMaxItemsMatchExpression maxItems("a.b"_sd, 2); SerializationOptions opts; + opts.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; opts.redactIdentifiers = true; - opts.replacementForLiteralArgs = "?"; opts.identifierRedactionPolicy = redactFieldNameForTest; - - ASSERT_BSONOBJ_EQ(maxItems.getSerializedRightHandSide(opts), - BSON("$_internalSchemaMaxItems" - << "?")); + ASSERT_BSONOBJ_EQ_AUTO( // NOLINT + R"({"$_internalSchemaMaxItems":"?number"})", + maxItems.getSerializedRightHandSide(opts)); } TEST(SerializeInternalSchema, MaxLengthRedactsCorrectly) { InternalSchemaMaxLengthMatchExpression maxLength("a"_sd, 2); SerializationOptions opts; - opts.replacementForLiteralArgs = "?"; - ASSERT_BSONOBJ_EQ(maxLength.getSerializedRightHandSide(opts), - BSON("$_internalSchemaMaxLength" - << "?")); + opts.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; + opts.redactIdentifiers = true; + opts.identifierRedactionPolicy = redactFieldNameForTest; + ASSERT_BSONOBJ_EQ_AUTO( // NOLINT + R"({"$_internalSchemaMaxLength":"?number"})", + maxLength.getSerializedRightHandSide(opts)); } TEST(SerializeInternalSchema, MinItemsRedactsCorrectly) { InternalSchemaMinItemsMatchExpression minItems("a.b"_sd, 2); SerializationOptions opts; + opts.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; opts.redactIdentifiers = true; - opts.replacementForLiteralArgs = "?"; opts.identifierRedactionPolicy = redactFieldNameForTest; - ASSERT_BSONOBJ_EQ(minItems.getSerializedRightHandSide(opts), - BSON("$_internalSchemaMinItems" - << "?")); + ASSERT_BSONOBJ_EQ_AUTO( // NOLINT + R"({"$_internalSchemaMinItems":"?number"})", + minItems.getSerializedRightHandSide(opts)); } TEST(SerializeInternalSchema, MinLengthRedactsCorrectly) { InternalSchemaMinLengthMatchExpression minLength("a"_sd, 2); SerializationOptions opts; - opts.replacementForLiteralArgs = "?"; - ASSERT_BSONOBJ_EQ(minLength.getSerializedRightHandSide(opts), - BSON("$_internalSchemaMinLength" - << "?")); + opts.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; + ASSERT_BSONOBJ_EQ_AUTO( // NOLINT + R"({"$_internalSchemaMinLength":"?number"})", + minLength.getSerializedRightHandSide(opts)); } TEST(SerializeInternalSchema, MinPropertiesRedactsCorrectly) { InternalSchemaMinPropertiesMatchExpression minProperties(5); SerializationOptions opts; - opts.replacementForLiteralArgs = "?"; + opts.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; BSONObjBuilder bob; minProperties.serialize(&bob, opts); - ASSERT_BSONOBJ_EQ(bob.done(), - BSON("$_internalSchemaMinProperties" - << "?")); + ASSERT_BSONOBJ_EQ_AUTO( // NOLINT + R"({"$_internalSchemaMinProperties":"?number"})", + bob.done()); } TEST(SerializeInternalSchema, ObjectMatchRedactsCorrectly) { SerializationOptions opts; + opts.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; opts.redactIdentifiers = true; opts.identifierRedactionPolicy = redactFieldNameForTest; - opts.replacementForLiteralArgs = "?"; auto query = fromjson( " {a: {$_internalSchemaObjectMatch: {" " c: {$eq: 3}" @@ -2029,30 +2058,32 @@ TEST(SerializeInternalSchema, ObjectMatchRedactsCorrectly) { auto objMatch = MatchExpressionParser::parse(query, expCtx); ASSERT_OK(objMatch.getStatus()); - ASSERT_BSONOBJ_EQ( - objMatch.getValue()->serialize(opts), - BSON("HASH(a)" << BSON("$_internalSchemaObjectMatch" << BSON("HASH(c)" << BSON("$eq" - << "?"))))); + ASSERT_BSONOBJ_EQ_AUTO( // NOLINT + R"({"HASH<a>":{"$_internalSchemaObjectMatch":{"HASH<c>":{"$eq":"?number"}}}})", + objMatch.getValue()->serialize(opts)); } TEST(SerializeInternalSchema, RootDocEqRedactsCorrectly) { auto query = fromjson("{$_internalSchemaRootDocEq: {a:1, b: {c: 1, d: [1]}}}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); SerializationOptions opts; + opts.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; opts.redactIdentifiers = true; opts.identifierRedactionPolicy = redactFieldNameForTest; - opts.replacementForLiteralArgs = "?"; auto objMatch = MatchExpressionParser::parse(query, expCtx); - ASSERT_OK(objMatch.getStatus()); - - ASSERT_BSONOBJ_EQ( - objMatch.getValue()->serialize(opts), - BSON("$_internalSchemaRootDocEq" << BSON("HASH(a)" - << "?" - << "HASH(b)" - << BSON("HASH(c)" - << "?" - << "HASH(d)" << BSON_ARRAY("?"))))); + ASSERT_BSONOBJ_EQ_AUTO( // NOLINT + R"({ + "$_internalSchemaRootDocEq": { + "HASH<a>": "?number", + "HASH<b>": { + "HASH<c>": "?number", + "HASH<d>": [ + "?number" + ] + } + } + })", + objMatch.getValue()->serialize(opts)); } TEST(SerializeInternalSchema, BinDataEncryptedTypeRedactsCorrectly) { @@ -2061,35 +2092,36 @@ TEST(SerializeInternalSchema, BinDataEncryptedTypeRedactsCorrectly) { typeSet.bsonTypes.insert(BSONType::Date); InternalSchemaBinDataEncryptedTypeExpression e("a"_sd, std::move(typeSet)); SerializationOptions opts; - opts.replacementForLiteralArgs = "?"; - ASSERT_BSONOBJ_EQ(BSON("$_internalSchemaBinDataEncryptedType" - << "?"), - e.getSerializedRightHandSide(opts)); + opts.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; + ASSERT_BSONOBJ_EQ_AUTO( // NOLINT + R"({"$_internalSchemaBinDataEncryptedType":"?array<?number>"})", + e.getSerializedRightHandSide(opts)); } TEST(SerializeInternalSchema, BinDataFLE2EncryptedTypeRedactsCorrectly) { InternalSchemaBinDataFLE2EncryptedTypeExpression e("ssn"_sd, BSONType::String); SerializationOptions opts; - opts.replacementForLiteralArgs = "?"; - ASSERT_BSONOBJ_EQ(BSON("$_internalSchemaBinDataFLE2EncryptedType" - << "?"), - e.getSerializedRightHandSide(opts)); + opts.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; + ASSERT_BSONOBJ_EQ_AUTO( // NOLINT + R"({"$_internalSchemaBinDataFLE2EncryptedType":"?array<?number>"})", + e.getSerializedRightHandSide(opts)); } TEST(SerializesInternalSchema, MaxPropertiesRedactsCorrectly) { InternalSchemaMaxPropertiesMatchExpression maxProperties(5); SerializationOptions opts; - opts.replacementForLiteralArgs = "?"; + opts.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; BSONObjBuilder bob; maxProperties.serialize(&bob, opts); - ASSERT_BSONOBJ_EQ(bob.done(), - BSON("$_internalSchemaMaxProperties" - << "?")); + ASSERT_BSONOBJ_EQ_AUTO( // NOLINT + R"({"$_internalSchemaMaxProperties":"?number"})", + bob.done()); } TEST(SerializesInternalSchema, EqRedactsCorrectly) { SerializationOptions opts; + opts.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; opts.identifierRedactionPolicy = redactFieldNameForTest; opts.redactIdentifiers = true; opts.replacementForLiteralArgs = "?"; @@ -2097,14 +2129,21 @@ TEST(SerializesInternalSchema, EqRedactsCorrectly) { BSONObjBuilder bob; InternalSchemaEqMatchExpression e("a"_sd, query.firstElement()); e.serialize(&bob, opts); - ASSERT_BSONOBJ_EQ(bob.done(), - BSON("HASH(a)" << BSON("$_internalSchemaEq" - << BSON("HASH(a)" - << "?" - << "HASH(b)" - << BSON("HASH(c)" - << "?" - << "HASH(d)" << BSON_ARRAY("?")))))); + ASSERT_BSONOBJ_EQ_AUTO( // NOLINT + R"({ + "HASH<a>": { + "$_internalSchemaEq": { + "HASH<a>": "?number", + "HASH<b>": { + "HASH<c>": "?number", + "HASH<d>": [ + "?number" + ] + } + } + } + })", + bob.done()); } TEST(InternalSchemaAllElemMatchFromIndexMatchExpression, RedactsExpressionCorrectly) { @@ -2118,12 +2157,20 @@ TEST(InternalSchemaAllElemMatchFromIndexMatchExpression, RedactsExpressionCorrec SerializationOptions opts; opts.redactIdentifiers = true; opts.identifierRedactionPolicy = redactFieldNameForTest; - opts.replacementForLiteralArgs = "?"; - - ASSERT_BSONOBJ_EQ(BSON("$_internalSchemaAllElemMatchFromIndex" - << BSON_ARRAY("?" << BSON("HASH(a)" << BSON("$lt" - << "?")))), - elemMatchExpr->getSerializedRightHandSide(opts)); + opts.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; + + ASSERT_BSONOBJ_EQ_AUTO( // NOLINT + R"({ + "$_internalSchemaAllElemMatchFromIndex": [ + "?number", + { + "HASH<a>": { + "$lt": "?number" + } + } + ] + })", + elemMatchExpr->getSerializedRightHandSide(opts)); } } // namespace } // namespace mongo diff --git a/src/mongo/db/matcher/expression_text_base.cpp b/src/mongo/db/matcher/expression_text_base.cpp index 8cdb360bf3a..fbf14910ecd 100644 --- a/src/mongo/db/matcher/expression_text_base.cpp +++ b/src/mongo/db/matcher/expression_text_base.cpp @@ -52,18 +52,12 @@ void TextMatchExpressionBase::debugString(StringBuilder& debug, int indentationL void TextMatchExpressionBase::serialize(BSONObjBuilder* out, SerializationOptions opts) const { const fts::FTSQuery& ftsQuery = getFTSQuery(); - if (opts.replacementForLiteralArgs) { - out->append("$text", - BSON("$search" << *opts.replacementForLiteralArgs << "$language" - << *opts.replacementForLiteralArgs << "$caseSensitive" - << *opts.replacementForLiteralArgs << "$diacriticSensitive" - << *opts.replacementForLiteralArgs)); - } else { - out->append("$text", - BSON("$search" << ftsQuery.getQuery() << "$language" << ftsQuery.getLanguage() - << "$caseSensitive" << ftsQuery.getCaseSensitive() - << "$diacriticSensitive" << ftsQuery.getDiacriticSensitive())); - } + out->append("$text", + BSON("$search" << opts.serializeLiteral(ftsQuery.getQuery()) << "$language" + << opts.serializeLiteral(ftsQuery.getLanguage()) << "$caseSensitive" + << opts.serializeLiteral(ftsQuery.getCaseSensitive()) + << "$diacriticSensitive" + << opts.serializeLiteral(ftsQuery.getDiacriticSensitive()))); } bool TextMatchExpressionBase::equivalent(const MatchExpression* other) const { diff --git a/src/mongo/db/matcher/expression_tree.cpp b/src/mongo/db/matcher/expression_tree.cpp index 5dae4cb0f6b..b6c50ae21a2 100644 --- a/src/mongo/db/matcher/expression_tree.cpp +++ b/src/mongo/db/matcher/expression_tree.cpp @@ -455,11 +455,7 @@ void NotMatchExpression::serializeNotExpressionToNor(MatchExpression* exp, void NotMatchExpression::serialize(BSONObjBuilder* out, SerializationOptions opts) const { if (_exp->matchType() == MatchType::AND && _exp->numChildren() == 0) { - if (opts.replacementForLiteralArgs) { - out->append("$alwaysFalse", *opts.replacementForLiteralArgs); - } else { - out->append("$alwaysFalse", 1); - } + opts.appendLiteral(out, "$alwaysFalse", 1); return; } diff --git a/src/mongo/db/matcher/expression_type.h b/src/mongo/db/matcher/expression_type.h index bc2a758caf7..35eda5f1698 100644 --- a/src/mongo/db/matcher/expression_type.h +++ b/src/mongo/db/matcher/expression_type.h @@ -77,15 +77,7 @@ public: } BSONObj getSerializedRightHandSide(SerializationOptions opts) const final { - BSONObjBuilder subBuilder; - if (opts.replacementForLiteralArgs) { - subBuilder.append(name(), opts.replacementForLiteralArgs.get()); - return subBuilder.obj(); - } - BSONArrayBuilder arrBuilder(subBuilder.subarrayStart(name())); - _typeSet.toBSONArray(&arrBuilder); - arrBuilder.doneFast(); - return subBuilder.obj(); + return BSON(name() << opts.serializeLiteral(_typeSet.toBSONArray())); } bool equivalent(const MatchExpression* other) const final { @@ -247,13 +239,16 @@ public: } BSONObj getSerializedRightHandSide(SerializationOptions opts) const final { - BSONObjBuilder bob; - if (opts.replacementForLiteralArgs) { - bob.append(name(), opts.replacementForLiteralArgs.get()); + if (opts.literalPolicy == LiteralSerializationPolicy::kUnchanged) { + return BSON(name() << _binDataSubType); } else { - bob.append(name(), _binDataSubType); + // There is some fancy serialization logic to get the above BSONObjBuilder append to + // work. We just want to make sure we're doing the same thing here. + static_assert(BSONObjAppendFormat<decltype(_binDataSubType)>::value == NumberInt, + "Expecting that the BinData sub type should be specified and serialized " + "as an int."); + return BSON(name() << opts.serializeLiteral(static_cast<int>(_binDataSubType))); } - return bob.obj(); } bool equivalent(const MatchExpression* other) const final { diff --git a/src/mongo/db/matcher/expression_type_test.cpp b/src/mongo/db/matcher/expression_type_test.cpp index d934f8a402c..54fa7f815d3 100644 --- a/src/mongo/db/matcher/expression_type_test.cpp +++ b/src/mongo/db/matcher/expression_type_test.cpp @@ -220,10 +220,10 @@ TEST(ExpressionTypeTest, InternalSchemaTypeExprWithMultipleTypesMatchesAllSuchTy TEST(ExpressionTypeTest, RedactsTypesCorrectly) { TypeMatchExpression type(""_sd, String); SerializationOptions opts; - opts.replacementForLiteralArgs = "?"; - ASSERT_BSONOBJ_EQ(BSON("$type" - << "?"), - type.getSerializedRightHandSide(opts)); + opts.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; + ASSERT_BSONOBJ_EQ_AUTO( // NOLINT + R"({"$type":"?array<?number>"})", + type.getSerializedRightHandSide(opts)); } TEST(ExpressionBinDataSubTypeTest, MatchesBinDataGeneral) { @@ -312,10 +312,10 @@ TEST(ExpressionBinDataSubTypeTest, Equivalent) { TEST(ExpressionBinDataSubTypeTest, RedactsCorrectly) { InternalSchemaBinDataSubTypeExpression e("b"_sd, BinDataType::newUUID); SerializationOptions opts; - opts.replacementForLiteralArgs = "?"; - ASSERT_BSONOBJ_EQ(BSON("$_internalSchemaBinDataSubType" - << "?"), - e.getSerializedRightHandSide(opts)); + opts.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; + ASSERT_BSONOBJ_EQ_AUTO( // NOLINT + R"({"$_internalSchemaBinDataSubType":"?number"})", + e.getSerializedRightHandSide(opts)); } TEST(InternalSchemaBinDataEncryptedTypeTest, DoesNotTraverseLeafArrays) { diff --git a/src/mongo/db/matcher/expression_where_base.cpp b/src/mongo/db/matcher/expression_where_base.cpp index 2d1bec727de..00c53723ff9 100644 --- a/src/mongo/db/matcher/expression_where_base.cpp +++ b/src/mongo/db/matcher/expression_where_base.cpp @@ -48,11 +48,7 @@ void WhereMatchExpressionBase::debugString(StringBuilder& debug, int indentation } void WhereMatchExpressionBase::serialize(BSONObjBuilder* out, SerializationOptions opts) const { - if (opts.replacementForLiteralArgs) { - out->append("$where", *opts.replacementForLiteralArgs); - } else { - out->appendCode("$where", getCode()); - } + opts.appendLiteral(out, "$where", BSONCode(getCode())); } bool WhereMatchExpressionBase::equivalent(const MatchExpression* other) const { diff --git a/src/mongo/db/matcher/schema/expression_internal_schema_all_elem_match_from_index.cpp b/src/mongo/db/matcher/schema/expression_internal_schema_all_elem_match_from_index.cpp index 97ceacf1e01..b78faffbae9 100644 --- a/src/mongo/db/matcher/schema/expression_internal_schema_all_elem_match_from_index.cpp +++ b/src/mongo/db/matcher/schema/expression_internal_schema_all_elem_match_from_index.cpp @@ -78,20 +78,8 @@ void InternalSchemaAllElemMatchFromIndexMatchExpression::debugString(StringBuild BSONObj InternalSchemaAllElemMatchFromIndexMatchExpression::getSerializedRightHandSide( SerializationOptions opts) const { - BSONObjBuilder allElemMatchBob; - BSONArrayBuilder subArray(allElemMatchBob.subarrayStart(kName)); - if (opts.replacementForLiteralArgs) { - subArray.append(opts.replacementForLiteralArgs.get()); - } else { - subArray.append(_index); - } - { - BSONObjBuilder eBuilder(subArray.subobjStart()); - _expression->getFilter()->serialize(&eBuilder, opts); - eBuilder.doneFast(); - } - subArray.doneFast(); - return allElemMatchBob.obj(); + return BSON(kName << BSON_ARRAY(opts.serializeLiteral(_index) + << _expression->getFilter()->serialize(opts))); } MatchExpression::ExpressionOptimizerFunc diff --git a/src/mongo/db/matcher/schema/expression_internal_schema_allowed_properties.cpp b/src/mongo/db/matcher/schema/expression_internal_schema_allowed_properties.cpp index c9a76a5d61f..1e2db96da10 100644 --- a/src/mongo/db/matcher/schema/expression_internal_schema_allowed_properties.cpp +++ b/src/mongo/db/matcher/schema/expression_internal_schema_allowed_properties.cpp @@ -128,26 +128,14 @@ void InternalSchemaAllowedPropertiesMatchExpression::serialize(BSONObjBuilder* b std::vector<StringData> sortedProperties(_properties.begin(), _properties.end()); std::sort(sortedProperties.begin(), sortedProperties.end()); - if (opts.replacementForLiteralArgs) { - expressionBuilder.append("properties", opts.replacementForLiteralArgs.get()); - expressionBuilder.append("namePlaceholder", opts.replacementForLiteralArgs.get()); - } else { - expressionBuilder.append("properties", sortedProperties); - expressionBuilder.append("namePlaceholder", _namePlaceholder); - } + opts.appendLiteral(&expressionBuilder, "properties", sortedProperties); + opts.appendLiteral(&expressionBuilder, "namePlaceholder", _namePlaceholder); BSONArrayBuilder patternPropertiesBuilder(expressionBuilder.subarrayStart("patternProperties")); - for (auto&& item : _patternProperties) { - BSONObjBuilder itemBuilder(patternPropertiesBuilder.subobjStart()); - if (opts.replacementForLiteralArgs) { - itemBuilder.appendRegex("regex", opts.replacementForLiteralArgs.get()); - } else { - itemBuilder.appendRegex("regex", item.first.rawRegex); - } - - BSONObjBuilder subexpressionBuilder(itemBuilder.subobjStart("expression")); - item.second->getFilter()->serialize(&subexpressionBuilder, opts); - subexpressionBuilder.doneFast(); + for (auto&& [pattern, expression] : _patternProperties) { + patternPropertiesBuilder << BSON( + "regex" << opts.serializeLiteral(BSONRegEx(pattern.rawRegex)) << "expression" + << expression->getFilter()->serialize(opts)); } patternPropertiesBuilder.doneFast(); diff --git a/src/mongo/db/matcher/schema/expression_internal_schema_eq.cpp b/src/mongo/db/matcher/schema/expression_internal_schema_eq.cpp index 43c0777f587..c41a1e51c4d 100644 --- a/src/mongo/db/matcher/schema/expression_internal_schema_eq.cpp +++ b/src/mongo/db/matcher/schema/expression_internal_schema_eq.cpp @@ -64,20 +64,15 @@ void InternalSchemaEqMatchExpression::debugString(StringBuilder& debug, BSONObj InternalSchemaEqMatchExpression::getSerializedRightHandSide( SerializationOptions opts) const { - BSONObjBuilder eqObj; - if (opts.redactIdentifiers || opts.replacementForLiteralArgs) { - if (_rhsElem.isABSONObj()) { - BSONObjBuilder exprSpec(eqObj.subobjStart(kName)); - opts.redactObjToBuilder(&exprSpec, _rhsElem.Obj()); - exprSpec.done(); - return eqObj.obj(); - } else if (opts.replacementForLiteralArgs) { - // If the element is not an object it must be a literal. - return BSON(kName << opts.replacementForLiteralArgs.get()); - } + if (opts.literalPolicy != LiteralSerializationPolicy::kUnchanged && _rhsElem.isABSONObj()) { + BSONObjBuilder eqObj; + BSONObjBuilder exprSpec(eqObj.subobjStart(kName)); + opts.redactObjToBuilder(&exprSpec, _rhsElem.Obj()); + exprSpec.done(); + return eqObj.obj(); } - eqObj.appendAs(_rhsElem, kName); - return eqObj.obj(); + // If the element is not an object it must be a literal. + return BSON(kName << opts.serializeLiteral(_rhsElem)); } bool InternalSchemaEqMatchExpression::equivalent(const MatchExpression* other) const { diff --git a/src/mongo/db/matcher/schema/expression_internal_schema_fmod.cpp b/src/mongo/db/matcher/schema/expression_internal_schema_fmod.cpp index 93280d7cb5a..cafbd4784a8 100644 --- a/src/mongo/db/matcher/schema/expression_internal_schema_fmod.cpp +++ b/src/mongo/db/matcher/schema/expression_internal_schema_fmod.cpp @@ -73,18 +73,8 @@ void InternalSchemaFmodMatchExpression::debugString(StringBuilder& debug, BSONObj InternalSchemaFmodMatchExpression::getSerializedRightHandSide( SerializationOptions opts) const { - BSONObjBuilder objMatchBob; - BSONArrayBuilder arrBuilder(objMatchBob.subarrayStart("$_internalSchemaFmod")); - // Divisor and Remainder are always literals. - if (opts.replacementForLiteralArgs) { - arrBuilder.append(opts.replacementForLiteralArgs.get()); - arrBuilder.append(opts.replacementForLiteralArgs.get()); - } else { - arrBuilder.append(_divisor); - arrBuilder.append(_remainder); - } - arrBuilder.doneFast(); - return objMatchBob.obj(); + return BSON("$_internalSchemaFmod"_sd << BSON_ARRAY(opts.serializeLiteral(_divisor) + << opts.serializeLiteral(_remainder))); } bool InternalSchemaFmodMatchExpression::equivalent(const MatchExpression* other) const { diff --git a/src/mongo/db/matcher/schema/expression_internal_schema_match_array_index.cpp b/src/mongo/db/matcher/schema/expression_internal_schema_match_array_index.cpp index fd1ce5f670e..e280ebc54a9 100644 --- a/src/mongo/db/matcher/schema/expression_internal_schema_match_array_index.cpp +++ b/src/mongo/db/matcher/schema/expression_internal_schema_match_array_index.cpp @@ -68,28 +68,11 @@ bool InternalSchemaMatchArrayIndexMatchExpression::equivalent(const MatchExpress BSONObj InternalSchemaMatchArrayIndexMatchExpression::getSerializedRightHandSide( SerializationOptions opts) const { - BSONObjBuilder objBuilder; - { - BSONObjBuilder matchArrayElemSubobj(objBuilder.subobjStart(kName)); - if (opts.replacementForLiteralArgs) { - matchArrayElemSubobj.append("index", opts.replacementForLiteralArgs.get()); - } else { - matchArrayElemSubobj.append("index", _index); - } - if (auto placeHolder = _expression->getPlaceholder()) { - matchArrayElemSubobj.append("namePlaceholder", - opts.serializeFieldPathFromString(placeHolder.get())); - } else { - matchArrayElemSubobj.append("namePlaceholder", ""); - } - { - BSONObjBuilder subexprSubObj(matchArrayElemSubobj.subobjStart("expression")); - _expression->getFilter()->serialize(&subexprSubObj, opts); - subexprSubObj.doneFast(); - } - matchArrayElemSubobj.doneFast(); - } - return objBuilder.obj(); + return BSON( + kName << BSON( + "index" << opts.serializeLiteral(_index) << "namePlaceholder" + << opts.serializeFieldPathFromString(_expression->getPlaceholder().value_or("")) + << "expression" << _expression->getFilter()->serialize(opts))); } std::unique_ptr<MatchExpression> InternalSchemaMatchArrayIndexMatchExpression::clone() const { diff --git a/src/mongo/db/matcher/schema/expression_internal_schema_num_array_items.cpp b/src/mongo/db/matcher/schema/expression_internal_schema_num_array_items.cpp index 3ef7921faf3..c552364b923 100644 --- a/src/mongo/db/matcher/schema/expression_internal_schema_num_array_items.cpp +++ b/src/mongo/db/matcher/schema/expression_internal_schema_num_array_items.cpp @@ -52,13 +52,7 @@ void InternalSchemaNumArrayItemsMatchExpression::debugString(StringBuilder& debu BSONObj InternalSchemaNumArrayItemsMatchExpression::getSerializedRightHandSide( SerializationOptions opts) const { - BSONObjBuilder objBuilder; - if (opts.replacementForLiteralArgs) { - objBuilder.append(_name, opts.replacementForLiteralArgs.get()); - } else { - objBuilder.append(_name, _numItems); - } - return objBuilder.obj(); + return BSON(_name << opts.serializeLiteral(_numItems)); } bool InternalSchemaNumArrayItemsMatchExpression::equivalent(const MatchExpression* other) const { diff --git a/src/mongo/db/matcher/schema/expression_internal_schema_num_properties.cpp b/src/mongo/db/matcher/schema/expression_internal_schema_num_properties.cpp index 94fa8206616..8a74f69325a 100644 --- a/src/mongo/db/matcher/schema/expression_internal_schema_num_properties.cpp +++ b/src/mongo/db/matcher/schema/expression_internal_schema_num_properties.cpp @@ -44,11 +44,7 @@ void InternalSchemaNumPropertiesMatchExpression::debugString(StringBuilder& debu void InternalSchemaNumPropertiesMatchExpression::serialize(BSONObjBuilder* out, SerializationOptions opts) const { - if (opts.replacementForLiteralArgs) { - out->append(_name, opts.replacementForLiteralArgs.get()); - } else { - out->append(_name, _numProperties); - } + opts.appendLiteral(out, _name, _numProperties); } bool InternalSchemaNumPropertiesMatchExpression::equivalent(const MatchExpression* other) const { diff --git a/src/mongo/db/matcher/schema/expression_internal_schema_object_match.cpp b/src/mongo/db/matcher/schema/expression_internal_schema_object_match.cpp index 8e8bf6c6f7a..760a57b1102 100644 --- a/src/mongo/db/matcher/schema/expression_internal_schema_object_match.cpp +++ b/src/mongo/db/matcher/schema/expression_internal_schema_object_match.cpp @@ -64,11 +64,7 @@ void InternalSchemaObjectMatchExpression::debugString(StringBuilder& debug, BSONObj InternalSchemaObjectMatchExpression::getSerializedRightHandSide( SerializationOptions opts) const { - BSONObjBuilder objMatchBob; - BSONObjBuilder subBob(objMatchBob.subobjStart(kName)); - _sub->serialize(&subBob, opts); - subBob.doneFast(); - return objMatchBob.obj(); + return BSON(kName << _sub->serialize(opts)); } bool InternalSchemaObjectMatchExpression::equivalent(const MatchExpression* other) const { diff --git a/src/mongo/db/matcher/schema/expression_internal_schema_str_length.cpp b/src/mongo/db/matcher/schema/expression_internal_schema_str_length.cpp index 114e24e3ca9..54cc8718d97 100644 --- a/src/mongo/db/matcher/schema/expression_internal_schema_str_length.cpp +++ b/src/mongo/db/matcher/schema/expression_internal_schema_str_length.cpp @@ -52,13 +52,7 @@ void InternalSchemaStrLengthMatchExpression::debugString(StringBuilder& debug, BSONObj InternalSchemaStrLengthMatchExpression::getSerializedRightHandSide( SerializationOptions opts) const { - BSONObjBuilder objBuilder; - if (opts.replacementForLiteralArgs) { - objBuilder.append(_name, opts.replacementForLiteralArgs.get()); - } else { - objBuilder.append(_name, _strLen); - } - return objBuilder.obj(); + return BSON(_name << opts.serializeLiteral(_strLen)); } bool InternalSchemaStrLengthMatchExpression::equivalent(const MatchExpression* other) const { diff --git a/src/mongo/db/pipeline/accumulator_test.cpp b/src/mongo/db/pipeline/accumulator_test.cpp index 5a38b558fb7..de86cf48b95 100644 --- a/src/mongo/db/pipeline/accumulator_test.cpp +++ b/src/mongo/db/pipeline/accumulator_test.cpp @@ -1745,8 +1745,10 @@ Value parseAndSerializeAccumExpr( std::function<boost::intrusive_ptr<Expression>( ExpressionContext* expCtx, BSONElement, const VariablesParseState&)> func) { SerializationOptions options; + // TODO SERVER-75399 Use only 'literalPolicy.' std::string replacementChar = "?"; options.replacementForLiteralArgs = replacementChar; + options.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; options.redactIdentifiers = true; options.identifierRedactionPolicy = redactFieldNameForTest; auto expCtx = make_intrusive<ExpressionContextForTest>(); @@ -1759,8 +1761,10 @@ Document parseAndSerializeAccum( std::function<AccumulationExpression( ExpressionContext* const expCtx, BSONElement, VariablesParseState)> func) { SerializationOptions options; + // TODO SERVER-75399 Use only 'literalPolicy.' std::string replacementChar = "?"; options.replacementForLiteralArgs = replacementChar; + options.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; options.redactIdentifiers = true; options.identifierRedactionPolicy = redactFieldNameForTest; auto expCtx = make_intrusive<ExpressionContextForTest>(); @@ -1789,9 +1793,7 @@ TEST(Accumulators, SerializeWithRedaction) { R"({ "$accumulator": { "init": "?", - "initArgs": { - "$const": "?" - }, + "initArgs": "[]", "accumulate": "?", "accumulateArgs": [ "$HASH<a>", @@ -1811,9 +1813,7 @@ TEST(Accumulators, SerializeWithRedaction) { ASSERT_DOCUMENT_EQ_AUTO( // NOLINT R"({ "$topN": { - "n": { - "$const": "?" - }, + "n": "?number", "output": { "HASH<output>": "$HASH<output>", "HASH<sortFields>": [ @@ -1831,34 +1831,21 @@ TEST(Accumulators, SerializeWithRedaction) { actual = parseAndSerializeAccum(addToSet.firstElement(), &genericParseSingleExpressionAccumulator<AccumulatorAddToSet>); ASSERT_DOCUMENT_EQ_AUTO( // NOLINT - R"({"$addToSet":{"$const":"?"}})", + R"({"$addToSet":"?object"})", actual); - auto sum = BSON("$sum" << BSON_ARRAY("$a" << 5 << 3 << BSON("$sum" << BSON_ARRAY(4 << 6)))); + auto sum = BSON("$sum" << BSON_ARRAY(4 << 6)); actual = parseAndSerializeAccum(sum.firstElement(), &genericParseSingleExpressionAccumulator<AccumulatorSum>); ASSERT_DOCUMENT_EQ_AUTO( // NOLINT - R"({ - "$sum": [ - "$HASH<a>", - { - "$const": "?" - }, - { - "$const": "?" - }, - { - "$sum": [ - { - "$const": "?" - }, - { - "$const": "?" - } - ] - } - ] - })", + R"({"$sum": "?array<?number>"})", + actual); + + sum = BSON("$sum" << BSON_ARRAY("$a" << 5 << 3 << BSON("$sum" << BSON_ARRAY(4 << 6)))); + actual = parseAndSerializeAccum(sum.firstElement(), + &genericParseSingleExpressionAccumulator<AccumulatorSum>); + ASSERT_DOCUMENT_EQ_AUTO( // NOLINT + R"({"$sum":["$HASH<a>","?number","?number",{"$sum":"?array<?number>"}]})", actual); auto mergeObjs = BSON("$mergeObjects" << BSON_ARRAY("$a" << BSON("b" @@ -1867,7 +1854,7 @@ TEST(Accumulators, SerializeWithRedaction) { parseAndSerializeAccum(mergeObjs.firstElement(), &genericParseSingleExpressionAccumulator<AccumulatorMergeObjects>); ASSERT_DOCUMENT_EQ_AUTO( // NOLINT - R"({"$mergeObjects":["$HASH<a>",{"$const":"?"}]})", + R"({"$mergeObjects":["$HASH<a>","?object"]})", actual); auto push = BSON("$push" << BSON("$eq" << BSON_ARRAY("$str" @@ -1875,7 +1862,7 @@ TEST(Accumulators, SerializeWithRedaction) { actual = parseAndSerializeAccum(push.firstElement(), &genericParseSingleExpressionAccumulator<AccumulatorPush>); ASSERT_DOCUMENT_EQ_AUTO( // NOLINT - R"({"$push":{"$eq":["$HASH<str>",{"$const":"?"}]}})", + R"({"$push":{"$eq":["$HASH<str>","?string"]}})", actual); auto top = BSON("$top" << BSON("output" @@ -1907,22 +1894,14 @@ TEST(Accumulators, SerializeWithRedaction) { R"({ "$max": [ "$HASH<a>", - { - "$const": "?" - }, - { - "$const": "?" - }, + "?number", + "?number", { "$max": [ [ "$HASH<b>", - { - "$const": "?" - }, - { - "$const": "?" - } + "?number", + "?number" ] ] } @@ -1948,7 +1927,7 @@ TEST(AccumulatorsToExpression, SerializeWithRedaction) { auto actual = parseAndSerializeAccumExpr(maxN, &AccumulatorMinMaxN::parseExpression<Sense::kMax>); ASSERT_DOCUMENT_EQ_AUTO( // NOLINT - R"({"$maxN":{"n":{"$const":"?"},"input":{"$const":"?"}}})", + R"({"$maxN":{"n":"?number","input":"?array<?number>"}})", actual.getDocument()); auto firstN = BSON("$firstN" << BSON("input" @@ -1959,7 +1938,7 @@ TEST(AccumulatorsToExpression, SerializeWithRedaction) { actual = parseAndSerializeAccumExpr( firstN, &AccumulatorFirstLastN::parseExpression<FirstLastSense::kFirst>); ASSERT_DOCUMENT_EQ_AUTO( // NOLINT - R"({"$firstN":{"n":{"$const":"?"},"input":"$HASH<sales>"}})", + R"({"$firstN":{"n":"?string","input":"$HASH<sales>"}})", actual.getDocument()); } diff --git a/src/mongo/db/pipeline/aggregation_context_fixture.h b/src/mongo/db/pipeline/aggregation_context_fixture.h index 51c5b97a002..fdc7bc2f7b6 100644 --- a/src/mongo/db/pipeline/aggregation_context_fixture.h +++ b/src/mongo/db/pipeline/aggregation_context_fixture.h @@ -83,6 +83,8 @@ public: SerializationOptions options; options.verbosity = verbosity; if (performRedaction) { + options.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; + // TODO SERVER-75399 Use only 'literalPolicy.' options.replacementForLiteralArgs = "?"; options.identifierRedactionPolicy = [](StringData s) -> std::string { return str::stream() << "HASH<" << s << ">"; @@ -99,7 +101,9 @@ public: bool performRedaction = true) { SerializationOptions options; if (performRedaction) { + // TODO SERVER-75399 Use only 'literalPolicy.' options.replacementForLiteralArgs = "?"; + options.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; options.identifierRedactionPolicy = [](StringData s) -> std::string { return str::stream() << "HASH<" << s << ">"; }; diff --git a/src/mongo/db/pipeline/document_source_bucket_auto_test.cpp b/src/mongo/db/pipeline/document_source_bucket_auto_test.cpp index 20d3ff3bb6c..1b12b23a252 100644 --- a/src/mongo/db/pipeline/document_source_bucket_auto_test.cpp +++ b/src/mongo/db/pipeline/document_source_bucket_auto_test.cpp @@ -872,9 +872,7 @@ TEST_F(BucketAutoTests, RedactionWithoutOutputField) { "granularity": "?", "output": { "HASH<count>": { - "$sum": { - "$const": "?" - } + "$sum": "?number" } } } @@ -891,8 +889,7 @@ TEST_F(BucketAutoTests, RedactionWithOutputField) { count: { $sum: 1 }, years: { $push: '$year' } } - } - })"); + }})"); auto docSource = DocumentSourceBucketAuto::createFromBson(spec.firstElement(), getExpCtx()); ASSERT_BSONOBJ_EQ_AUTO( // NOLINT @@ -902,9 +899,11 @@ TEST_F(BucketAutoTests, RedactionWithOutputField) { "buckets": "?", "output": { "HASH<count>": { - $sum: { "$const": "?" } + "$sum": "?number" }, - "HASH<years>": { $push: "$HASH<year>" } + "HASH<years>": { + "$push": "$HASH<year>" + } } } })", diff --git a/src/mongo/db/pipeline/document_source_documents_test.cpp b/src/mongo/db/pipeline/document_source_documents_test.cpp index e8c3d181ff8..f41d2021c76 100644 --- a/src/mongo/db/pipeline/document_source_documents_test.cpp +++ b/src/mongo/db/pipeline/document_source_documents_test.cpp @@ -50,14 +50,20 @@ TEST_F(DocumentSourceDocumentsTest, DocumentsStageRedactsCorrectly) { auto unwindStage = static_cast<DocumentSourceUnwind*>(docSourcesVec[2].get()); ASSERT(unwindStage); auto generatedField = unwindStage->getUnwindPath(); - ASSERT_BSONOBJ_EQ_AUTO( // NOLINT "{$queue: '?'}", redact(*docSourcesVec[0])); - ASSERT_BSONOBJ_EQ_AUTO( // NOLINT - "{'$project': {'HASH<_id>': true, 'HASH<" + generatedField + ">': { $const: '?' }}}", + ASSERT_BSONOBJ_EQ( // NOLINT + fromjson(R"({ + "$project": { + "HASH<_id>": true, + "HASH<)" + + generatedField + + R"(>": "?array<?object>" + } + })"), redact(*docSourcesVec[1])); diff --git a/src/mongo/db/pipeline/document_source_facet_test.cpp b/src/mongo/db/pipeline/document_source_facet_test.cpp index c06eac57679..73d11545ba6 100644 --- a/src/mongo/db/pipeline/document_source_facet_test.cpp +++ b/src/mongo/db/pipeline/document_source_facet_test.cpp @@ -954,9 +954,7 @@ TEST_F(DocumentSourceFacetTest, RedactsCorrectly) { "$group": { "_id": "$HASH<foo>", "HASH<count>": { - "$sum": { - "$const": "?" - } + "$sum": "?number" } } }, @@ -970,7 +968,7 @@ TEST_F(DocumentSourceFacetTest, RedactsCorrectly) { { "$match": { "HASH<bar>": { - "$exists": "?" + "$exists": "?bool" } } }, @@ -985,24 +983,18 @@ TEST_F(DocumentSourceFacetTest, RedactsCorrectly) { { "$gte": [ "$HASH<bar>.HASH<foo>", - { - "$const": "?" - } + "?number" ] }, { "$lt": [ "$HASH<bar>.HASH<foo>", - { - "$const": "?" - } + "?number" ] } ] }, - "then": { - "$const": "?" - } + "then": "?number" }, { "case": { @@ -1010,24 +1002,18 @@ TEST_F(DocumentSourceFacetTest, RedactsCorrectly) { { "$gte": [ "$HASH<bar>.HASH<foo>", - { - "$const": "?" - } + "?number" ] }, { "$lt": [ "$HASH<bar>.HASH<foo>", - { - "$const": "?" - } + "?number" ] } ] }, - "then": { - "$const": "?" - } + "then": "?number" }, { "case": { @@ -1035,32 +1021,24 @@ TEST_F(DocumentSourceFacetTest, RedactsCorrectly) { { "$gte": [ "$HASH<bar>.HASH<foo>", - { - "$const": "?" - } + "?number" ] }, { "$lt": [ "$HASH<bar>.HASH<foo>", - { - "$const": "?" - } + "?number" ] } ] }, - "then": { - "$const": "?" - } + "then": "?number" } ] } }, "HASH<z>": { - "$sum": { - "$const": "?" - } + "$sum": "?number" } } }, @@ -1077,9 +1055,7 @@ TEST_F(DocumentSourceFacetTest, RedactsCorrectly) { "buckets": "?", "output": { "HASH<count>": { - "$sum": { - "$const": "?" - } + "$sum": "?number" } } } diff --git a/src/mongo/db/pipeline/document_source_geo_near_test.cpp b/src/mongo/db/pipeline/document_source_geo_near_test.cpp index 714f98c1908..fb752983c82 100644 --- a/src/mongo/db/pipeline/document_source_geo_near_test.cpp +++ b/src/mongo/db/pipeline/document_source_geo_near_test.cpp @@ -130,15 +130,13 @@ TEST_F(DocumentSourceGeoNearTest, RedactionWithGeoJSONPoint) { ASSERT_BSONOBJ_EQ_AUTO( // NOLINT R"({ "$geoNear": { - "near": { - "$const": "?" - }, + "near": "?object", "distanceField": "HASH<a>", "maxDistance": "?", "minDistance": "?", "query": { "HASH<foo>": { - "$eq": "?" + "$eq": "?string" } }, "spherical": "?" @@ -162,9 +160,7 @@ TEST_F(DocumentSourceGeoNearTest, RedactionWithGeoJSONLineString) { ASSERT_BSONOBJ_EQ_AUTO( // NOLINT R"({ "$geoNear": { - "near": { - "$const": "?" - }, + "near": "?object", "distanceField": "HASH<a>", "minDistance": "?", "query": {}, @@ -193,13 +189,11 @@ TEST_F(DocumentSourceGeoNearTest, RedactionWithLegacyCoordinates) { R"({ "$geoNear": { "key": "HASH<z>", - "near": { - "$const": "?" - }, + "near": "?array<?number>", "distanceField": "HASH<foo>", "query": { "HASH<a>": { - "$gt": "?" + "$gt": "?number" } }, "spherical": "?", diff --git a/src/mongo/db/pipeline/document_source_graph_lookup_test.cpp b/src/mongo/db/pipeline/document_source_graph_lookup_test.cpp index 34e2a163eb4..cf1d045a35a 100644 --- a/src/mongo/db/pipeline/document_source_graph_lookup_test.cpp +++ b/src/mongo/db/pipeline/document_source_graph_lookup_test.cpp @@ -816,8 +816,16 @@ TEST_F(DocumentSourceGraphLookUpTest, RedactionStartWithSingleField) { "maxDepth": "?", "restrictSearchWithMatch": { "$and": [ - { "HASH<foo>": { "$eq": "?" } }, - { "HASH<bar>.HASH<baz>": { "$gt": "?" } } + { + "HASH<foo>": { + "$eq": "?string" + } + }, + { + "HASH<bar>.HASH<baz>": { + "$gt": "?number" + } + } ] } } diff --git a/src/mongo/db/pipeline/document_source_group_test.cpp b/src/mongo/db/pipeline/document_source_group_test.cpp index 889e68e9911..df839e0a35e 100644 --- a/src/mongo/db/pipeline/document_source_group_test.cpp +++ b/src/mongo/db/pipeline/document_source_group_test.cpp @@ -254,18 +254,7 @@ TEST_F(DocumentSourceGroupTest, GroupRedactsCorrectWithIdNull) { })"); auto docSource = DocumentSourceGroup::createFromBson(spec.firstElement(), getExpCtx()); ASSERT_BSONOBJ_EQ_AUTO( // NOLINT - R"({ - "$group": { - "_id": { - "$const": "?" - }, - "HASH<foo>": { - "$sum": { - "$const": "?" - } - } - } - })", + R"({"$group":{"_id":"?null","HASH<foo>":{"$sum":"?number"}}})", redact(*docSource)); } diff --git a/src/mongo/db/pipeline/document_source_lookup_test.cpp b/src/mongo/db/pipeline/document_source_lookup_test.cpp index 8a3fb727ea1..1f404764392 100644 --- a/src/mongo/db/pipeline/document_source_lookup_test.cpp +++ b/src/mongo/db/pipeline/document_source_lookup_test.cpp @@ -1540,7 +1540,7 @@ TEST_F(DocumentSourceLookUpTest, RedactsCorrectlyWithPipeline) { { "$match": { "HASH<a>": { - "$eq": "?" + "$eq": "?string" } } }, diff --git a/src/mongo/db/pipeline/document_source_match_test.cpp b/src/mongo/db/pipeline/document_source_match_test.cpp index e699537521d..25c3bd507a0 100644 --- a/src/mongo/db/pipeline/document_source_match_test.cpp +++ b/src/mongo/db/pipeline/document_source_match_test.cpp @@ -724,12 +724,12 @@ TEST_F(DocumentSourceMatchTest, RedactionWithAnd) { "$and": [ { "HASH<a>.HASH<c>": { - "$eq": "?" + "$eq": "?string" } }, { "HASH<b>": { - "$gt": "?" + "$gt": "?number" } } ] diff --git a/src/mongo/db/pipeline/document_source_plan_cache_stats_test.cpp b/src/mongo/db/pipeline/document_source_plan_cache_stats_test.cpp index 7a78430fb94..797b09f2b41 100644 --- a/src/mongo/db/pipeline/document_source_plan_cache_stats_test.cpp +++ b/src/mongo/db/pipeline/document_source_plan_cache_stats_test.cpp @@ -160,11 +160,7 @@ TEST_F(DocumentSourcePlanCacheStatsTest, RedactsSuccessfullyAfterAbsorbingMatch) ASSERT_BSONOBJ_EQ(specObj, serialized[0].getDocument().toBson()); ASSERT_BSONOBJ_EQ_AUTO( // NOLINT - R"({ - "$match": { - "HASH<foo>": { "$eq": "?" } - } - })", + R"({"$match":{"HASH<foo>":{"$eq":"?string"}}})", serialized[1].getDocument().toBson().getOwned()); } diff --git a/src/mongo/db/pipeline/document_source_set_window_fields_test.cpp b/src/mongo/db/pipeline/document_source_set_window_fields_test.cpp index e6705d64e01..d5afe359205 100644 --- a/src/mongo/db/pipeline/document_source_set_window_fields_test.cpp +++ b/src/mongo/db/pipeline/document_source_set_window_fields_test.cpp @@ -322,9 +322,7 @@ TEST_F(DocumentSourceSetWindowFieldsTest, RedactionOnExpressionNOperator) { "output": { "HASH<b>": { "$minN": { - "n": { - "$const": "?" - }, + "n": "?number", "input": "$HASH<y>" }, "window": { diff --git a/src/mongo/db/pipeline/document_source_union_with_test.cpp b/src/mongo/db/pipeline/document_source_union_with_test.cpp index 92919e5b729..e42ad6d23f3 100644 --- a/src/mongo/db/pipeline/document_source_union_with_test.cpp +++ b/src/mongo/db/pipeline/document_source_union_with_test.cpp @@ -650,7 +650,7 @@ TEST_F(DocumentSourceUnionWithTest, RedactsCorrectlyWithPipeline) { { "$match": { "HASH<a>": { - "$eq": "?" + "$eq": "?number" } } }, diff --git a/src/mongo/db/pipeline/expression.cpp b/src/mongo/db/pipeline/expression.cpp index 6e707f1529b..b67858f9c20 100644 --- a/src/mongo/db/pipeline/expression.cpp +++ b/src/mongo/db/pipeline/expression.cpp @@ -74,12 +74,18 @@ using std::string; using std::vector; /// Helper function to easily wrap constants with $const. -static Value serializeConstant(Value val) { +Value ExpressionConstant::serializeConstant(const SerializationOptions& opts, Value val) { if (val.missing()) { return Value("$$REMOVE"_sd); } + if (opts.literalPolicy == LiteralSerializationPolicy::kToDebugTypeString) { + return opts.serializeLiteral(val); + } - return Value(DOC("$const" << val)); + // Other serialization policies need to include this $const in order to be unambiguous for + // re-parsing this output later. If for example the constant was '$cashMoney' - we don't want to + // misinterpret it as a field path when parsing. + return opts.serializeLiteral(Value(DOC("$const" << val))); } /* --------------------------- Expression ------------------------------ */ @@ -639,8 +645,10 @@ Value ExpressionArray::evaluate(const Document& root, Variables* variables) cons } Value ExpressionArray::serialize(SerializationOptions options) const { - if (options.replacementForLiteralArgs && selfAndChildrenAreConstant()) { - return serializeConstant(Value(options.replacementForLiteralArgs.get())); + if (options.literalPolicy != LiteralSerializationPolicy::kUnchanged && + selfAndChildrenAreConstant()) { + return ExpressionConstant::serializeConstant( + options, evaluate(Document{}, &(getExpressionContext()->variables))); } vector<Value> expressions; expressions.reserve(_children.size()); @@ -1223,10 +1231,7 @@ Value ExpressionConstant::evaluate(const Document& root, Variables* variables) c } Value ExpressionConstant::serialize(SerializationOptions options) const { - if (options.replacementForLiteralArgs) { - return serializeConstant(Value(options.replacementForLiteralArgs.get())); - } - return serializeConstant(_value); + return ExpressionConstant::serializeConstant(options, _value); } REGISTER_STABLE_EXPRESSION(const, ExpressionConstant::parse); @@ -2385,8 +2390,9 @@ bool ExpressionObject::selfAndChildrenAreConstant() const { } Value ExpressionObject::serialize(SerializationOptions options) const { - if (options.replacementForLiteralArgs && selfAndChildrenAreConstant()) { - return serializeConstant(Value(options.replacementForLiteralArgs.get())); + if (options.literalPolicy != LiteralSerializationPolicy::kUnchanged && + selfAndChildrenAreConstant()) { + return ExpressionConstant::serializeConstant(options, Value(Document{})); } MutableDocument outputDoc; for (auto&& pair : _expressions) { @@ -7983,7 +7989,6 @@ Value ExpressionGetField::evaluate(const Document& root, Variables* variables) c return Value(); } - return inputValue.getDocument().getField(fieldValue.getString()); } @@ -7992,19 +7997,23 @@ intrusive_ptr<Expression> ExpressionGetField::optimize() { } Value ExpressionGetField::serialize(SerializationOptions options) const { - MutableDocument argDoc; - if (options.redactIdentifiers) { - // The parser guarantees that the '_children[_kField]' expression evaluates to a constant - // string. - auto strPath = - static_cast<ExpressionConstant*>(_children[_kField].get())->getValue().getString(); - argDoc.addField("field"_sd, Value(options.serializeFieldPathFromString(strPath))); - } else { - argDoc.addField("field"_sd, _children[_kField]->serialize(options)); + // The parser guarantees that the '_children[_kField]' expression evaluates to a constant + // string. + auto strPath = + static_cast<ExpressionConstant*>(_children[_kField].get())->getValue().getString(); + + Value maybeRedactedPath{options.serializeFieldPathFromString(strPath)}; + // This is a pretty unique option to serialize. It is both a constant and a field path, which + // means that it: + // - should be redacted (if that option is set). + // - should *not* be wrapped in $const iff we are serializing for a debug string + if (options.literalPolicy != LiteralSerializationPolicy::kToDebugTypeString) { + maybeRedactedPath = Value(Document{{"$const"_sd, maybeRedactedPath}}); } - argDoc.addField("input"_sd, _children[_kInput]->serialize(options)); - return Value(Document{{"$getField"_sd, argDoc.freezeToValue()}}); + return Value(Document{{"$getField"_sd, + Document{{"field"_sd, std::move(maybeRedactedPath)}, + {"input"_sd, _children[_kInput]->serialize(options)}}}}); } /* -------------------------- ExpressionSetField ------------------------------ */ @@ -8115,20 +8124,24 @@ intrusive_ptr<Expression> ExpressionSetField::optimize() { } Value ExpressionSetField::serialize(SerializationOptions options) const { - MutableDocument argDoc; - if (options.redactIdentifiers) { - // The parser guarantees that the '_children[_kField]' expression evaluates to a constant - // string. - auto strPath = - static_cast<ExpressionConstant*>(_children[_kField].get())->getValue().getString(); - argDoc.addField("field"_sd, Value(options.serializeFieldPathFromString(strPath))); - } else { - argDoc.addField("field"_sd, _children[_kField]->serialize(options)); - } - argDoc.addField("input"_sd, _children[_kInput]->serialize(options)); - argDoc.addField("value"_sd, _children[_kValue]->serialize(options)); - - return Value(Document{{"$setField"_sd, argDoc.freezeToValue()}}); + // The parser guarantees that the '_children[_kField]' expression evaluates to a constant + // string. + auto strPath = + static_cast<ExpressionConstant*>(_children[_kField].get())->getValue().getString(); + + Value maybeRedactedPath{options.serializeFieldPathFromString(strPath)}; + // This is a pretty unique option to serialize. It is both a constant and a field path, which + // means that it: + // - should be redacted (if that option is set). + // - should *not* be wrapped in $const iff we are serializing for a debug string + if (options.literalPolicy != LiteralSerializationPolicy::kToDebugTypeString) { + maybeRedactedPath = Value(Document{{"$const"_sd, maybeRedactedPath}}); + } + + return Value(Document{{"$setField"_sd, + Document{{"field"_sd, std::move(maybeRedactedPath)}, + {"input"_sd, _children[_kInput]->serialize(options)}, + {"value"_sd, _children[_kValue]->serialize(options)}}}}); } /* ------------------------- ExpressionTsSecond ----------------------------- */ diff --git a/src/mongo/db/pipeline/expression.h b/src/mongo/db/pipeline/expression.h index fc0a8a0fa61..297592a2cf7 100644 --- a/src/mongo/db/pipeline/expression.h +++ b/src/mongo/db/pipeline/expression.h @@ -378,6 +378,93 @@ private: }; /** + * A constant expression. Repeated calls to evaluate() will always return the same thing. + */ +class ExpressionConstant final : public Expression { +public: + ExpressionConstant(ExpressionContext* expCtx, const Value& value); + + boost::intrusive_ptr<Expression> optimize() final; + Value evaluate(const Document& root, Variables* variables) const final; + Value serialize(SerializationOptions options) const final; + + const char* getOpName() const; + + /** + * Creates a new ExpressionConstant with value 'value'. + */ + static boost::intrusive_ptr<ExpressionConstant> create(ExpressionContext* expCtx, + const Value& value); + + static boost::intrusive_ptr<Expression> parse(ExpressionContext* expCtx, + BSONElement bsonExpr, + const VariablesParseState& vps); + + /** + * Returns true if 'expression' is nullptr or if 'expression' is an instance of an + * ExpressionConstant. + */ + static bool isNullOrConstant(boost::intrusive_ptr<Expression> expression) { + return !expression || dynamic_cast<ExpressionConstant*>(expression.get()); + } + + /** + * Returns true if 'expression' is an instance of an ExpressionConstant. + */ + static bool isConstant(boost::intrusive_ptr<Expression> expression) { + return dynamic_cast<ExpressionConstant*>(expression.get()); + } + static Value serializeConstant(const SerializationOptions& opts, Value val); + + bool selfAndChildrenAreConstant() const override final { + return true; + } + + /** + * Returns true if every expression in 'expressions' is either a nullptr or an instance of an + * ExpressionConstant. + */ + static bool allNullOrConstant( + const std::initializer_list<boost::intrusive_ptr<Expression>>& expressions) { + return std::all_of(expressions.begin(), expressions.end(), [](auto exp) { + return ExpressionConstant::isNullOrConstant(exp); + }); + } + template <typename ExpressionContainer> + static bool allConstant(const ExpressionContainer& expressions) { + return std::all_of(expressions.begin(), expressions.end(), [](auto exp) { + return ExpressionConstant::isConstant(exp); + }); + } + + /** + * Returns the constant value represented by this Expression. + */ + Value getValue() const { + return _value; + } + + void setValue(const Value& value) { + _value = value; + }; + + void acceptVisitor(ExpressionMutableVisitor* visitor) final { + return visitor->visit(this); + } + + void acceptVisitor(ExpressionConstVisitor* visitor) const final { + return visitor->visit(this); + } + +private: + monotonic::State getMonotonicState(const FieldPath& sortedFieldPath) const final { + return monotonic::State::Constant; + } + + Value _value; +}; + +/** * Inherit from ExpressionVariadic or ExpressionFixedArity instead of directly from this class. */ class ExpressionNary : public Expression { @@ -448,6 +535,33 @@ public: : ExpressionNaryBase<SubClass>(expCtx) {} ExpressionVariadic(ExpressionContext* const expCtx, Expression::ExpressionVector&& children) : ExpressionNaryBase<SubClass>(expCtx, std::move(children)) {} + + Value serialize(SerializationOptions options) const { + // As a special case, we would like to serialize a variadic number of children as + // "?array<?subtype>" if they are all constant. Check for that here, otherwise default to + // the normal one-by-one serialization of the children. + if (options.literalPolicy == LiteralSerializationPolicy::kToDebugTypeString && + ExpressionConstant::allConstant(this->_children)) { + // We could evaluate the expression right here and now and end up with just the one + // constant answer, but this is not an optimization funciton, it is meant to just + // serialize what we have, so let's preserve the array of constants. + auto args = [&]() { + std::vector<Value> values; + const auto& constants = this->_children; + values.reserve(constants.size()); + std::transform(constants.begin(), + constants.end(), + std::back_inserter(values), + [](const auto& exp) { + return static_cast<ExpressionConstant*>(exp.get())->getValue(); + }); + return values; + }(); + return Value(Document{ + {this->getOpName(), ExpressionConstant::serializeConstant(options, Value(args))}}); + } + return ExpressionNary::serialize(options); + } }; /** @@ -753,85 +867,6 @@ public: }; /** - * A constant expression. Repeated calls to evaluate() will always return the same thing. - */ -class ExpressionConstant final : public Expression { -public: - ExpressionConstant(ExpressionContext* expCtx, const Value& value); - - boost::intrusive_ptr<Expression> optimize() final; - Value evaluate(const Document& root, Variables* variables) const final; - Value serialize(SerializationOptions options) const final; - - const char* getOpName() const; - - /** - * Creates a new ExpressionConstant with value 'value'. - */ - static boost::intrusive_ptr<ExpressionConstant> create(ExpressionContext* expCtx, - const Value& value); - - static boost::intrusive_ptr<Expression> parse(ExpressionContext* expCtx, - BSONElement bsonExpr, - const VariablesParseState& vps); - - /** - * Returns true if 'expression' is nullptr or if 'expression' is an instance of an - * ExpressionConstant. - */ - static bool isNullOrConstant(boost::intrusive_ptr<Expression> expression) { - return !expression || dynamic_cast<ExpressionConstant*>(expression.get()); - } - - /** - * Returns true if 'expression' is an instance of an ExpressionConstant. - */ - static bool isConstant(boost::intrusive_ptr<Expression> expression) { - return dynamic_cast<ExpressionConstant*>(expression.get()); - } - bool selfAndChildrenAreConstant() const override final { - return true; - } - - /** - * Returns true if every expression in 'expressions' is either a nullptr or an instance of an - * ExpressionConstant. - */ - static bool allNullOrConstant( - const std::initializer_list<boost::intrusive_ptr<Expression>>& expressions) { - return std::all_of(expressions.begin(), expressions.end(), [](auto exp) { - return ExpressionConstant::isNullOrConstant(exp); - }); - } - - /** - * Returns the constant value represented by this Expression. - */ - Value getValue() const { - return _value; - } - - void setValue(const Value& value) { - _value = value; - }; - - void acceptVisitor(ExpressionMutableVisitor* visitor) final { - return visitor->visit(this); - } - - void acceptVisitor(ExpressionConstVisitor* visitor) const final { - return visitor->visit(this); - } - -private: - monotonic::State getMonotonicState(const FieldPath& sortedFieldPath) const final { - return monotonic::State::Constant; - } - - Value _value; -}; - -/** * Inherit from this class if your expression works with date types, and accepts either a single * argument which is a date, or an object {date: <date>, timezone: <string>}. */ diff --git a/src/mongo/db/pipeline/expression_field_path_test.cpp b/src/mongo/db/pipeline/expression_field_path_test.cpp index 4b332d9e4f4..480cfe0d2fd 100644 --- a/src/mongo/db/pipeline/expression_field_path_test.cpp +++ b/src/mongo/db/pipeline/expression_field_path_test.cpp @@ -280,12 +280,11 @@ TEST(FieldPath, SerializeWithRedaction) { expression->serialize(options).getDocument()); // Test that a variable followed by user fields is properly hashed. - std::string replacementChar = "?"; - options.replacementForLiteralArgs = replacementChar; + options.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; expression = expr(R"({$gt: ["$$ROOT.a.b", 5]})"); ASSERT_DOCUMENT_EQ_AUTO( // NOLINT - R"({"$gt":["$$ROOT.HASH<a>.HASH<b>",{"$const":"?"}]})", + R"({"$gt":["$$ROOT.HASH<a>.HASH<b>","?number"]})", expression->serialize(options).getDocument()); expression = expr(R"({$gt: ["$foo", "$$NOW"]})"); diff --git a/src/mongo/db/pipeline/expression_function.cpp b/src/mongo/db/pipeline/expression_function.cpp index 4e895b3e1c5..ae7c8b10777 100644 --- a/src/mongo/db/pipeline/expression_function.cpp +++ b/src/mongo/db/pipeline/expression_function.cpp @@ -47,17 +47,17 @@ ExpressionFunction::ExpressionFunction(ExpressionContext* const expCtx, } Value ExpressionFunction::serialize(SerializationOptions options) const { - MutableDocument d; - d["body"] = options.replacementForLiteralArgs ? Value(*options.replacementForLiteralArgs) - : Value(_funcSource); - d["args"] = Value(_passedArgs->serialize(options)); - d["lang"] = Value(_lang); + MutableDocument innerOpts(Document{{"body"_sd, options.serializeLiteral(_funcSource)}, + {"args"_sd, _passedArgs->serialize(options)}, + // "lang" is purposefully not treated as a literal since it + // is more of a selection of an enum + {"lang"_sd, _lang}}); // This field will only be seralized when desugaring $where in $expr + $_internalJs if (_assignFirstArgToThis) { - d["_internalSetObjToThis"] = Value(_assignFirstArgToThis); + innerOpts["_internalSetObjToThis"] = options.serializeLiteral(_assignFirstArgToThis); } - return Value(Document{{kExpressionName, d.freezeToValue()}}); + return Value(Document{{kExpressionName, innerOpts.freezeToValue()}}); } boost::intrusive_ptr<Expression> ExpressionFunction::parse(ExpressionContext* const expCtx, diff --git a/src/mongo/db/pipeline/expression_function_test.cpp b/src/mongo/db/pipeline/expression_function_test.cpp index 640804b55b4..8c4d841a0b5 100644 --- a/src/mongo/db/pipeline/expression_function_test.cpp +++ b/src/mongo/db/pipeline/expression_function_test.cpp @@ -49,6 +49,7 @@ TEST(ExpressionFunction, SerializeAndRedactArgs) { SerializationOptions options; std::string replacementChar = "?"; + options.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; options.replacementForLiteralArgs = replacementChar; options.redactIdentifiers = true; options.identifierRedactionPolicy = redactFieldNameForTest; @@ -61,7 +62,7 @@ TEST(ExpressionFunction, SerializeAndRedactArgs) { VariablesParseState vps = expCtx.variablesParseState; auto exprFunc = ExpressionFunction::parse(&expCtx, expr.firstElement(), vps); ASSERT_DOCUMENT_EQ_AUTO( // NOLINT - R"({"$function":{"body":"?","args":["$HASH<age>"],"lang":"js"}})", + R"({"$function":{"body":"?string","args":["$HASH<age>"],"lang":"js"}})", exprFunc->serialize(options).getDocument()); } } // namespace diff --git a/src/mongo/db/pipeline/expression_test.cpp b/src/mongo/db/pipeline/expression_test.cpp index 0b1631d6bff..05209b55f63 100644 --- a/src/mongo/db/pipeline/expression_test.cpp +++ b/src/mongo/db/pipeline/expression_test.cpp @@ -822,42 +822,20 @@ TEST(ExpressionConstantTest, ConstantOfValueMissingSerializesToRemoveSystemVar) TEST(ExpressionConstantTest, ConstantRedaction) { SerializationOptions options; - std::string replacementChar = "?"; - options.replacementForLiteralArgs = replacementChar; + options.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; // Test that a constant is replaced. auto expCtx = ExpressionContextForTest{}; intrusive_ptr<Expression> expression = ExpressionConstant::create(&expCtx, Value("my_ssn"_sd)); ASSERT_BSONOBJ_EQ_AUTO( // NOLINT - R"({"field":{"$const":"?"}})", + R"({"field":"?string"})", BSON("field" << expression->serialize(options))); auto expressionBSON = BSON("$and" << BSON_ARRAY(BSON("$gt" << BSON_ARRAY("$foo" << 5)) << BSON("$lt" << BSON_ARRAY("$foo" << 10)))); expression = Expression::parseExpression(&expCtx, expressionBSON, expCtx.variablesParseState); ASSERT_BSONOBJ_EQ_AUTO( // NOLINT - R"({ - "field": { - "$and": [ - { - "$gt": [ - "$foo", - { - "$const": "?" - } - ] - }, - { - "$lt": [ - "$foo", - { - "$const": "?" - } - ] - } - ] - } - })", + R"({"field":{"$and":[{"$gt":["$foo","?number"]},{"$lt":["$foo","?number"]}]}})", BSON("field" << expression->serialize(options))); } @@ -3707,11 +3685,18 @@ TEST(ExpressionGetFieldTest, GetFieldSerializesStringArgumentCorrectly) { VariablesParseState vps = expCtx.variablesParseState; BSONObj expr = fromjson("{$meta: \"foo\"}"); auto expression = ExpressionGetField::parse(&expCtx, expr.firstElement(), vps); - ASSERT_BSONOBJ_EQ(BSON("ignoredField" << BSON("$getField" << BSON("field" << BSON("$const" - << "foo") - << "input" - << "$$CURRENT"))), - BSON("ignoredField" << expression->serialize(false))); + ASSERT_BSONOBJ_EQ_AUTO( // NOLINT + R"({ + "ignoredField": { + "$getField": { + "field": { + "$const": "foo" + }, + "input": "$$CURRENT" + } + } + })", + BSON("ignoredField" << expression->serialize(false))); } TEST(ExpressionGetFieldTest, GetFieldSerializesCorrectly) { @@ -3719,20 +3704,29 @@ TEST(ExpressionGetFieldTest, GetFieldSerializesCorrectly) { VariablesParseState vps = expCtx.variablesParseState; BSONObj expr = fromjson("{$meta: {\"field\": \"foo\", \"input\": {a: 1}}}"); auto expression = ExpressionGetField::parse(&expCtx, expr.firstElement(), vps); - ASSERT_BSONOBJ_EQ( - BSON("ignoredField" << BSON( - "$getField" << BSON("field" << BSON("$const" - << "foo") - << "input" << BSON("a" << BSON("$const" << 1))))), + ASSERT_BSONOBJ_EQ_AUTO( // NOLINT + R"({ + "ignoredField": { + "$getField": { + "field": { + "$const": "foo" + }, + "input": { + "a": { + "$const": 1 + } + } + } + } + })", BSON("ignoredField" << expression->serialize(false))); } TEST(ExpressionGetFieldTest, GetFieldSerializesAndRedactsCorrectly) { SerializationOptions options; - std::string replacementChar = "?"; - options.replacementForLiteralArgs = replacementChar; - options.identifierRedactionPolicy = redactFieldNameForTest; + options.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; options.redactIdentifiers = true; + options.identifierRedactionPolicy = redactFieldNameForTest; auto expCtx = ExpressionContextForTest{}; VariablesParseState vps = expCtx.variablesParseState; @@ -3770,12 +3764,43 @@ TEST(ExpressionGetFieldTest, GetFieldSerializesAndRedactsCorrectly) { } })", BSON("field" << expression->serialize(options))); + + // Test a field with a '$' character. + expressionBSON = BSON("$getField" + << "a.$b.c"); + + expression = ExpressionGetField::parse(&expCtx, expressionBSON.firstElement(), vps); + ASSERT_BSONOBJ_EQ_AUTO( // NOLINT + R"({ + "field": { + "$getField": { + "field": "HASH<dollarPlaceholder>", + "input": "$$CURRENT" + } + } + })", + BSON("field" << expression->serialize(options))); + + // Test a field with a trailing '.' character (invalid FieldPath). + expressionBSON = BSON("$getField" + << "a.b.c."); + + expression = ExpressionGetField::parse(&expCtx, expressionBSON.firstElement(), vps); + ASSERT_BSONOBJ_EQ_AUTO( // NOLINT + R"({ + "field": { + "$getField": { + "field": "HASH<dollarPlaceholder>", + "input": "$$CURRENT" + } + } + })", + BSON("field" << expression->serialize(options))); } TEST(ExpressionSetFieldTest, SetFieldRedactsCorrectly) { SerializationOptions options; - std::string replacementChar = "?"; - options.replacementForLiteralArgs = replacementChar; + options.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; options.identifierRedactionPolicy = redactFieldNameForTest; options.redactIdentifiers = true; auto expCtx = ExpressionContextForTest{}; @@ -3811,12 +3836,8 @@ TEST(ExpressionSetFieldTest, SetFieldRedactsCorrectly) { "field": { "$setField": { "field": "HASH<a>", - "input": { - "$const": "?" - }, - "value": { - "$const": "?" - } + "input": "?object", + "value": "?number" } } })", @@ -3833,12 +3854,8 @@ TEST(ExpressionSetFieldTest, SetFieldRedactsCorrectly) { "field": { "$setField": { "field": "HASH<a>", - "input": { - "$const": "?" - }, - "value": { - "$const": "?" - } + "input": "?object", + "value": "?number" } } })", @@ -3860,9 +3877,7 @@ TEST(ExpressionSetFieldTest, SetFieldRedactsCorrectly) { "input": { "HASH<a>": "$HASH<field>" }, - "value": { - "$const": "?" - } + "value": "?number" } } })", @@ -3883,9 +3898,7 @@ TEST(ExpressionSetFieldTest, SetFieldRedactsCorrectly) { "field": { "$setField": { "field": "HASH<a>", - "input": { - "$const": "?" - }, + "input": "?object", "value": { "HASH<c>": "$HASH<d>" } @@ -3905,12 +3918,8 @@ TEST(ExpressionSetFieldTest, SetFieldRedactsCorrectly) { "field": { "$setField": { "field": "HASH<a>", - "input": { - "$const": "?" - }, - "value": { - "$const": "?" - } + "input": "?object", + "value": "?number" } } })", diff --git a/src/mongo/db/query/projection_ast_test.cpp b/src/mongo/db/query/projection_ast_test.cpp index ee8f4fd7e6f..98655778be3 100644 --- a/src/mongo/db/query/projection_ast_test.cpp +++ b/src/mongo/db/query/projection_ast_test.cpp @@ -780,6 +780,7 @@ std::string redactFieldNameForTest(StringData s) { TEST_F(ProjectionASTTest, TestASTRedaction) { SerializationOptions options; options.replacementForLiteralArgs = "?"; + options.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; options.redactIdentifiers = true; options.identifierRedactionPolicy = redactFieldNameForTest; @@ -806,7 +807,7 @@ TEST_F(ProjectionASTTest, TestASTRedaction) { proj = fromjson("{f: {$elemMatch: {foo: 'bar'}}}"); output = projection_ast::serialize(*parseWithFindFeaturesEnabled(proj).root(), options); ASSERT_BSONOBJ_EQ_AUTO( // - R"({"HASH<f>":{"$elemMatch":{"HASH<foo>":{"$eq":"?"}}},"HASH<_id>":true})", + R"({"HASH<f>":{"$elemMatch":{"HASH<foo>":{"$eq":"?string"}}},"HASH<_id>":true})", output); // Positional projection diff --git a/src/mongo/db/query/query_shape.cpp b/src/mongo/db/query/query_shape.cpp index 4f5bddc1788..cf690090d09 100644 --- a/src/mongo/db/query/query_shape.cpp +++ b/src/mongo/db/query/query_shape.cpp @@ -31,16 +31,31 @@ namespace mongo::query_shape { -BSONObj predicateShape(const MatchExpression* predicate) { +BSONObj debugPredicateShape(const MatchExpression* predicate) { SerializationOptions opts; - opts.replacementForLiteralArgs = kLiteralArgString; + opts.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; + return predicate->serialize(opts); +} +BSONObj representativePredicateShape(const MatchExpression* predicate) { + SerializationOptions opts; + opts.literalPolicy = LiteralSerializationPolicy::kToRepresentativeParseableValue; + return predicate->serialize(opts); +} + +BSONObj debugPredicateShape(const MatchExpression* predicate, + std::function<std::string(StringData)> identifierRedactionPolicy) { + SerializationOptions opts; + opts.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; + opts.identifierRedactionPolicy = identifierRedactionPolicy; + opts.redactIdentifiers = true; return predicate->serialize(opts); } -BSONObj predicateShape(const MatchExpression* predicate, - std::function<std::string(StringData)> identifierRedactionPolicy) { +BSONObj representativePredicateShape( + const MatchExpression* predicate, + std::function<std::string(StringData)> identifierRedactionPolicy) { SerializationOptions opts; - opts.replacementForLiteralArgs = kLiteralArgString; + opts.literalPolicy = LiteralSerializationPolicy::kToRepresentativeParseableValue; opts.identifierRedactionPolicy = identifierRedactionPolicy; opts.redactIdentifiers = true; return predicate->serialize(opts); diff --git a/src/mongo/db/query/query_shape.h b/src/mongo/db/query/query_shape.h index 3ec7c6696b6..a556c76efc4 100644 --- a/src/mongo/db/query/query_shape.h +++ b/src/mongo/db/query/query_shape.h @@ -49,9 +49,13 @@ constexpr StringData kLiteralArgString = "?"_sd; * TODO better consider how this interacts with persistent query settings project, and document it. * TODO (TODO SERVER ticket) better distinguish this from a plan cache or CQ 'query shape'. */ -BSONObj predicateShape(const MatchExpression* predicate); +BSONObj debugPredicateShape(const MatchExpression* predicate); +BSONObj representativePredicateShape(const MatchExpression* predicate); -BSONObj predicateShape(const MatchExpression* predicate, - std::function<std::string(StringData)> identifierRedactionPolicy); +BSONObj debugPredicateShape(const MatchExpression* predicate, + std::function<std::string(StringData)> identifierRedactionPolicy); +BSONObj representativePredicateShape( + const MatchExpression* predicate, + std::function<std::string(StringData)> identifierRedactionPolicy); } // namespace mongo::query_shape diff --git a/src/mongo/db/query/query_shape_test.cpp b/src/mongo/db/query/query_shape_test.cpp index 77df7e71027..e15d5f8b485 100644 --- a/src/mongo/db/query/query_shape_test.cpp +++ b/src/mongo/db/query/query_shape_test.cpp @@ -46,18 +46,18 @@ std::string redactFieldNameForTest(StringData sd) { return "REDACT_" + sd.toString(); } -static const SerializationOptions literalAndFieldRedactOpts{redactFieldNameForTest, - query_shape::kLiteralArgString}; +static const SerializationOptions literalAndFieldRedactOpts{ + redactFieldNameForTest, LiteralSerializationPolicy::kToDebugTypeString}; BSONObj predicateShape(std::string filterJson) { ParsedMatchExpressionForTest expr(filterJson); - return query_shape::predicateShape(expr.get()); + return query_shape::debugPredicateShape(expr.get()); } BSONObj predicateShapeRedacted(std::string filterJson) { ParsedMatchExpressionForTest expr(filterJson); - return query_shape::predicateShape(expr.get(), redactFieldNameForTest); + return query_shape::debugPredicateShape(expr.get(), redactFieldNameForTest); } #define ASSERT_SHAPE_EQ_AUTO(expected, actual) \ @@ -70,47 +70,83 @@ BSONObj predicateShapeRedacted(std::string filterJson) { TEST(QueryPredicateShape, Equals) { ASSERT_SHAPE_EQ_AUTO( // Implicit equals - R"({"a":{"$eq":"?"}})", + R"({"a":{"$eq":"?number"}})", "{a: 5}"); ASSERT_SHAPE_EQ_AUTO( // Explicit equals - R"({"a":{"$eq":"?"}})", + R"({"a":{"$eq":"?number"}})", "{a: {$eq: 5}}"); ASSERT_SHAPE_EQ_AUTO( // implicit $and - R"({"$and":[{"a":{"$eq":"?"}},{"b":{"$eq":"?"}}]})", + R"({"$and":[{"a":{"$eq":"?number"}},{"b":{"$eq":"?number"}}]})", "{a: 5, b: 6}"); ASSERT_REDACTED_SHAPE_EQ_AUTO( // Implicit equals - R"({"REDACT_a":{"$eq":"?"}})", + R"({"REDACT_a":{"$eq":"?number"}})", "{a: 5}"); ASSERT_REDACTED_SHAPE_EQ_AUTO( // Explicit equals - R"({"REDACT_a":{"$eq":"?"}})", + R"({"REDACT_a":{"$eq":"?number"}})", "{a: {$eq: 5}}"); ASSERT_REDACTED_SHAPE_EQ_AUTO( // NOLINT - R"({"$and":[{"REDACT_a":{"$eq":"?"}},{"REDACT_b":{"$eq":"?"}}]})", + R"({"$and":[{"REDACT_a":{"$eq":"?number"}},{"REDACT_b":{"$eq":"?number"}}]})", "{a: 5, b: 6}"); } +TEST(QueryPredicateShape, ArraySubTypes) { + ASSERT_SHAPE_EQ_AUTO( // NOLINT + "{a: {$eq: '[]'}}", + "{a: []}"); + ASSERT_SHAPE_EQ_AUTO( // NOLINT + "{a: {$eq: '?array<?number>'}}", + "{a: [2]}"); + ASSERT_SHAPE_EQ_AUTO( // NOLINT + R"({"a":{"$eq":"?array<?number>"}})", + "{a: [2, 3]}"); + ASSERT_SHAPE_EQ_AUTO( // NOLINT + R"({"a":{"$eq":"?array<?object>"}})", + "{a: [{}]}"); + ASSERT_SHAPE_EQ_AUTO( // NOLINT + R"({"a":{"$eq":"?array<?object>"}})", + "{a: [{}, {}]}"); + ASSERT_SHAPE_EQ_AUTO( // NOLINT + R"({"a":{"$eq":"?array<?array>"}})", + "{a: [[], [], []]}"); + ASSERT_SHAPE_EQ_AUTO( // NOLINT + R"({"a":{"$eq":"?array<?array>"}})", + "{a: [[2, 3], ['string'], []]}"); + ASSERT_SHAPE_EQ_AUTO( // NOLINT + R"({"a":{"$eq":"?array<>"}})", + "{a: [{}, 2]}"); + ASSERT_SHAPE_EQ_AUTO( // NOLINT + R"({"a":{"$eq":"?array<>"}})", + "{a: [[], 2]}"); + ASSERT_SHAPE_EQ_AUTO( // NOLINT + R"({"a":{"$eq":"?array<>"}})", + "{a: [[{}, 'string'], 2]}"); + ASSERT_SHAPE_EQ_AUTO( // NOLINT + R"({"a":{"$eq":"?array<>"}})", + "{a: [[{}, 'string'], 2]}"); +} + TEST(QueryPredicateShape, Comparisons) { ASSERT_SHAPE_EQ_AUTO( // NOLINT R"({ "$and": [ { "a": { - "$lt": "?" + "$lt": "?number" } }, { "b": { - "$gt": "?" + "$gt": "?number" } }, { "c": { - "$gte": "?" + "$gte": "?number" } }, { "c": { - "$lte": "?" + "$lte": "?number" } } ] @@ -121,114 +157,184 @@ TEST(QueryPredicateShape, Comparisons) { namespace { void assertShapeIs(std::string filterJson, BSONObj expectedShape) { ParsedMatchExpressionForTest expr(filterJson); - ASSERT_BSONOBJ_EQ(expectedShape, query_shape::predicateShape(expr.get())); + ASSERT_BSONOBJ_EQ(expectedShape, query_shape::debugPredicateShape(expr.get())); } void assertRedactedShapeIs(std::string filterJson, BSONObj expectedShape) { ParsedMatchExpressionForTest expr(filterJson); ASSERT_BSONOBJ_EQ(expectedShape, - query_shape::predicateShape(expr.get(), redactFieldNameForTest)); + query_shape::debugPredicateShape(expr.get(), redactFieldNameForTest)); } } // namespace TEST(QueryPredicateShape, Regex) { // Note/warning: 'fromjson' will parse $regex into a /regex/, so these tests can't use // auto-updating BSON assertions. - assertShapeIs("{a: /a+/}", BSON("a" << BSON("$regex" << query_shape::kLiteralArgString))); + assertShapeIs("{a: /a+/}", + BSON("a" << BSON("$regex" + << "?string"))); assertShapeIs("{a: /a+/i}", - BSON("a" << BSON("$regex" << query_shape::kLiteralArgString << "$options" - << query_shape::kLiteralArgString))); + BSON("a" << BSON("$regex" + << "?string" + << "$options" + << "?string"))); + assertRedactedShapeIs("{a: /a+/}", + BSON("REDACT_a" << BSON("$regex" + << "?string"))); assertRedactedShapeIs("{a: /a+/}", - BSON("REDACT_a" << BSON("$regex" << query_shape::kLiteralArgString))); + BSON("REDACT_a" << BSON("$regex" + << "?string"))); } TEST(QueryPredicateShape, Mod) { ASSERT_SHAPE_EQ_AUTO( // NOLINT - R"({"a":{"$mod":"?"}})", + R"({"a":{"$mod":["?number","?number"]}})", "{a: {$mod: [2, 0]}}"); } TEST(QueryPredicateShape, Exists) { ASSERT_SHAPE_EQ_AUTO( // NOLINT - R"({"a":{"$exists":"?"}})", + R"({"a":{"$exists":"?bool"}})", "{a: {$exists: true}}"); } TEST(QueryPredicateShape, In) { // Any number of children is always the same shape ASSERT_SHAPE_EQ_AUTO( // NOLINT - R"({"a":{"$in":["?"]}})", + R"({"a":{"$in":"?array<?number>"}})", "{a: {$in: [1]}}"); ASSERT_SHAPE_EQ_AUTO( // NOLINT - R"({"a":{"$in":["?"]}})", + R"({"a":{"$in":"?array<>"}})", "{a: {$in: [1, 4, 'str', /regex/]}}"); } TEST(QueryPredicateShape, BitTestOperators) { ASSERT_SHAPE_EQ_AUTO( // NOLINT - R"({"a":{"$bitsAllSet":"?"}})", + R"({"a":{"$bitsAllSet":"?array<?number>"}})", "{a: {$bitsAllSet: [1, 5]}}"); ASSERT_SHAPE_EQ_AUTO( // NOLINT - R"({"a":{"$bitsAllSet":"?"}})", + R"({"a":{"$bitsAllSet":"?array<?number>"}})", "{a: {$bitsAllSet: 50}}"); ASSERT_SHAPE_EQ_AUTO( // NOLINT - R"({"a":{"$bitsAnySet":"?"}})", + R"({"a":{"$bitsAnySet":"?array<?number>"}})", "{a: {$bitsAnySet: [1, 5]}}"); ASSERT_SHAPE_EQ_AUTO( // NOLINT - R"({"a":{"$bitsAnySet":"?"}})", + R"({"a":{"$bitsAnySet":"?array<?number>"}})", "{a: {$bitsAnySet: 50}}"); ASSERT_SHAPE_EQ_AUTO( // NOLINT - R"({"a":{"$bitsAllClear":"?"}})", + R"({"a":{"$bitsAllClear":"?array<?number>"}})", "{a: {$bitsAllClear: [1, 5]}}"); ASSERT_SHAPE_EQ_AUTO( // NOLINT - R"({"a":{"$bitsAllClear":"?"}})", + R"({"a":{"$bitsAllClear":"?array<?number>"}})", "{a: {$bitsAllClear: 50}}"); ASSERT_SHAPE_EQ_AUTO( // NOLINT - R"({"a":{"$bitsAnyClear":"?"}})", + R"({"a":{"$bitsAnyClear":"?array<?number>"}})", "{a: {$bitsAnyClear: [1, 5]}}"); ASSERT_SHAPE_EQ_AUTO( // NOLINT - R"({"a":{"$bitsAnyClear":"?"}})", + R"({"a":{"$bitsAnyClear":"?array<?number>"}})", "{a: {$bitsAnyClear: 50}}"); } TEST(QueryPredicateShape, AlwaysBoolean) { ASSERT_SHAPE_EQ_AUTO( // NOLINT - R"({"$alwaysTrue":"?"})", + R"({"$alwaysTrue":"?number"})", "{$alwaysTrue: 1}"); ASSERT_SHAPE_EQ_AUTO( // NOLINT - R"({"$alwaysFalse":"?"})", + R"({"$alwaysFalse":"?number"})", "{$alwaysFalse: 1}"); } TEST(QueryPredicateShape, And) { ASSERT_SHAPE_EQ_AUTO( // NOLINT - R"({"$and":[{"a":{"$lt":"?"}},{"b":{"$gte":"?"}},{"c":{"$lte":"?"}}]})", + R"({ + "$and": [ + { + "a": { + "$lt": "?number" + } + }, + { + "b": { + "$gte": "?number" + } + }, + { + "c": { + "$lte": "?number" + } + } + ] + })", "{$and: [{a: {$lt: 5}}, {b: {$gte: 3}}, {c: {$lte: 10}}]}"); } TEST(QueryPredicateShape, Or) { ASSERT_SHAPE_EQ_AUTO( // NOLINT - R"({"$or":[{"a":{"$eq":"?"}},{"b":{"$in":["?"]}},{"c":{"$gt":"?"}}]})", + R"({ + "$or": [ + { + "a": { + "$eq": "?number" + } + }, + { + "b": { + "$in": "?array<?number>" + } + }, + { + "c": { + "$gt": "?number" + } + } + ] + })", "{$or: [{a: 5}, {b: {$in: [1,2,3]}}, {c: {$gt: 10}}]}"); } TEST(QueryPredicateShape, ElemMatch) { // ElemMatchObjectMatchExpression ASSERT_SHAPE_EQ_AUTO( // NOLINT - R"({"a":{"$elemMatch":{"$and":[{"b":{"$eq":"?"}},{"c":{"$exists":"?"}}]}}})", + R"({ + "a": { + "$elemMatch": { + "$and": [ + { + "b": { + "$eq": "?number" + } + }, + { + "c": { + "$exists": "?bool" + } + } + ] + } + } + })", "{a: {$elemMatch: {b: 5, c: {$exists: true}}}}"); // ElemMatchValueMatchExpression ASSERT_SHAPE_EQ_AUTO( // NOLINT - R"({"a":{"$elemMatch":{"$gt":"?","$lt":"?"}}})", + R"({"a":{"$elemMatch":{"$gt":"?number","$lt":"?number"}}})", "{a: {$elemMatch: {$gt: 5, $lt: 10}}}"); // Nested ASSERT_REDACTED_SHAPE_EQ_AUTO( // NOLINT - R"({"REDACT_a":{"$elemMatch":{"$elemMatch":{"$gt":"?","$lt":"?"}}}})", + R"({ + "REDACT_a": { + "$elemMatch": { + "$elemMatch": { + "$gt": "?number", + "$lt": "?number" + } + } + } + })", "{a: {$elemMatch: {$elemMatch: {$gt: 5, $lt: 10}}}}"); } @@ -240,7 +346,7 @@ TEST(QueryPredicateShape, InternalBucketGeoWithinMatchExpression) { R"({ "$_internalBucketGeoWithin": { "withinRegion": { - "$centerSphere": "?" + "$centerSphere": "?array<>" }, "field": "REDACT_a" } @@ -250,26 +356,26 @@ TEST(QueryPredicateShape, InternalBucketGeoWithinMatchExpression) { TEST(QueryPredicateShape, NorMatchExpression) { ASSERT_REDACTED_SHAPE_EQ_AUTO( // NOLINT - R"({"$nor":[{"REDACT_a":{"$lt":"?"}},{"REDACT_b":{"$gt":"?"}}]})", + R"({"$nor":[{"REDACT_a":{"$lt":"?number"}},{"REDACT_b":{"$gt":"?number"}}]})", "{ $nor: [ { a: {$lt: 5} }, { b: {$gt: 4} } ] }"); } TEST(QueryPredicateShape, NotMatchExpression) { ASSERT_REDACTED_SHAPE_EQ_AUTO( // NOLINT - R"({"REDACT_price":{"$not":{"$gt":"?"}}})", + R"({"REDACT_price":{"$not":{"$gt":"?number"}}})", "{ price: { $not: { $gt: 1.99 } } }"); // Test the special case where NotMatchExpression::serialize() reduces to $alwaysFalse. auto emptyAnd = std::make_unique<AndMatchExpression>(); const MatchExpression& notExpr = NotMatchExpression(std::move(emptyAnd)); auto serialized = notExpr.serialize(literalAndFieldRedactOpts); ASSERT_BSONOBJ_EQ_AUTO( // NOLINT - R"({"$alwaysFalse":"?"})", + R"({"$alwaysFalse":"?number"})", serialized); } TEST(QueryPredicateShape, SizeMatchExpression) { ASSERT_REDACTED_SHAPE_EQ_AUTO( // NOLINT - R"({"REDACT_price":{"$size":"?"}})", + R"({"REDACT_price":{"$size":"?number"}})", "{ price: { $size: 2 } }"); } @@ -279,10 +385,10 @@ TEST(QueryPredicateShape, TextMatchExpression) { ASSERT_BSONOBJ_EQ_AUTO( // NOLINT R"({ "$text": { - "$search": "?", - "$language": "?", - "$caseSensitive": "?", - "$diacriticSensitive": "?" + "$search": "?string", + "$language": "?string", + "$caseSensitive": "?bool", + "$diacriticSensitive": "?bool" } })", expr->serialize(literalAndFieldRedactOpts)); @@ -297,7 +403,7 @@ TEST(QueryPredicateShape, TwoDPtInAnnulusExpression) { TEST(QueryPredicateShape, WhereMatchExpression) { ASSERT_SHAPE_EQ_AUTO( // NOLINT - R"({"$where":"?"})", + R"({"$where":"?javascript"})", "{$where: \"some_code()\"}"); } @@ -308,7 +414,7 @@ BSONObj queryShapeForOptimizedExprExpression(std::string exprPredicateJson) { // the computation on any MatchExpression, and this is the easiest way we can create this type // of MatchExpression node. auto optimized = MatchExpression::optimize(expr.release()); - return query_shape::predicateShape(optimized.get()); + return query_shape::debugPredicateShape(optimized.get()); } TEST(QueryPredicateShape, OptimizedExprPredicates) { @@ -317,16 +423,14 @@ TEST(QueryPredicateShape, OptimizedExprPredicates) { "$and": [ { "a": { - "$_internalExprEq": "?" + "$_internalExprEq": "?number" } }, { "$expr": { "$eq": [ "$a", - { - "$const": "?" - } + "?number" ] } } @@ -339,16 +443,14 @@ TEST(QueryPredicateShape, OptimizedExprPredicates) { "$and": [ { "a": { - "$_internalExprLt": "?" + "$_internalExprLt": "?number" } }, { "$expr": { "$lt": [ "$a", - { - "$const": "?" - } + "?number" ] } } @@ -361,16 +463,14 @@ TEST(QueryPredicateShape, OptimizedExprPredicates) { "$and": [ { "a": { - "$_internalExprLte": "?" + "$_internalExprLte": "?number" } }, { "$expr": { "$lte": [ "$a", - { - "$const": "?" - } + "?number" ] } } @@ -383,16 +483,14 @@ TEST(QueryPredicateShape, OptimizedExprPredicates) { "$and": [ { "a": { - "$_internalExprGt": "?" + "$_internalExprGt": "?number" } }, { "$expr": { "$gt": [ "$a", - { - "$const": "?" - } + "?number" ] } } @@ -405,16 +503,14 @@ TEST(QueryPredicateShape, OptimizedExprPredicates) { "$and": [ { "a": { - "$_internalExprGte": "?" + "$_internalExprGte": "?number" } }, { "$expr": { "$gte": [ "$a", - { - "$const": "?" - } + "?number" ] } } diff --git a/src/mongo/db/query/serialization_options.cpp b/src/mongo/db/query/serialization_options.cpp new file mode 100644 index 00000000000..71ecf426e5a --- /dev/null +++ b/src/mongo/db/query/serialization_options.cpp @@ -0,0 +1,384 @@ +/** + * Copyright (C) 2023-present MongoDB, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * <http://www.mongodb.com/licensing/server-side-public-license>. + * + * As a special exception, the copyright holders give permission to link the + * code of portions of this program with the OpenSSL library under certain + * conditions as described in each individual source file and distribute + * linked combinations including the program with the OpenSSL library. You + * must comply with the Server Side Public License in all respects for + * all of the code used other than as permitted herein. If you modify file(s) + * with this exception, you may extend this exception to your version of the + * file(s), but you are not obligated to do so. If you do not wish to do so, + * delete this exception statement from your version. If you delete this + * exception statement from all source files in the program, then also delete + * it in the license file. + */ + +#define MONGO_LOGV2_DEFAULT_COMPONENT ::mongo::logv2::LogComponent::kQuery + +#include "mongo/db/query/serialization_options.h" + +#include <boost/optional.hpp> +#include <string> + +#include "mongo/base/string_data.h" +#include "mongo/bson/timestamp.h" +#include "mongo/db/exec/document_value/document.h" +#include "mongo/db/exec/document_value/value.h" +#include "mongo/db/query/explain_options.h" +#include "mongo/logv2/log.h" +#include "mongo/util/assert_util.h" + +namespace mongo { + +namespace { + +// We'll pre-declare all of these strings so that we can avoid the allocations when we reference +// them later. +static constexpr StringData kUndefinedTypeString = "?undefined"_sd; +static constexpr StringData kStringTypeString = "?string"_sd; +static constexpr StringData kNumberTypeString = "?number"_sd; +static constexpr StringData kMinKeyTypeString = "?minKey"_sd; +static constexpr StringData kObjectTypeString = "?object"_sd; +static constexpr StringData kArrayTypeString = "?array"_sd; +static constexpr StringData kBinDataTypeString = "?binData"_sd; +static constexpr StringData kObjectIdTypeString = "?objectId"_sd; +static constexpr StringData kBoolTypeString = "?bool"_sd; +static constexpr StringData kDateTypeString = "?date"_sd; +static constexpr StringData kNullTypeString = "?null"_sd; +static constexpr StringData kRegexTypeString = "?regex"_sd; +static constexpr StringData kDbPointerTypeString = "?dbPointer"_sd; +static constexpr StringData kJavascriptTypeString = "?javascript"_sd; +static constexpr StringData kJavascriptWithScopeTypeString = "?javascriptWithScope"_sd; +static constexpr StringData kTimestampTypeString = "?timestamp"_sd; +static constexpr StringData kMaxKeyTypeString = "?maxKey"_sd; + +static const StringMap<StringData> kArrayTypeStringConstants{ + {kUndefinedTypeString.rawData(), "?array<?undefined>"_sd}, + {kStringTypeString.rawData(), "?array<?string>"_sd}, + {kNumberTypeString.rawData(), "?array<?number>"_sd}, + {kMinKeyTypeString.rawData(), "?array<?minKey>"_sd}, + {kObjectTypeString.rawData(), "?array<?object>"_sd}, + {kArrayTypeString.rawData(), "?array<?array>"_sd}, + {kBinDataTypeString.rawData(), "?array<?binData>"_sd}, + {kObjectIdTypeString.rawData(), "?array<?objectId>"_sd}, + {kBoolTypeString.rawData(), "?array<?bool>"_sd}, + {kDateTypeString.rawData(), "?array<?date>"_sd}, + {kNullTypeString.rawData(), "?array<?null>"_sd}, + {kRegexTypeString.rawData(), "?array<?regex>"_sd}, + {kDbPointerTypeString.rawData(), "?array<?dbPointer>"_sd}, + {kJavascriptTypeString.rawData(), "?array<?javascript>"_sd}, + {kJavascriptWithScopeTypeString.rawData(), "?array<?javascriptWithScope>"_sd}, + {kTimestampTypeString.rawData(), "?array<?timestamp>"_sd}, + {kMaxKeyTypeString.rawData(), "?array<?maxKey>"_sd}, +}; + +/** + * Computes a debug string meant to represent "any value of type t", where "t" is the type of the + * provided argument. For example "?number" for any number (int, double, etc.). + */ +StringData debugTypeString(BSONType t) { + // This is tightly coupled with 'canonicalizeBSONType' and therefore also with + // sorting/comparison semantics. + switch (t) { + case EOO: + case Undefined: + return kUndefinedTypeString; + case Symbol: + case String: + return kStringTypeString; + case NumberInt: + case NumberLong: + case NumberDouble: + case NumberDecimal: + return kNumberTypeString; + case MinKey: + return kMinKeyTypeString; + case Object: + return kObjectTypeString; + case Array: + // This case should only happen if we have an array within an array. + return kArrayTypeString; + case BinData: + return kBinDataTypeString; + case jstOID: + return kObjectIdTypeString; + case Bool: + return kBoolTypeString; + case Date: + return kDateTypeString; + case jstNULL: + return kNullTypeString; + case RegEx: + return kRegexTypeString; + case DBRef: + return kDbPointerTypeString; + case Code: + return kJavascriptTypeString; + case CodeWScope: + return kJavascriptWithScopeTypeString; + case bsonTimestamp: + return kTimestampTypeString; + case MaxKey: + return kMaxKeyTypeString; + default: + MONGO_UNREACHABLE_TASSERT(7539806); + } +} + +/** + * Returns an arbitrary value of the same type as the one given. For any number, this will be the + * number 1. For any boolean this will be true. + * TODO if you need a different value to make sure it will parse, you should not use this API. + */ +ImplicitValue defaultLiteralOfType(BSONType t) { + // This is tightly coupled with 'canonicalizeBSONType' and therefore also with + // sorting/comparison semantics. + switch (t) { + case EOO: + case Undefined: + return BSONUndefined; + case Symbol: + case String: + return "?"_sd; + case NumberInt: + case NumberLong: + case NumberDouble: + case NumberDecimal: + return 1; + case MinKey: + return MINKEY; + case Object: + return Document{{"?"_sd, "?"_sd}}; + case Array: + // This case should only happen if we have an array within an array. + return BSONArray(); + case BinData: + return BSONBinData(); + case jstOID: + return OID::max(); + case Bool: + return true; + case Date: + return Date_t::fromMillisSinceEpoch(0); + case jstNULL: + return BSONNULL; + case RegEx: + return BSONRegEx("/\?/"); + case DBRef: + return BSONDBRef("?.?", OID::max()); + case Code: + return BSONCode("return ?;"); + case CodeWScope: + return BSONCodeWScope("return ?;", BSONObj()); + case bsonTimestamp: + return Timestamp::min(); + case MaxKey: + return MAXKEY; + default: + MONGO_UNREACHABLE_TASSERT(7539803); + } +} + +/** + * A struct representing the sub-type information for an array. + */ +struct ArraySubtypeInfo { + /** + * Whether the values of an array are all the same BSON type or not (mixed). + */ + enum class NTypes { kEmpty, kOneType, kMixed }; + ArraySubtypeInfo(NTypes nTypes_) : nTypes(nTypes_) {} + ArraySubtypeInfo(BSONType oneType) : nTypes(NTypes::kOneType), singleType(oneType) {} + + NTypes nTypes; + boost::optional<BSONType> singleType = boost::none; +}; + +template <typename ValueType> +using GetTypeFn = std::function<BSONType(ValueType)>; + +static GetTypeFn<BSONElement> getBSONElementType = [](const BSONElement& e) { + return e.type(); +}; +static GetTypeFn<Value> getValueType = [](const Value& v) { + return v.getType(); +}; + +/** + * Scans 'arrayOfValues' to see if all values are of the same type or not. Returns this info in a + * struct - see the struct definition for how it is represented. + * + * Templated algorithm to handle both iterators of BSONElements or iterators of Values. + * 'getTypeCallback' is provided to abstract away the different '.type()' vs '.getType()' APIs. + */ +template <typename ArrayType, typename ValueType> +ArraySubtypeInfo determineArraySubType(const ArrayType& arrayOfValues, + GetTypeFn<ValueType> getTypeCallback) { + boost::optional<BSONType> firstType = boost::none; + for (auto&& v : arrayOfValues) { + if (!firstType) { + firstType.emplace(getTypeCallback(v)); + } else if (*firstType != getTypeCallback(v)) { + return {ArraySubtypeInfo::NTypes::kMixed}; + } + } + return firstType ? ArraySubtypeInfo{*firstType} + : ArraySubtypeInfo{ArraySubtypeInfo::NTypes::kEmpty}; +} + +ArraySubtypeInfo determineArraySubType(const BSONObj& arrayAsObj) { + return determineArraySubType<BSONObj, BSONElement>(arrayAsObj, getBSONElementType); +} +ArraySubtypeInfo determineArraySubType(const std::vector<Value>& values) { + return determineArraySubType<std::vector<Value>, Value>(values, getValueType); +} + +template <typename ValueType> +StringData debugTypeString( + const ValueType& v, + GetTypeFn<ValueType> getTypeCallback, + std::function<ArraySubtypeInfo(ValueType)> determineArraySubTypeCallback) { + if (getTypeCallback(v) == BSONType::Array) { + // Iterating the array as .Obj(), as if it were a BSONObj (with field names '0', '1', etc.) + // is faster than converting the whole thing to an array which would force a copy. + auto typeInfo = determineArraySubTypeCallback(v); + switch (typeInfo.nTypes) { + case ArraySubtypeInfo::NTypes::kEmpty: + return "[]"_sd; + case ArraySubtypeInfo::NTypes::kOneType: + return kArrayTypeStringConstants.at(debugTypeString(*typeInfo.singleType)); + case ArraySubtypeInfo::NTypes::kMixed: + return "?array<>"; + default: + MONGO_UNREACHABLE_TASSERT(7539801); + } + } + return debugTypeString(getTypeCallback(v)); +} + +template <typename ValueType> +ImplicitValue defaultLiteralOfType( + const ValueType& v, + GetTypeFn<ValueType> getTypeCallback, + std::function<ArraySubtypeInfo(ValueType)> determineArraySubTypeCallback) { + if (getTypeCallback(v) == BSONType::Array) { + auto typeInfo = determineArraySubTypeCallback(v); + switch (typeInfo.nTypes) { + case ArraySubtypeInfo::NTypes::kEmpty: + return BSONArray(); + case ArraySubtypeInfo::NTypes::kOneType: + return std::vector<Value>{defaultLiteralOfType(*typeInfo.singleType)}; + case ArraySubtypeInfo::NTypes::kMixed: + // We don't care which types, we'll use a number and a string as the canonical + // mixed type array regardless. This is to ensure we don't get 2^N possibilities + // for mixed type scenarios - we wish to collapse all "mixed type" arrays to one + // canonical mix. The choice of int and string is mostly arbitrary - hopefully + // somewhat comprehensible at a glance. + return std::vector<Value>{Value(2), Value("or more types"_sd)}; + default: + MONGO_UNREACHABLE_TASSERT(7539805); + } + } + return defaultLiteralOfType(getTypeCallback(v)); +} + +ArraySubtypeInfo getSubTypeFromBSONElemArray(BSONElement arrayElem) { + // Iterating the array as .Obj(), as if it were a BSONObj (with field names '0', '1', etc.) + // is faster than converting the whole thing to an array which would force a copy. + return determineArraySubType(arrayElem.Obj()); +} +ArraySubtypeInfo getSubTypeFromValueArray(const Value& arrayVal) { + return determineArraySubType(arrayVal.getArray()); +} + +} // namespace + +// Overloads for BSONElem and Value. +StringData debugTypeString(BSONElement e) { + return debugTypeString<BSONElement>(e, getBSONElementType, getSubTypeFromBSONElemArray); +} +StringData debugTypeString(const Value& v) { + return debugTypeString<Value>(v, getValueType, getSubTypeFromValueArray); +} + +// Overloads for BSONElem and Value. +ImplicitValue defaultLiteralOfType(const Value& v) { + return defaultLiteralOfType<Value>(v, getValueType, getSubTypeFromValueArray); +} +ImplicitValue defaultLiteralOfType(BSONElement e) { + return defaultLiteralOfType<BSONElement>(e, getBSONElementType, getSubTypeFromBSONElemArray); +} + +void SerializationOptions::appendLiteral(BSONObjBuilder* bob, const BSONElement& e) const { + serializeLiteral(e).addToBsonObj(bob, e.fieldNameStringData()); +} + +void SerializationOptions::appendLiteral(BSONObjBuilder* bob, + StringData fieldName, + const ImplicitValue& v) const { + serializeLiteral(v).addToBsonObj(bob, fieldName); +} + +Value SerializationOptions::serializeLiteral(const BSONElement& e) const { + switch (literalPolicy) { + case LiteralSerializationPolicy::kUnchanged: + return Value(e); + case LiteralSerializationPolicy::kToDebugTypeString: + return Value(debugTypeString(e)); + case LiteralSerializationPolicy::kToRepresentativeParseableValue: + return defaultLiteralOfType(e); + default: + MONGO_UNREACHABLE_TASSERT(7539802); + } +} + +Value SerializationOptions::serializeLiteral(const ImplicitValue& v) const { + switch (literalPolicy) { + case LiteralSerializationPolicy::kUnchanged: + return v; + case LiteralSerializationPolicy::kToDebugTypeString: + return Value(debugTypeString(v)); + case LiteralSerializationPolicy::kToRepresentativeParseableValue: + return defaultLiteralOfType(v); + default: + MONGO_UNREACHABLE_TASSERT(7539804); + } +} + +std::string SerializationOptions::serializeFieldPathFromString(StringData path) const { + if (redactIdentifiers) { + // Some valid field names are considered invalid as a FieldPath (for example, fields + // like "foo.$bar" where a sub-component is prefixed with "$"). For now, if + // serializeFieldPath errors due to an "invalid" field name, we'll serialize that field + // name with this placeholder. + // TODO SERVER-75623 Implement full redaction for all field names and remove placeholder + try { + return serializeFieldPath(path); + } catch (DBException& ex) { + LOGV2_DEBUG( + 7549808, + 1, + "Failed to convert a path string to a FieldPath, so replacing with placeholder", + "pathString"_attr = path, + "failure"_attr = ex.toStatus()); + return serializeFieldPath("dollarPlaceholder"); + } + } + return path.toString(); +} + +} // namespace mongo diff --git a/src/mongo/db/query/serialization_options.h b/src/mongo/db/query/serialization_options.h index 4cd00dd3faf..82db054d7ec 100644 --- a/src/mongo/db/query/serialization_options.h +++ b/src/mongo/db/query/serialization_options.h @@ -31,6 +31,7 @@ #include "mongo/base/string_data.h" #include "mongo/bson/bsonobj.h" #include "mongo/bson/bsonobjbuilder.h" +#include "mongo/db/exec/document_value/document.h" #include "mongo/db/exec/document_value/value.h" #include "mongo/db/pipeline/field_path.h" #include "mongo/db/query/explain_options.h" @@ -47,6 +48,25 @@ std::string defaultRedactionStrategy(StringData s) { } // namespace /** + * A policy enum for how to serialize literal values. + */ +enum class LiteralSerializationPolicy { + // The default way to serialize. Just serialize whatever literals were given if they are still + // available, or whatever you parsed them to. This is expected to be able to parse again, since + // it worked the first time. + kUnchanged, + // Serialize any literal value as "?number" or similar. For example "?bool" for any boolean. Use + // 'debugTypeString()' helper. + kToDebugTypeString, + // Serialize any literal value to one canonical value of the given type, with the constraint + // that the chosen representative value should be parseable in this context. There are some + // default implementations that will usually work (e.g. using the number 1 almost always works + // for numbers), but serializers should be careful to think about and test this if their parsers + // reject certain values. + kToRepresentativeParseableValue, +}; + +/** * A struct with options for how you want to serialize a match or aggregation expression. */ struct SerializationOptions { @@ -69,6 +89,12 @@ struct SerializationOptions { redactIdentifiers(identifierRedactionPolicy_), identifierRedactionPolicy(identifierRedactionPolicy_) {} + SerializationOptions(std::function<std::string(StringData)> redactFieldNamesStrategy_, + LiteralSerializationPolicy policy) + : literalPolicy(policy), + redactIdentifiers(redactFieldNamesStrategy_), + identifierRedactionPolicy(redactFieldNamesStrategy_) {} + // Helper function for redacting identifiable information (like collection/db names). // Note: serializeFieldPath/serializeFieldPathFromString should be used for redacting field // names. @@ -97,21 +123,7 @@ struct SerializationOptions { return "$" + serializeFieldPath(path); } - std::string serializeFieldPathFromString(StringData path) const { - if (redactIdentifiers) { - // Some valid field names are considered invalid as a FieldPath (for example, fields - // like "foo.$bar" where a sub-component is prefixed with "$"). For now, if - // serializeFieldPath errors due to an "invalid" field name, we'll serialize that field - // name with this placeholder. - // TODO SERVER-75623 Implement full redaction for all field names and remove placeholder - try { - return serializeFieldPath(path); - } catch (DBException&) { - return serializeFieldPath("dollarPlaceholder"); - } - } - return path.toString(); - } + std::string serializeFieldPathFromString(StringData path) const; template <class T> Value serializeLiteralValue(T n) const { @@ -134,11 +146,7 @@ struct SerializationOptions { redactArrayToBuilder(&subArr, elem.Array()); subArr.done(); } else { - if (replacementForLiteralArgs) { - bab->append(replacementForLiteralArgs.get()); - } else { - bab->append(elem); - } + *bab << serializeLiteral(elem); } } } @@ -155,15 +163,46 @@ struct SerializationOptions { redactArrayToBuilder(&subArr, elem.Array()); subArr.done(); } else { - if (replacementForLiteralArgs) { - bob->append(fieldName, replacementForLiteralArgs.get()); - } else { - bob->appendAs(elem, fieldName); - } + appendLiteral(bob, fieldName, elem); } } } + template <class T> + Value serializeLiteralValue(T n) { + if (replacementForLiteralArgs) { + return Value(*replacementForLiteralArgs); + } + return Value(n); + } + + /** + * Helper method to call 'serializeLiteral()' on 'e' and append the resulting value to 'bob' + * using the same name as 'e'. + */ + void appendLiteral(BSONObjBuilder* bob, const BSONElement& e) const; + /** + * Helper method to call 'serializeLiteral()' on 'v' and append the result to 'bob' using field + * name 'fieldName'. + */ + void appendLiteral(BSONObjBuilder* bob, StringData fieldName, const ImplicitValue& v) const; + + /** + * This is the recommended API for adding any literals to serialization output. For example, + * BSON("myArg" << options.serializeLiteral(_myArg)); + * + * Depending on the configured 'literalPolicy', it will do the right thing. + * - If 'literalPolicy' is 'kUnchanged', returns the input value unmodified. + * - If it is 'kToDebugTypeString', computes and returns the type string as a string Value. + * - If it is 'kToRepresentativeValue', it Returns an arbitrary value of the same type as the + * one given. For any number, this will be the number 1. For any boolean this will be true. + * + * TODO SERVER-XYZ If you need a different value to make sure it will parse, you should not + * use this API - but use serializeConstrainedLiteral() instead. + */ + Value serializeLiteral(const BSONElement& e) const; + Value serializeLiteral(const ImplicitValue& v) const; + // 'replacementForLiteralArgs' is an independent option to serialize in a genericized format // with the aim of similar "shaped" queries serializing to the same object. For example, if // set to '?' then the serialization of {a: {$gt: 2}} will result in {a: {$gt: '?'}}, as @@ -172,8 +211,20 @@ struct SerializationOptions { // "Literal" here is meant to stand in contrast to expression arguements, as in the $gt // expressions in {$and: [{a: {$gt: 3}}, {b: {$gt: 4}}]}. There the only literals are 3 and // 4, so the serialization expected would be {$and: [{a: {$gt: '?'}}, {b: {$lt: '?'}}]}. + // + // TODO SERVER-XXX remove this option in favor of 'literalPolicy' below. boost::optional<StringData> replacementForLiteralArgs = boost::none; + // 'literalPolicy' is an independent option to serialize in a general format with the aim of + // similar "shaped" queries serializing to the same object. For example, if set to + // 'kToDebugTypeString', then the serialization of {a: {$gt: 2}} should result in {a: {$gt: + // '?number'}}, as will the serialization of {a: {$gt: 3}}. + // + // "Literal" here is meant to stand in contrast to expression arguments, as in the $gt + // expressions in {$and: [{a: {$gt: 3}}, {b: {$gt: 4}}]}. There the only literals are 3 and 4, + // so the serialization expected would be {$and: [{a: {$gt: '?'}}, {b: {$lt: '?'}}]}. + LiteralSerializationPolicy literalPolicy = LiteralSerializationPolicy::kUnchanged; + // If true the caller must set identifierRedactionPolicy. 'redactIdentifiers' if set along with // a strategy the redaction strategy will be called on any personal identifiable information // (e.g., field paths/names, collection names) encountered before serializing them. diff --git a/src/mongo/db/query/telemetry.cpp b/src/mongo/db/query/telemetry.cpp index b1c73c70d50..cd7dcbc667e 100644 --- a/src/mongo/db/query/telemetry.cpp +++ b/src/mongo/db/query/telemetry.cpp @@ -67,42 +67,27 @@ namespace telemetry { */ namespace { static std::string hintSpecialField = "$hint"; -void addLiteralFieldsWithRedaction(BSONObjBuilder* bob, - const FindCommandRequest& findCommand, - StringData newLiteral) { - if (findCommand.getLimit()) { - bob->append(FindCommandRequest::kLimitFieldName, newLiteral); - } - if (findCommand.getSkip()) { - bob->append(FindCommandRequest::kSkipFieldName, newLiteral); - } - if (findCommand.getBatchSize()) { - bob->append(FindCommandRequest::kBatchSizeFieldName, newLiteral); - } - if (findCommand.getMaxTimeMS()) { - bob->append(FindCommandRequest::kMaxTimeMSFieldName, newLiteral); - } - if (findCommand.getNoCursorTimeout()) { - bob->append(FindCommandRequest::kNoCursorTimeoutFieldName, newLiteral); - } -} +void addLiteralFields(BSONObjBuilder* bob, + const FindCommandRequest& findCommand, + const SerializationOptions& opts) { -void addLiteralFieldsWithoutRedaction(BSONObjBuilder* bob, const FindCommandRequest& findCommand) { - if (auto param = findCommand.getLimit()) { - bob->append(FindCommandRequest::kLimitFieldName, param.get()); + if (auto limit = findCommand.getLimit()) { + opts.appendLiteral( + bob, FindCommandRequest::kLimitFieldName, static_cast<long long>(*limit)); } - if (auto param = findCommand.getSkip()) { - bob->append(FindCommandRequest::kSkipFieldName, param.get()); + if (auto skip = findCommand.getSkip()) { + opts.appendLiteral(bob, FindCommandRequest::kSkipFieldName, static_cast<long long>(*skip)); } - if (auto param = findCommand.getBatchSize()) { - bob->append(FindCommandRequest::kBatchSizeFieldName, param.get()); + if (auto batchSize = findCommand.getBatchSize()) { + opts.appendLiteral( + bob, FindCommandRequest::kBatchSizeFieldName, static_cast<long long>(*batchSize)); } - if (auto param = findCommand.getMaxTimeMS()) { - bob->append(FindCommandRequest::kMaxTimeMSFieldName, param.get()); + if (auto maxTimeMs = findCommand.getMaxTimeMS()) { + opts.appendLiteral(bob, FindCommandRequest::kMaxTimeMSFieldName, *maxTimeMs); } - if (findCommand.getNoCursorTimeout().has_value()) { - bob->append(FindCommandRequest::kNoCursorTimeoutFieldName, - findCommand.getNoCursorTimeout().value_or(false)); + if (auto noCursorTimeout = findCommand.getNoCursorTimeout()) { + opts.appendLiteral( + bob, FindCommandRequest::kNoCursorTimeoutFieldName, bool(noCursorTimeout)); } } @@ -126,19 +111,22 @@ std::vector<std::pair<StringData, std::function<const BSONObj(const FindCommandR }; -void addRemainingFindCommandFields(BSONObjBuilder* bob, const FindCommandRequest& findCommand) { +void addRemainingFindCommandFields(BSONObjBuilder* bob, + const FindCommandRequest& findCommand, + const SerializationOptions& opts) { for (auto [fieldName, getterFunction] : boolArgMap) { auto optBool = getterFunction(findCommand); if (optBool.has_value()) { - bob->append(fieldName, optBool.value_or(false)); + opts.appendLiteral(bob, fieldName, optBool.value_or(false)); } } if (auto optObj = findCommand.getReadConcern()) { + // Read concern should not be considered a literal. bob->append(FindCommandRequest::kReadConcernFieldName, optObj.get()); } auto collation = findCommand.getCollation(); if (!collation.isEmpty()) { - bob->append(FindCommandRequest::kCollationFieldName, collation); + opts.appendLiteral(bob, FindCommandRequest::kCollationFieldName, collation); } } @@ -305,15 +293,10 @@ StatusWith<BSONObj> makeTelemetryKey(const FindCommandRequest& findCommand, } // Fields for literal redaction. Adds limit, skip, batchSize, maxTimeMS, and noCursorTimeOut - if (opts.replacementForLiteralArgs) { - addLiteralFieldsWithRedaction(&bob, findCommand, opts.replacementForLiteralArgs.get()); - } else { - addLiteralFieldsWithoutRedaction(&bob, findCommand); - } + addLiteralFields(&bob, findCommand, opts); // Add the fields that require no redaction. - addRemainingFindCommandFields(&bob, findCommand); - + addRemainingFindCommandFields(&bob, findCommand, opts); auto appName = [&]() -> boost::optional<std::string> { if (existingMetrics.has_value()) { @@ -618,7 +601,7 @@ StatusWith<BSONObj> TelemetryMetrics::redactKey(const BSONObj& key, SerializationOptions options( [&](StringData sd) { return sha256HmacStringDataHasher(redactionKey, sd); }, - replacementForLiteralArgs); + LiteralSerializationPolicy::kToDebugTypeString); auto nss = findCommand->getNamespaceOrUUID().nss(); uassert(7349400, "Namespace must be defined", nss.has_value()); auto expCtx = make_intrusive<ExpressionContext>(opCtx, nullptr, nss.value()); @@ -751,6 +734,7 @@ void registerFindRequest(const FindCommandRequest& request, } SerializationOptions options; + options.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; options.replacementForLiteralArgs = replacementForLiteralArgs; auto swTelemetryKey = makeTelemetryKey(request, options, expCtx); tassert(7349402, diff --git a/src/mongo/db/query/telemetry_store_test.cpp b/src/mongo/db/query/telemetry_store_test.cpp index d2e8e505008..372d67da911 100644 --- a/src/mongo/db/query/telemetry_store_test.cpp +++ b/src/mongo/db/query/telemetry_store_test.cpp @@ -129,7 +129,9 @@ TEST_F(TelemetryStoreTest, CorrectlyRedactsFindCommandRequestAllFields) { FindCommandRequest fcr(NamespaceStringOrUUID(NamespaceString("testDB.testColl"))); fcr.setFilter(BSON("a" << 1)); SerializationOptions opts; + // TODO SERVER-75419 Use only 'literalPolicy.' opts.replacementForLiteralArgs = "?"; + opts.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; opts.redactIdentifiers = true; opts.identifierRedactionPolicy = redactFieldNameForTest; @@ -144,7 +146,7 @@ TEST_F(TelemetryStoreTest, CorrectlyRedactsFindCommandRequestAllFields) { "find": "HASH<testColl>", "filter": { "HASH<a>": { - "$eq": "?" + "$eq": "?number" } } })", @@ -162,7 +164,7 @@ TEST_F(TelemetryStoreTest, CorrectlyRedactsFindCommandRequestAllFields) { "find": "HASH<testColl>", "filter": { "HASH<a>": { - "$eq": "?" + "$eq": "?number" } }, "sort": { @@ -184,7 +186,7 @@ TEST_F(TelemetryStoreTest, CorrectlyRedactsFindCommandRequestAllFields) { "find": "HASH<testColl>", "filter": { "HASH<a>": { - "$eq": "?" + "$eq": "?number" } }, "projection": { @@ -214,14 +216,12 @@ TEST_F(TelemetryStoreTest, CorrectlyRedactsFindCommandRequestAllFields) { "find": "HASH<testColl>", "filter": { "HASH<a>": { - "$eq": "?" + "$eq": "?number" } }, "let": { "HASH<var1>": "$HASH<a>", - "HASH<var2>": { - "$const": "?" - } + "HASH<var2>": "?string" }, "projection": { "HASH<e>": true, @@ -249,14 +249,12 @@ TEST_F(TelemetryStoreTest, CorrectlyRedactsFindCommandRequestAllFields) { "find": "HASH<testColl>", "filter": { "HASH<a>": { - "$eq": "?" + "$eq": "?number" } }, "let": { "HASH<var1>": "$HASH<a>", - "HASH<var2>": { - "$const": "?" - } + "HASH<var2>": "?string" }, "projection": { "HASH<e>": true, @@ -297,14 +295,12 @@ TEST_F(TelemetryStoreTest, CorrectlyRedactsFindCommandRequestAllFields) { "find": "HASH<testColl>", "filter": { "HASH<a>": { - "$eq": "?" + "$eq": "?number" } }, "let": { "HASH<var1>": "$HASH<a>", - "HASH<var2>": { - "$const": "?" - } + "HASH<var2>": "?string" }, "projection": { "HASH<e>": true, @@ -325,10 +321,10 @@ TEST_F(TelemetryStoreTest, CorrectlyRedactsFindCommandRequestAllFields) { "HASH<sortVal>": 1, "HASH<otherSort>": -1 }, - "limit": "?", - "skip": "?", - "batchSize": "?", - "maxTimeMS": "?" + "limit": "?number", + "skip": "?number", + "batchSize": "?number", + "maxTimeMS": "?number" })", redacted); @@ -349,14 +345,12 @@ TEST_F(TelemetryStoreTest, CorrectlyRedactsFindCommandRequestAllFields) { "find": "HASH<testColl>", "filter": { "HASH<a>": { - "$eq": "?" + "$eq": "?number" } }, "let": { "HASH<var1>": "$HASH<a>", - "HASH<var2>": { - "$const": "?" - } + "HASH<var2>": "?string" }, "projection": { "HASH<e>": true, @@ -377,10 +371,10 @@ TEST_F(TelemetryStoreTest, CorrectlyRedactsFindCommandRequestAllFields) { "HASH<sortVal>": 1, "HASH<otherSort>": -1 }, - "limit": "?", - "skip": "?", - "batchSize": "?", - "maxTimeMS": "?" + "limit": "?number", + "skip": "?number", + "batchSize": "?number", + "maxTimeMS": "?number" })", redacted); } @@ -391,7 +385,7 @@ TEST_F(TelemetryStoreTest, CorrectlyRedactsFindCommandRequestEmptyFields) { fcr.setSort(BSONObj()); fcr.setProjection(BSONObj()); SerializationOptions opts; - opts.replacementForLiteralArgs = "?"; + opts.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; opts.redactIdentifiers = true; opts.identifierRedactionPolicy = redactFieldNameForTest; @@ -413,7 +407,9 @@ TEST_F(TelemetryStoreTest, CorrectlyRedactsHintsWithOptions) { FindCommandRequest fcr(NamespaceStringOrUUID(NamespaceString("testDB.testColl"))); fcr.setFilter(BSON("b" << 1)); SerializationOptions opts; + // TODO SERVER-75419 Use only 'literalPolicy.' opts.replacementForLiteralArgs = "?"; + opts.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; fcr.setHint(BSON("z" << 1 << "c" << 1)); fcr.setMax(BSON("z" << 25)); fcr.setMin(BSON("z" << 80)); @@ -429,7 +425,7 @@ TEST_F(TelemetryStoreTest, CorrectlyRedactsHintsWithOptions) { "find": "testColl", "filter": { "b": { - "$eq": "?" + "$eq": "?number" } }, "hint": { @@ -459,7 +455,7 @@ TEST_F(TelemetryStoreTest, CorrectlyRedactsHintsWithOptions) { "find": "testColl", "filter": { "b": { - "$eq": "?" + "$eq": "?number" } }, "hint": { @@ -478,6 +474,7 @@ TEST_F(TelemetryStoreTest, CorrectlyRedactsHintsWithOptions) { opts.identifierRedactionPolicy = redactFieldNameForTest; opts.redactIdentifiers = true; opts.replacementForLiteralArgs = boost::none; + opts.literalPolicy = LiteralSerializationPolicy::kUnchanged; redacted = uassertStatusOK(telemetry::makeTelemetryKey(fcr, opts, expCtx)); ASSERT_BSONOBJ_EQ_AUTO( // NOLINT R"({ @@ -504,33 +501,9 @@ TEST_F(TelemetryStoreTest, CorrectlyRedactsHintsWithOptions) { })", redacted); - redacted = uassertStatusOK(telemetry::makeTelemetryKey(fcr, opts, expCtx)); - ASSERT_BSONOBJ_EQ_AUTO( // NOLINT - R"({ - "cmdNs": { - "db": "HASH<testDB>", - "coll": "HASH<testColl>" - }, - "find": "HASH<testColl>", - "filter": { - "HASH<b>": { - "$eq": 1 - } - }, - "hint": { - "HASH<z>": 1, - "HASH<c>": 1 - }, - "max": { - "HASH<z>": 25 - }, - "min": { - "HASH<z>": 80 - } - })", - redacted); - + // TODO SERVER-75419 Use only 'literalPolicy.' opts.replacementForLiteralArgs = "?"; + opts.literalPolicy = LiteralSerializationPolicy::kToDebugTypeString; redacted = uassertStatusOK(telemetry::makeTelemetryKey(fcr, opts, expCtx)); ASSERT_BSONOBJ_EQ_AUTO( // NOLINT R"({ @@ -541,7 +514,7 @@ TEST_F(TelemetryStoreTest, CorrectlyRedactsHintsWithOptions) { "find": "HASH<testColl>", "filter": { "HASH<b>": { - "$eq": "?" + "$eq": "?number" } }, "hint": { @@ -567,7 +540,7 @@ TEST_F(TelemetryStoreTest, CorrectlyRedactsHintsWithOptions) { "find": "HASH<testColl>", "filter": { "HASH<b>": { - "$eq": "?" + "$eq": "?number" } }, "hint": { |