commit 7f25ab0eecea99b5b0e3f58612d9b0d62c325df6 Author: Charlie Swanson Date: Thu Jan 9 12:34:47 2020 -0500 Add support for storing $unionWith in a view diff --git a/jstests/core/views/views_aggregation.js b/jstests/core/views/views_aggregation.js index 6f60c0027c..f81643ea7d 100644 --- a/jstests/core/views/views_aggregation.js +++ b/jstests/core/views/views_aggregation.js @@ -352,4 +352,35 @@ assert.commandWorked(viewsDB.runCommand({ [{$unionWith: {coll: "identityView", pipeline: [{$match: {_id: "New York"}}]}}]) .itcount()); }()); + +(function testUnionInViewDefinition() { + const secondCollection = viewsDB.secondCollection; + secondCollection.drop(); + assert.commandWorked(secondCollection.insert(allDocuments)); + const viewName = "unionView"; + + // Test with a simple $unionWith with no custom pipeline. + assert.commandWorked(viewsDB.runCommand({ + create: viewName, + viewOn: coll.getName(), + pipeline: [{$unionWith: secondCollection.getName()}] + })); + assert.eq(2 * allDocuments.length, viewsDB[viewName].find().itcount()); + assert.eq(allDocuments.length, + viewsDB[viewName].aggregate([{$group: {_id: "$_id"}}]).itcount()); + assert.eq(allDocuments.length, viewsDB[viewName].distinct("_id").length); + viewsDB[viewName].drop(); + + // Now test again with a custom pipeline in the view definition. + assert.commandWorked(viewsDB.runCommand({ + create: viewName, + viewOn: coll.getName(), + pipeline: + [{$unionWith: {coll: secondCollection.getName(), pipeline: [{$match: {state: "NY"}}]}}] + })); + assert.eq(allDocuments.length + 1, viewsDB[viewName].find().itcount()); + assert.eq(allDocuments.length, + viewsDB[viewName].aggregate([{$group: {_id: "$_id"}}]).itcount()); + assert.eq(allDocuments.length, viewsDB[viewName].distinct("_id").length); +}()); }()); diff --git a/jstests/core/views/views_validation.js b/jstests/core/views/views_validation.js index 70a87a1fa1..c35db48162 100644 --- a/jstests/core/views/views_validation.js +++ b/jstests/core/views/views_validation.js @@ -43,6 +43,10 @@ function makeFacet(from) { return {$facet: {"Facet Key": [makeLookup(from)]}}; } +function makeUnion(from) { + return {$unionWith: from}; +} + function clear() { assert.commandWorked(viewsDb.dropDatabase()); } @@ -51,12 +55,10 @@ clear(); // Check that simple cycles are disallowed. makeView("a", "a", [], ErrorCodes.GraphContainsCycle); -makeView("a", "b", [makeLookup("a")], ErrorCodes.GraphContainsCycle); clear(); -makeView("a", "b", ErrorCodes.OK); +makeView("a", "b"); makeView("b", "a", [], ErrorCodes.GraphContainsCycle); -makeView("b", "c", [makeLookup("a")], ErrorCodes.GraphContainsCycle); clear(); makeView("a", "b"); @@ -64,6 +66,32 @@ makeView("b", "c"); makeView("c", "a", [], ErrorCodes.GraphContainsCycle); clear(); +// Test that $lookup checks for cycles. +makeView("a", "b", [makeLookup("a")], ErrorCodes.GraphContainsCycle); +clear(); + +makeView("a", "b"); +makeView("b", "c", [makeLookup("a")], ErrorCodes.GraphContainsCycle); +clear(); + +// Test that $graphLookup checks for cycles. +makeView("a", "b", [makeGraphLookup("a")], ErrorCodes.GraphContainsCycle); +makeView("a", "b", [makeGraphLookup("b")]); +makeView("b", "c", [makeGraphLookup("a")], ErrorCodes.GraphContainsCycle); +clear(); + +// Test that $facet checks for cycles. +makeView("a", "b", [makeFacet("a")], ErrorCodes.GraphContainsCycle); +makeView("a", "b", [makeFacet("b")]); +makeView("b", "c", [makeFacet("a")], ErrorCodes.GraphContainsCycle); +clear(); + +// Test that $unionWith checks for cycles. +makeView("a", "b", [makeUnion("a")], ErrorCodes.GraphContainsCycle); +makeView("a", "b", [makeUnion("b")]); +makeView("b", "c", [makeUnion("a")], ErrorCodes.GraphContainsCycle); +clear(); + /* * Check that view validation does not naively recurse on already visited views. * @@ -82,30 +110,27 @@ clear(); for (let i = 1; i <= kMaxViewDepth; i++) { let childView = "v" + (i + 1); - makeView("v" + i, - childView, - [makeLookup(childView), makeGraphLookup(childView), makeFacet(childView)]); + makeView("v" + i, childView, [ + makeLookup(childView), + makeGraphLookup(childView), + makeFacet(childView), + makeUnion(childView), + ]); } // Check that any higher depth leads to failure makeView("v21", "v22", [], ErrorCodes.ViewDepthLimitExceeded); makeView("v0", "v1", [], ErrorCodes.ViewDepthLimitExceeded); makeView("v0", "ok", [makeLookup("v1")], ErrorCodes.ViewDepthLimitExceeded); +makeView("v0", "ok", [makeGraphLookup("v1")], ErrorCodes.ViewDepthLimitExceeded); +makeView("v0", "ok", [makeFacet("v1")], ErrorCodes.ViewDepthLimitExceeded); +makeView("v0", "ok", [makeUnion("v1")], ErrorCodes.ViewDepthLimitExceeded); // But adding to the middle should be ok. makeView("vMid", "v10"); clear(); -// Check that $graphLookup and $facet also check for cycles. -makeView("a", "b", [makeGraphLookup("a")], ErrorCodes.GraphContainsCycle); -makeView("a", "b", [makeGraphLookup("b")]); -makeView("b", "c", [makeGraphLookup("a")], ErrorCodes.GraphContainsCycle); -clear(); - -makeView("a", "b", [makeFacet("a")], ErrorCodes.GraphContainsCycle); -makeView("a", "b", [makeFacet("b")]); -makeView("b", "c", [makeFacet("a")], ErrorCodes.GraphContainsCycle); -clear(); +// Test that $facet checks for cycles. // Check that collMod also checks for cycles. makeView("a", "b"); diff --git a/src/mongo/db/pipeline/document_source_lookup.cpp b/src/mongo/db/pipeline/document_source_lookup.cpp index c30c3a9d6c..f63def27de 100644 --- a/src/mongo/db/pipeline/document_source_lookup.cpp +++ b/src/mongo/db/pipeline/document_source_lookup.cpp @@ -50,8 +50,6 @@ namespace mongo { using boost::intrusive_ptr; using std::vector; -constexpr size_t DocumentSourceLookUp::kMaxSubPipelineDepth; - DocumentSourceLookUp::DocumentSourceLookUp(NamespaceString fromNs, std::string as, const boost::intrusive_ptr& expCtx) @@ -68,8 +66,8 @@ DocumentSourceLookUp::DocumentSourceLookUp(NamespaceString fromNs, _fromExpCtx->subPipelineDepth += 1; uassert(ErrorCodes::MaxSubPipelineDepthExceeded, str::stream() << "Maximum number of nested $lookup sub-pipelines exceeded. Limit is " - << kMaxSubPipelineDepth, - _fromExpCtx->subPipelineDepth <= kMaxSubPipelineDepth); + << ExpressionContext::kMaxSubPipelineViewDepth, + _fromExpCtx->subPipelineDepth <= ExpressionContext::kMaxSubPipelineViewDepth); } DocumentSourceLookUp::DocumentSourceLookUp(NamespaceString fromNs, diff --git a/src/mongo/db/pipeline/document_source_lookup.h b/src/mongo/db/pipeline/document_source_lookup.h index 6e72a8d7f4..65848ccb80 100644 --- a/src/mongo/db/pipeline/document_source_lookup.h +++ b/src/mongo/db/pipeline/document_source_lookup.h @@ -48,7 +48,6 @@ namespace mongo { */ class DocumentSourceLookUp final : public DocumentSource { public: - static constexpr size_t kMaxSubPipelineDepth = 20; static constexpr StringData kStageName = "$lookup"_sd; struct LetVariable { diff --git a/src/mongo/db/pipeline/document_source_union_with.cpp b/src/mongo/db/pipeline/document_source_union_with.cpp index b474fcd5a0..831d317be6 100644 --- a/src/mongo/db/pipeline/document_source_union_with.cpp +++ b/src/mongo/db/pipeline/document_source_union_with.cpp @@ -86,12 +86,20 @@ DocumentSourceUnionWith::DocumentSourceUnionWith( // Copy the ExpressionContext of the base aggregation, using the inner namespace instead. _unionExpCtx = expCtx->copyWith(_resolvedNss); + _unionExpCtx->subPipelineDepth += 1; + // TODO add test for this. + uassert(ErrorCodes::MaxSubPipelineDepthExceeded, + str::stream() << "Maximum number of nested $lookup sub-pipelines exceeded. Limit is " + << ExpressionContext::kMaxSubPipelineViewDepth, + _unionExpCtx->subPipelineDepth <= ExpressionContext::kMaxSubPipelineViewDepth); _rawPipeline.insert( _rawPipeline.begin(), resolvedNamespace.pipeline.begin(), resolvedNamespace.pipeline.end()); - // TODO SERVER-XXXX: This can't happen here in a sharded cluster, since it attaches a - // non-serializable $cursor stage. - _pipeline = pExpCtx->mongoProcessInterface->makePipeline(_rawPipeline, _unionExpCtx); + if (!expCtx->isParsingViewDefinition) { + // TODO SERVER-XXXX: This can't happen here in a sharded cluster, since it attaches a + // non-serializable $cursor stage. + _pipeline = pExpCtx->mongoProcessInterface->makePipeline(_rawPipeline, _unionExpCtx); + } } boost::intrusive_ptr DocumentSourceUnionWith::createFromBson( diff --git a/src/mongo/db/pipeline/expression_context.h b/src/mongo/db/pipeline/expression_context.h index 6fde76c2c0..d693a5d169 100644 --- a/src/mongo/db/pipeline/expression_context.h +++ b/src/mongo/db/pipeline/expression_context.h @@ -59,6 +59,7 @@ namespace mongo { class ExpressionContext : public RefCountable { public: + static constexpr size_t kMaxSubPipelineViewDepth = 20; struct ResolvedNamespace { ResolvedNamespace() = default; ResolvedNamespace(NamespaceString ns, std::vector pipeline);