diff options
| -rw-r--r-- | jstests/aggregation/sources/group/group_by_system_variable.js | 27 | ||||
| -rw-r--r-- | src/mongo/db/pipeline/expression.h | 18 | ||||
| -rw-r--r-- | src/mongo/db/pipeline/semantic_analysis.cpp | 2 |
3 files changed, 44 insertions, 3 deletions
diff --git a/jstests/aggregation/sources/group/group_by_system_variable.js b/jstests/aggregation/sources/group/group_by_system_variable.js new file mode 100644 index 00000000000..6c223343166 --- /dev/null +++ b/jstests/aggregation/sources/group/group_by_system_variable.js @@ -0,0 +1,27 @@ +/* + * Tests that the server doesn't crash when you group by a system variable. + * Reproduces SERVER-57164. + */ +(function() { +"use strict"; + +const coll = db.group_by_system_variable; +coll.drop(); +assert.commandWorked(coll.insert({})); + +// These explains all should not throw. +coll.explain().aggregate({$group: {_id: "$$IS_MR"}}); +coll.explain().aggregate({$group: {_id: "$$JS_SCOPE"}}); +coll.explain().aggregate({$group: {_id: "$$CLUSTER_TIME"}}); + +// These queries throw, but the server should stay up. +assert.throws(() => coll.aggregate({$group: {_id: "$$IS_MR"}})); +assert.throws(() => coll.aggregate({$group: {_id: "$$JS_SCOPE"}})); +try { + // This one may or may not throw: CLUSTER_TIME may or may not be defined, + // depending on what kind of cluster we're running against. + coll.aggregate({$group: {_id: "$$CLUSTER_TIME"}}); +} catch (e) { +} +db.hello(); +})(); diff --git a/src/mongo/db/pipeline/expression.h b/src/mongo/db/pipeline/expression.h index d5f29a021b5..61479668339 100644 --- a/src/mongo/db/pipeline/expression.h +++ b/src/mongo/db/pipeline/expression.h @@ -1553,12 +1553,26 @@ public: class ExpressionFieldPath : public Expression { public: - bool isRootFieldPath() const { + /** + * Checks whether this field path is exactly "$$ROOT". + */ + bool isROOT() const { return _variable == Variables::kRootId && _fieldPath.getPathLength() == 1; } + /** + * Checks whether this field path starts with a variable besides ROOT. + * + * For example, these are variable references: + * "$$NOW" + * "$$NOW.x" + * and these are not: + * "$x" + * "$$ROOT" + * "$$ROOT.x" + */ bool isVariableReference() const { - return Variables::isUserDefinedVariable(_variable); + return _variable != Variables::kRootId; } boost::intrusive_ptr<Expression> optimize() final; diff --git a/src/mongo/db/pipeline/semantic_analysis.cpp b/src/mongo/db/pipeline/semantic_analysis.cpp index 36f1985a913..c1613a6a85f 100644 --- a/src/mongo/db/pipeline/semantic_analysis.cpp +++ b/src/mongo/db/pipeline/semantic_analysis.cpp @@ -130,7 +130,7 @@ boost::optional<std::string> replaceRootNestsRoot( } auto&& [nestedName, expression] = children[0]; if (!dynamic_cast<ExpressionFieldPath*>(expression.get()) || - !dynamic_cast<ExpressionFieldPath*>(expression.get())->isRootFieldPath()) { + !dynamic_cast<ExpressionFieldPath*>(expression.get())->isROOT()) { return boost::none; } return nestedName; |