Skip to content

Commit 31e870e

Browse files
ericoxEvergreen Agent
ericox
authored and
Evergreen Agent
committed
SERVER-47191 Implement evalutation of Expressions to produce let parameters
1 parent 74ac57a commit 31e870e

File tree

16 files changed

+483
-119
lines changed

16 files changed

+483
-119
lines changed

‎jstests/aggregation/expressions/let.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,8 @@ assertErrorCode(coll, {$project: {a: {$let: {vars: {FOO: 1}, in : '$$FOO'}}}}, 1
116116
assertErrorCode(coll, {$project: {a: {$let: {vars: {_underbar: 1}, in : '$$FOO'}}}}, 16867);
117117
assertErrorCode(coll, {$project: {a: {$let: {vars: {'a.b': 1}, in : '$$FOO'}}}}, 16868);
118118
assertErrorCode(coll, {$project: {a: {$let: {vars: {'a b': 1}, in : '$$FOO'}}}}, 16868);
119-
assertErrorCode(coll, {$project: {a: '$$_underbar'}}, 16870);
120-
assertErrorCode(coll, {$project: {a: '$$with spaces'}}, 16871);
119+
assertErrorCode(coll, {$project: {a: '$$_underbar'}}, 16867);
120+
assertErrorCode(coll, {$project: {a: '$$with spaces'}}, 16868);
121121

122122
// Verify that variables defined in '$let' cannot be used to initialize other variables.
123123
assertErrorCode(

‎jstests/aggregation/variables/runtime_constants.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,11 @@ assert.commandWorked(coll.insert({x: true}));
1616
assert.commandFailedWithCode(
1717
db.runCommand(
1818
{aggregate: coll.getName(), pipeline: [{$addFields: {testField: "$$IS_MR"}}], cursor: {}}),
19-
17276);
19+
4631101);
2020

2121
// Runtime constant $$JS_SCOPE is unable to be retrieved by users.
2222
assert.commandFailedWithCode(
2323
db.runCommand(
2424
{aggregate: coll.getName(), pipeline: [{$addFields: {field: "$$JS_SCOPE"}}], cursor: {}}),
25-
17276);
26-
})();
25+
4631100);
26+
})();

‎src/mongo/db/matcher/SConscript‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ env.Library(
6767
'$BUILD_DIR/mongo/db/geo/geoparser',
6868
'$BUILD_DIR/mongo/db/query/collation/collator_interface',
6969
'$BUILD_DIR/mongo/db/query/query_knobs',
70-
'$BUILD_DIR/mongo/db/pipeline/expression',
70+
'$BUILD_DIR/mongo/db/pipeline/expression_context',
7171
'$BUILD_DIR/mongo/idl/idl_parser',
7272
'$BUILD_DIR/mongo/util/regex_util',
7373
'$BUILD_DIR/third_party/shim_pcrecpp',

‎src/mongo/db/pipeline/SConscript‎

Lines changed: 19 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -57,17 +57,30 @@ env.Library(
5757
env.Library(
5858
target='expression_context',
5959
source=[
60+
'expression.cpp',
6061
'expression_context.cpp',
62+
'expression_function.cpp',
63+
'expression_js_emit.cpp',
64+
'expression_trigonometric.cpp',
6165
'javascript_execution.cpp',
66+
'make_js_function.cpp',
6267
'variables.cpp',
6368
],
6469
LIBDEPS=[
65-
'aggregation_request',
70+
'$BUILD_DIR/mongo/db/query/query_knobs',
71+
'$BUILD_DIR/mongo/db/exec/document_value/document_value',
6672
'$BUILD_DIR/mongo/db/query/collation/collator_factory_interface',
67-
'$BUILD_DIR/mongo/db/query/query_knobs',
73+
'$BUILD_DIR/mongo/db/query/datetime/date_time_support',
74+
'$BUILD_DIR/mongo/db/query/query_knobs',
75+
'$BUILD_DIR/mongo/db/server_options_core',
6876
'$BUILD_DIR/mongo/db/service_context',
6977
'$BUILD_DIR/mongo/scripting/scripting',
78+
'$BUILD_DIR/mongo/scripting/scripting_common',
7079
'$BUILD_DIR/mongo/util/intrusive_counter',
80+
'$BUILD_DIR/mongo/util/regex_util',
81+
'$BUILD_DIR/mongo/util/summation',
82+
'aggregation_request',
83+
'dependencies',
7184
]
7285
)
7386

@@ -82,37 +95,6 @@ env.Library(
8295
]
8396
)
8497

85-
env.Library(
86-
target='expression',
87-
source=[
88-
'expression.cpp',
89-
'expression_trigonometric.cpp',
90-
'make_js_function.cpp'
91-
],
92-
LIBDEPS=[
93-
'$BUILD_DIR/mongo/db/exec/document_value/document_value',
94-
'$BUILD_DIR/mongo/db/query/datetime/date_time_support',
95-
'$BUILD_DIR/mongo/db/server_options_core',
96-
'$BUILD_DIR/mongo/util/regex_util',
97-
'$BUILD_DIR/mongo/util/summation',
98-
'dependencies',
99-
'expression_context',
100-
]
101-
)
102-
103-
env.Library(
104-
target='expression_javascript',
105-
source=[
106-
'expression_function.cpp',
107-
'expression_js_emit.cpp'
108-
],
109-
LIBDEPS=[
110-
'$BUILD_DIR/mongo/scripting/scripting_common',
111-
'$BUILD_DIR/mongo/db/query/query_knobs',
112-
'expression',
113-
]
114-
)
115-
11698
env.Library(
11799
target='accumulator',
118100
source=[
@@ -133,7 +115,7 @@ env.Library(
133115
'$BUILD_DIR/mongo/db/query/query_knobs',
134116
'$BUILD_DIR/mongo/scripting/scripting_common',
135117
'$BUILD_DIR/mongo/util/summation',
136-
'expression',
118+
'expression_context',
137119
'field_path',
138120
]
139121
)
@@ -147,7 +129,7 @@ env.Library(
147129
],
148130
LIBDEPS=[
149131
'$BUILD_DIR/mongo/db/exec/document_value/document_value',
150-
'expression',
132+
'expression_context',
151133
'field_path',
152134
]
153135
)
@@ -289,9 +271,7 @@ pipelineEnv.Library(
289271
'dependencies',
290272
'document_path_support',
291273
'document_sources_idl',
292-
'expression',
293274
'expression_context',
294-
'expression_javascript',
295275
'granularity_rounder',
296276
],
297277
LIBDEPS_PRIVATE=[
@@ -379,6 +359,7 @@ env.CppUnitTest(
379359
'document_source_unwind_test.cpp',
380360
'expression_and_test.cpp',
381361
'expression_compare_test.cpp',
362+
'expression_context_test.cpp',
382363
'expression_convert_test.cpp',
383364
'expression_date_test.cpp',
384365
'expression_field_path_test.cpp',
@@ -428,7 +409,7 @@ env.CppUnitTest(
428409
'aggregation_request',
429410
'document_source_mock',
430411
'document_sources_idl',
431-
'expression',
412+
'expression_context',
432413
'field_path',
433414
'granularity_rounder',
434415
'pipeline',

‎src/mongo/db/pipeline/document_source_lookup.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ DocumentSourceLookUp::DocumentSourceLookUp(NamespaceString fromNs,
9797

9898
for (auto&& varElem : letVariables) {
9999
const auto varName = varElem.fieldNameStringData();
100-
Variables::uassertValidNameForUserWrite(varName);
100+
Variables::validateNameForUserWrite(varName);
101101

102102
_letVariables.emplace_back(
103103
varName.toString(),

‎src/mongo/db/pipeline/document_source_merge.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,7 @@ DocumentSourceMerge::DocumentSourceMerge(NamespaceString outputNs,
370370

371371
for (auto&& varElem : *letVariables) {
372372
const auto varName = varElem.fieldNameStringData();
373-
Variables::uassertValidNameForUserWrite(varName);
373+
Variables::validateNameForUserWrite(varName);
374374

375375
_letVariables->emplace(
376376
varName.toString(),

‎src/mongo/db/pipeline/expression.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2068,7 +2068,7 @@ intrusive_ptr<ExpressionFieldPath> ExpressionFieldPath::parse(
20682068
const StringData rawSD = raw;
20692069
const StringData fieldPath = rawSD.substr(2); // strip off $$
20702070
const StringData varName = fieldPath.substr(0, fieldPath.find('.'));
2071-
Variables::uassertValidNameForUserRead(varName);
2071+
Variables::validateNameForUserRead(varName);
20722072
auto varId = vps.getVariable(varName);
20732073
return new ExpressionFieldPath(expCtx, fieldPath.toString(), varId);
20742074
} else {
@@ -2252,7 +2252,7 @@ intrusive_ptr<Expression> ExpressionFilter::parse(
22522252
// If "as" is not specified, then use "this" by default.
22532253
auto varName = asElem.eoo() ? "this" : asElem.str();
22542254

2255-
Variables::uassertValidNameForUserWrite(varName);
2255+
Variables::validateNameForUserWrite(varName);
22562256
Variables::Id varId = vpsSub.defineVariable(varName);
22572257

22582258
// Parse "cond", has access to "as" variable.
@@ -2382,7 +2382,7 @@ intrusive_ptr<Expression> ExpressionLet::parse(
23822382
std::vector<boost::intrusive_ptr<Expression>>::size_type index = 0;
23832383
for (auto&& varElem : varsObj) {
23842384
const string varName = varElem.fieldName();
2385-
Variables::uassertValidNameForUserWrite(varName);
2385+
Variables::validateNameForUserWrite(varName);
23862386
Variables::Id id = vpsSub.defineVariable(varName);
23872387

23882388
vars.emplace(id, NameAndExpression{varName, children[index]}); // only has outer vars
@@ -2490,7 +2490,7 @@ intrusive_ptr<Expression> ExpressionMap::parse(
24902490
// If "as" is not specified, then use "this" by default.
24912491
auto varName = asElem.eoo() ? "this" : asElem.str();
24922492

2493-
Variables::uassertValidNameForUserWrite(varName);
2493+
Variables::validateNameForUserWrite(varName);
24942494
Variables::Id varId = vpsSub.defineVariable(varName);
24952495

24962496
// parse "in"

‎src/mongo/db/pipeline/expression_context.cpp‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
#include "mongo/db/query/collation/collation_spec.h"
3737
#include "mongo/db/query/collation/collator_factory_interface.h"
3838
#include "mongo/util/intrusive_counter.h"
39+
#include "mongo/util/scopeguard.h"
3940

4041
namespace mongo {
4142

@@ -87,7 +88,8 @@ ExpressionContext::ExpressionContext(
8788
const std::shared_ptr<MongoProcessInterface>& mongoProcessInterface,
8889
StringMap<ExpressionContext::ResolvedNamespace> resolvedNamespaces,
8990
boost::optional<UUID> collUUID,
90-
bool mayDbProfile)
91+
bool mayDbProfile,
92+
const boost::optional<BSONObj>& letParameters)
9193
: explain(explain),
9294
fromMongos(fromMongos),
9395
needsMerge(needsMerge),
@@ -107,7 +109,13 @@ ExpressionContext::ExpressionContext(
107109
_valueComparator(_collator.get()),
108110
_resolvedNamespaces(std::move(resolvedNamespaces)) {
109111

110-
if (runtimeConstants) {
112+
if (runtimeConstants && runtimeConstants->getClusterTime().isNull()) {
113+
// Try to get a default value for clusterTime if a logical clock exists.
114+
auto genConsts = variables.generateRuntimeConstants(opCtx);
115+
genConsts.setJsScope(runtimeConstants->getJsScope());
116+
genConsts.setIsMapReduce(runtimeConstants->getIsMapReduce());
117+
variables.setRuntimeConstants(genConsts);
118+
} else if (runtimeConstants) {
111119
variables.setRuntimeConstants(*runtimeConstants);
112120
} else {
113121
variables.setDefaultRuntimeConstants(opCtx);
@@ -116,6 +124,16 @@ ExpressionContext::ExpressionContext(
116124
if (!isMapReduce) {
117125
jsHeapLimitMB = internalQueryJavaScriptHeapSizeLimitMB.load();
118126
}
127+
if (letParameters) {
128+
// TODO SERVER-47713: One possible fix is to change the interface of everything that needs
129+
// an expression context intrusive_ptr to take a raw ptr.
130+
auto intrusiveThis = boost::intrusive_ptr{this};
131+
ON_BLOCK_EXIT([&] {
132+
intrusiveThis.detach();
133+
unsafeRefDecRefCountTo(0u);
134+
});
135+
variables.seedVariablesWithLetParameters(intrusiveThis, *letParameters);
136+
}
119137
}
120138

121139
ExpressionContext::ExpressionContext(OperationContext* opCtx,

‎src/mongo/db/pipeline/expression_context.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,9 @@ class ExpressionContext : public RefCountable {
128128
const std::shared_ptr<MongoProcessInterface>& mongoProcessInterface,
129129
StringMap<ExpressionContext::ResolvedNamespace> resolvedNamespaces,
130130
boost::optional<UUID> collUUID,
131-
bool mayDbProfile);
131+
bool mayDbProfile,
132+
const boost::optional<BSONObj>& letParameters = boost::none);
133+
132134

133135
/**
134136
* Constructs an ExpressionContext suitable for use outside of the aggregation system, including

0 commit comments

Comments
 (0)