diff options
Diffstat (limited to 'src/mongo/db/pipeline/window_function')
14 files changed, 97 insertions, 161 deletions
diff --git a/src/mongo/db/pipeline/window_function/partition_iterator.cpp b/src/mongo/db/pipeline/window_function/partition_iterator.cpp index 83925b862b3..d8992aec28c 100644 --- a/src/mongo/db/pipeline/window_function/partition_iterator.cpp +++ b/src/mongo/db/pipeline/window_function/partition_iterator.cpp @@ -116,8 +116,8 @@ optional<Document> PartitionIterator::operator[](int index) { for (int i = _cache->getHighestIndex(); i < docDesired; i++) { // Pull in document from prior stage. getNextDocument(); - // Check whether the next document is available. - if (isPaused() || _state == IteratorState::kAwaitingAdvanceToNext || + // Check for EOF or the next partition. + if (_state == IteratorState::kAwaitingAdvanceToNext || _state == IteratorState::kAwaitingAdvanceToEOF) { return boost::none; } @@ -163,7 +163,6 @@ PartitionIterator::AdvanceResult PartitionIterator::advanceInternal() { // whether to pull from the prior stage. switch (_state) { case IteratorState::kNotInitialized: - case IteratorState::kPauseExecution: case IteratorState::kIntraPartition: // Pull in the next document and advance the pointer. getNextDocument(); @@ -302,13 +301,8 @@ optional<std::pair<int, int>> PartitionIterator::getEndpointsRangeBased( for (int i = start; (doc = (*this)[i]); ++i) { Value v = (*_sortExpr)->evaluate(*doc, &_expCtx->variables); if (!lessThan(v, threshold)) { - // This is the first doc we've scanned that crossed the threshold, - // so it's the first doc in the window (as long as it's the expected type). - if (hasExpectedType(v)) { - return i; - } else { - return boost::none; - } + // This is the first doc we've scanned that crossed the threshold. + return i; } } // We scanned every document in the partition, and none crossed the @@ -473,20 +467,17 @@ void PartitionIterator::getNextDocument() { return; } - if (getNextRes.isPaused()) { - _state = IteratorState::kPauseExecution; + if (!getNextRes.isAdvanced()) return; - } - tassert(7169100, "getNextResult must have advanced", getNextRes.isAdvanced()); auto doc = getNextRes.releaseDocument(); // Greedily populate the internal document cache to enable easier memory tracking versus // detecting the changing document size during execution of each function. - doc = doc.shred(); + doc.fillCache(); if (_partitionExpr) { - if (!_partitionComparator) { + if (_state == IteratorState::kNotInitialized) { _partitionComparator = std::make_unique<PartitionKeyComparator>(_expCtx, *_partitionExpr, doc); _nextPartitionDoc = std::move(doc); diff --git a/src/mongo/db/pipeline/window_function/partition_iterator.h b/src/mongo/db/pipeline/window_function/partition_iterator.h index 28e0e6a6242..128901834ee 100644 --- a/src/mongo/db/pipeline/window_function/partition_iterator.h +++ b/src/mongo/db/pipeline/window_function/partition_iterator.h @@ -77,13 +77,6 @@ public: return (*this)[0]; } - /** - * Returns true if iterator execution is paused. - */ - bool isPaused() { - return _state == IteratorState::kPauseExecution; - } - enum class AdvanceResult { kAdvanced, kNewPartition, @@ -294,9 +287,6 @@ private: enum class IteratorState { // Default state, no documents have been pulled into the cache. kNotInitialized, - // Input sources do not have a result to be processed yet, but there may be more results in - // the future. - kPauseExecution, // Iterating the current partition. We don't know where the current partition ends, or // whether it's the last partition. kIntraPartition, diff --git a/src/mongo/db/pipeline/window_function/partition_iterator_test.cpp b/src/mongo/db/pipeline/window_function/partition_iterator_test.cpp index 8b7a6beab76..cb6b5bcf1bf 100644 --- a/src/mongo/db/pipeline/window_function/partition_iterator_test.cpp +++ b/src/mongo/db/pipeline/window_function/partition_iterator_test.cpp @@ -499,20 +499,22 @@ TEST_F(PartitionIteratorTest, MemoryUsageAccountsForDocumentIteratorCache) { const auto mock = DocumentSourceMock::createForTest(docs, getExpCtx()); [[maybe_unused]] auto accessor = makeDefaultAccessor(mock, boost::none); - size_t initialDocSize = docs[0].getDocument().getCurrentApproximateSize(); + size_t initialDocSize = docs[0].getDocument().getApproximateSize(); - // Pull in the first document, and verify the reported size of the iterator is roughly the size - // of the document. + // Pull in the first document, and verify the reported size of the iterator is roughly double + // the size of the document. The size of the iterator is double the size of the document because + // we greedily fill the cache, so each internal document in memory stores two copies of + // largeStr. ASSERT_DOCUMENT_EQ(*_iter->current(), docs[0].getDocument()); - ASSERT_GT(_iter->getApproximateSize(), initialDocSize); - ASSERT_LT(_iter->getApproximateSize(), initialDocSize + 500); + ASSERT_GT(_iter->getApproximateSize(), initialDocSize * 2); + ASSERT_LT(_iter->getApproximateSize(), initialDocSize * 2 + 500); // Pull in the second document. Both docs remain in the cache so the reported memory should // include both. advance(); ASSERT_DOCUMENT_EQ(*_iter->current(), docs[1].getDocument()); - ASSERT_GT(_iter->getApproximateSize(), initialDocSize); - ASSERT_LT(_iter->getApproximateSize(), initialDocSize * 2 + 500); + ASSERT_GT(_iter->getApproximateSize(), initialDocSize * 2 * 2); + ASSERT_LT(_iter->getApproximateSize(), initialDocSize * 2 * 2 + 500); } TEST_F(PartitionIteratorTest, MemoryUsageAccountsForArraysInDocumentIteratorCache) { @@ -523,21 +525,21 @@ TEST_F(PartitionIteratorTest, MemoryUsageAccountsForArraysInDocumentIteratorCach const auto mock = DocumentSourceMock::createForTest(docs, getExpCtx()); [[maybe_unused]] auto accessor = makeDefaultAccessor(mock, boost::none); - size_t initialDocSize = docs[0].getDocument().getCurrentApproximateSize(); + size_t initialDocSize = docs[0].getDocument().getApproximateSize(); - // Pull in the first document, and verify the reported size of the iterator is roughly the size - // of the document. The reason we can't use EQ is that for memory tracking we call shred() so - // that the document cache will not increase when fields are accessed + // Pull in the first document, and verify the reported size of the iterator is roughly + // triple the size of the document. The reason for this is that 'largeStr' is cached twice; once + // for the 'arr' element and once for the nested 'subObj' element. ASSERT_DOCUMENT_EQ(*_iter->current(), docs[0].getDocument()); - ASSERT_GT(_iter->getApproximateSize(), initialDocSize); - ASSERT_LT(_iter->getApproximateSize(), initialDocSize + 1024); + ASSERT_GT(_iter->getApproximateSize(), initialDocSize * 3); + ASSERT_LT(_iter->getApproximateSize(), initialDocSize * 3 + 1024); // Pull in the second document. Both docs remain in the cache so the reported memory should // include both. advance(); ASSERT_DOCUMENT_EQ(*_iter->current(), docs[1].getDocument()); - ASSERT_GT(_iter->getApproximateSize(), initialDocSize * 2); - ASSERT_LT(_iter->getApproximateSize(), initialDocSize * 2 + 1024); + ASSERT_GT(_iter->getApproximateSize(), (initialDocSize * 3) * 2); + ASSERT_LT(_iter->getApproximateSize(), (initialDocSize * 3) * 2 + 1024); } TEST_F(PartitionIteratorTest, MemoryUsageAccountsForNestedArraysInDocumentIteratorCache) { @@ -548,21 +550,21 @@ TEST_F(PartitionIteratorTest, MemoryUsageAccountsForNestedArraysInDocumentIterat const auto mock = DocumentSourceMock::createForTest(docs, getExpCtx()); [[maybe_unused]] auto accessor = makeDefaultAccessor(mock, boost::none); - size_t initialDocSize = docs[0].getDocument().getCurrentApproximateSize(); + size_t initialDocSize = docs[0].getDocument().getApproximateSize(); - // Pull in the first document, and verify the reported size of the iterator is roughly the size - // of the document. The reason we can't use EQ is that for memory tracking we call shred() so - // that the document cache will not increase when fields are accessed + // Pull in the first document, and verify the reported size of the iterator is roughly + // triple the size of the document. The reason for this is that 'largeStr' is cached twice; once + // for the 'arr' element and once for the nested 'subObj' element. ASSERT_DOCUMENT_EQ(*_iter->current(), docs[0].getDocument()); - ASSERT_GT(_iter->getApproximateSize(), initialDocSize); - ASSERT_LT(_iter->getApproximateSize(), initialDocSize + 1024); + ASSERT_GT(_iter->getApproximateSize(), initialDocSize * 3); + ASSERT_LT(_iter->getApproximateSize(), initialDocSize * 3 + 1024); // Pull in the second document. Both docs remain in the cache so the reported memory should // include both. advance(); ASSERT_DOCUMENT_EQ(*_iter->current(), docs[1].getDocument()); - ASSERT_GT(_iter->getApproximateSize(), initialDocSize * 2); - ASSERT_LT(_iter->getApproximateSize(), initialDocSize * 2 + 1024); + ASSERT_GT(_iter->getApproximateSize(), (initialDocSize * 3) * 2); + ASSERT_LT(_iter->getApproximateSize(), (initialDocSize * 3) * 2 + 1024); } TEST_F(PartitionIteratorTest, MemoryUsageAccountsForNestedObjInDocumentIteratorCache) { @@ -573,12 +575,13 @@ TEST_F(PartitionIteratorTest, MemoryUsageAccountsForNestedObjInDocumentIteratorC const auto mock = DocumentSourceMock::createForTest(docs, getExpCtx()); [[maybe_unused]] auto accessor = makeDefaultAccessor(mock, boost::none); - size_t initialDocSize = docs[0].getDocument().getCurrentApproximateSize(); + size_t initialDocSize = docs[0].getDocument().getApproximateSize(); - // Pull in the first document, and verify the reported size. + // Pull in the first document, and verify the reported size. TODO SERVER-57011: The approximate + // size should not double count the nested strings. ASSERT_DOCUMENT_EQ(*_iter->current(), docs[0].getDocument()); - ASSERT_GT(_iter->getApproximateSize(), initialDocSize); - ASSERT_LT(_iter->getApproximateSize(), initialDocSize * 2); + ASSERT_GT(_iter->getApproximateSize(), initialDocSize * 3); + ASSERT_LT(_iter->getApproximateSize(), initialDocSize * 4); } TEST_F(PartitionIteratorTest, MemoryUsageAccountsForReleasedDocuments) { @@ -589,27 +592,20 @@ TEST_F(PartitionIteratorTest, MemoryUsageAccountsForReleasedDocuments) { const auto mock = DocumentSourceMock::createForTest(docs, getExpCtx()); auto accessor = makeDefaultAccessor(mock, boost::none); - size_t initialDocSize = docs[0].getDocument().getCurrentApproximateSize(); + size_t initialDocSize = docs[0].getDocument().getApproximateSize(); - // Pull in the first document, and verify the reported size of the iterator is roughly the size - // of the document. + // Pull in the first document, and verify the reported size of the iterator is roughly double + // the size of the document. ASSERT_DOCUMENT_EQ(*accessor[0], docs[0].getDocument()); - ASSERT_GT(_iter->getApproximateSize(), initialDocSize); - ASSERT_LT(_iter->getApproximateSize(), initialDocSize + 1024); - - // Read the field so that it is coppied into the cache. This will make the document bigger but - // shouldn't affect memory tracking. - auto iterSizeBeforeAccess = _iter->getApproximateSize(); - docs[0].getDocument()["a"]; - ASSERT_GT(docs[0].getDocument().getCurrentApproximateSize(), initialDocSize); - ASSERT_EQ(_iter->getApproximateSize(), iterSizeBeforeAccess); + ASSERT_GT(_iter->getApproximateSize(), initialDocSize * 2); + ASSERT_LT(_iter->getApproximateSize(), initialDocSize * 2 + 1024); // The accessor will have marked the first document as expired, and thus freed on the next call // to advance(). advance(); ASSERT_DOCUMENT_EQ(*_iter->current(), docs[1].getDocument()); - ASSERT_GT(_iter->getApproximateSize(), initialDocSize); - ASSERT_LT(_iter->getApproximateSize(), initialDocSize + 1024); + ASSERT_GT(_iter->getApproximateSize(), initialDocSize * 2); + ASSERT_LT(_iter->getApproximateSize(), initialDocSize * 2 + 1024); } TEST_F(PartitionIteratorTest, ManualPolicy) { diff --git a/src/mongo/db/pipeline/window_function/spillable_cache_test.cpp b/src/mongo/db/pipeline/window_function/spillable_cache_test.cpp index 7599842de57..62ec29b4d9c 100644 --- a/src/mongo/db/pipeline/window_function/spillable_cache_test.cpp +++ b/src/mongo/db/pipeline/window_function/spillable_cache_test.cpp @@ -30,7 +30,7 @@ #include "mongo/platform/basic.h" #include "mongo/db/catalog_raii.h" -#include "mongo/db/concurrency/exception_util.h" +#include "mongo/db/concurrency/write_conflict_exception.h" #include "mongo/db/exec/document_value/document_value_test_util.h" #include "mongo/db/pipeline/aggregation_mongod_context_fixture.h" #include "mongo/db/pipeline/window_function/spillable_cache.h" diff --git a/src/mongo/db/pipeline/window_function/window_bounds.cpp b/src/mongo/db/pipeline/window_function/window_bounds.cpp index 7ede49d0065..dd082135e02 100644 --- a/src/mongo/db/pipeline/window_function/window_bounds.cpp +++ b/src/mongo/db/pipeline/window_function/window_bounds.cpp @@ -63,19 +63,12 @@ WindowBounds::Bound<T> parseBound(ExpressionContext* expCtx, } template <class T> -Value serializeBound(const WindowBounds::Bound<T>& bound, - const SerializationOptions& opts, - const Value& representativeValue) { +Value serializeBound(const WindowBounds::Bound<T>& bound) { return stdx::visit( visit_helper::Overloaded{ - [&](const WindowBounds::Unbounded&) { return Value(WindowBounds::kValUnbounded); }, - [&](const WindowBounds::Current&) { return Value(WindowBounds::kValCurrent); }, - [&](const T& n) { - // If not "unbounded" or "current", n must be a literal constant - // The upper bound must be greater than the lower bound. We override the - // representative value to meet this constraint. - return opts.serializeLiteral(n, representativeValue); - }, + [](const WindowBounds::Unbounded&) { return Value(WindowBounds::kValUnbounded); }, + [](const WindowBounds::Current&) { return Value(WindowBounds::kValCurrent); }, + [](const T& n) { return Value(n); }, }, bound); } @@ -222,31 +215,22 @@ WindowBounds WindowBounds::parse(BSONObj args, uassert(5339902, "Range-based bounds require sortBy a single field", sortBy && sortBy->size() == 1); - const SortPattern::SortPatternPart& part = *sortBy->begin(); - uassert(8947400, - "Range-based bounds require a non-expression sortBy", - part.fieldPath && !part.expression); - uassert(8947401, "Range-based bounds require an ascending sortBy", part.isAscending); return bounds; } } -void WindowBounds::serialize(MutableDocument& args, const SerializationOptions& opts) const { +void WindowBounds::serialize(MutableDocument& args) const { stdx::visit( visit_helper::Overloaded{ [&](const DocumentBased& docBounds) { args[kArgDocuments] = Value{std::vector<Value>{ - serializeBound( - docBounds.lower, opts, /* representative value, if needed */ Value(0LL)), - serializeBound( - docBounds.upper, opts, /* representative value, if needed */ Value(1LL)), + serializeBound(docBounds.lower), + serializeBound(docBounds.upper), }}; }, [&](const RangeBased& rangeBounds) { args[kArgRange] = Value{std::vector<Value>{ - serializeBound( - rangeBounds.lower, opts, /* representative value, if needed */ Value(0LL)), - serializeBound( - rangeBounds.upper, opts, /* representative value, if needed */ Value(1LL)), + serializeBound(rangeBounds.lower), + serializeBound(rangeBounds.upper), }}; if (rangeBounds.unit) { args[kArgUnit] = Value{serializeTimeUnit(*rangeBounds.unit)}; diff --git a/src/mongo/db/pipeline/window_function/window_bounds.h b/src/mongo/db/pipeline/window_function/window_bounds.h index 90d0adf0371..6999f8fcdbc 100644 --- a/src/mongo/db/pipeline/window_function/window_bounds.h +++ b/src/mongo/db/pipeline/window_function/window_bounds.h @@ -121,7 +121,7 @@ struct WindowBounds { const boost::optional<SortPattern>& sortBy, ExpressionContext* expCtx); - void serialize(MutableDocument& args, const SerializationOptions& opts) const; + void serialize(MutableDocument& args) const; }; } // namespace mongo diff --git a/src/mongo/db/pipeline/window_function/window_function_exec_first_last.h b/src/mongo/db/pipeline/window_function/window_function_exec_first_last.h index cf6dd3b2ed6..f64b77c28f8 100644 --- a/src/mongo/db/pipeline/window_function/window_function_exec_first_last.h +++ b/src/mongo/db/pipeline/window_function/window_function_exec_first_last.h @@ -51,28 +51,18 @@ protected: Value getFirst() { auto endpoints = _iter.getEndpoints(_bounds); - if (!endpoints) { + if (!endpoints) return _default; - } const Document doc = *(_iter)[endpoints->first]; - auto result = _input->evaluate(doc, &_input->getExpressionContext()->variables); - if (result.missing()) { - result = _default; - } - return result; + return _input->evaluate(doc, &_input->getExpressionContext()->variables); } Value getLast() { auto endpoints = _iter.getEndpoints(_bounds); - if (!endpoints) { + if (!endpoints) return _default; - } const Document doc = *(_iter)[endpoints->second]; - auto result = _input->evaluate(doc, &_input->getExpressionContext()->variables); - if (result.missing()) { - result = _default; - } - return result; + return _input->evaluate(doc, &_input->getExpressionContext()->variables); } void reset() final {} diff --git a/src/mongo/db/pipeline/window_function/window_function_expression.cpp b/src/mongo/db/pipeline/window_function/window_function_expression.cpp index a46bac72dfa..54f0552b59b 100644 --- a/src/mongo/db/pipeline/window_function/window_function_expression.cpp +++ b/src/mongo/db/pipeline/window_function/window_function_expression.cpp @@ -37,7 +37,6 @@ #include "mongo/db/pipeline/document_source_set_window_fields_gen.h" #include "mongo/db/pipeline/lite_parsed_document_source.h" #include "mongo/db/query/query_feature_flags_gen.h" -#include "mongo/db/stats/counters.h" #include "mongo/db/pipeline/window_function/partition_iterator.h" #include "mongo/db/pipeline/window_function/window_function_exec.h" @@ -153,7 +152,6 @@ intrusive_ptr<Expression> Expression::parse(BSONObj obj, assertLanguageFeatureIsAllowed( opCtx, exprName, allowedWithApi, AllowedWithClientType::kAny); - expCtx->incrementWindowAccumulatorExprCounter(exprName); return parser(obj, sortBy, expCtx); } @@ -189,7 +187,6 @@ void Expression::registerParser( AllowedWithApiStrict allowedWithApi) { invariant(parserMap.find(functionName) == parserMap.end()); ExpressionParserRegistration r{parser, requiredMinVersion, allowedWithApi}; - operatorCountersWindowAccumulatorExpressions.addCounter(functionName); parserMap.emplace(std::move(functionName), std::move(r)); } @@ -310,12 +307,12 @@ boost::intrusive_ptr<Expression> ExpressionFirstLast::parse( template <typename WindowFunctionN, typename AccumulatorNType> Value ExpressionN<WindowFunctionN, AccumulatorNType>::serialize( - const SerializationOptions& opts) const { + boost::optional<ExplainOptions::Verbosity> explain) const { auto acc = buildAccumulatorOnly(); - MutableDocument result(acc->serialize(nExpr, _input, opts)); + MutableDocument result(acc->serialize(nExpr, _input, static_cast<bool>(explain))); MutableDocument windowField; - _bounds.serialize(windowField, opts); + _bounds.serialize(windowField); result[kWindowArg] = windowField.freezeToValue(); return result.freezeToValue(); } diff --git a/src/mongo/db/pipeline/window_function/window_function_expression.h b/src/mongo/db/pipeline/window_function/window_function_expression.h index b14a81d1ae3..0d638ac1350 100644 --- a/src/mongo/db/pipeline/window_function/window_function_expression.h +++ b/src/mongo/db/pipeline/window_function/window_function_expression.h @@ -189,16 +189,17 @@ public: } }; - virtual Value serialize(const SerializationOptions& opts) const { + virtual Value serialize(boost::optional<ExplainOptions::Verbosity> explain) const { MutableDocument args; - args[_accumulatorName] = _input->serialize(opts); + args[_accumulatorName] = _input->serialize(static_cast<bool>(explain)); MutableDocument windowField; - _bounds.serialize(windowField, opts); + _bounds.serialize(windowField); args[kWindowArg] = windowField.freezeToValue(); return args.freezeToValue(); } + protected: ExpressionContext* _expCtx; std::string _accumulatorName; @@ -325,9 +326,9 @@ public: << " is not supported as a removable window function"); } - Value serialize(const SerializationOptions& opts) const final { + Value serialize(boost::optional<ExplainOptions::Verbosity> explain) const final { MutableDocument args; - args.addField(_accumulatorName, Value(_input->serialize(opts))); + args.addField(_accumulatorName, Value(_input->serialize(static_cast<bool>(explain)))); return args.freezeToValue(); } }; @@ -443,7 +444,7 @@ public: << " is not supported with a removable window"); } - Value serialize(const SerializationOptions& opts) const final { + Value serialize(boost::optional<ExplainOptions::Verbosity> explain) const final { MutableDocument args; args.addField(_accumulatorName, Value(Document())); return args.freezeToValue(); @@ -492,17 +493,15 @@ public: << " is not supported with a removable window"); } - Value serialize(const SerializationOptions& opts) const final { + Value serialize(boost::optional<ExplainOptions::Verbosity> explain) const final { MutableDocument subObj; tassert(5433604, "ExpMovingAvg neither N nor alpha was set", _N || _alpha); if (_N) { - subObj[kNArg] = opts.serializeLiteral(_N.get()); + subObj[kNArg] = Value(_N.get()); } else { - // Alpha must be between zero and one (exclusive), so choose a legal representative - // value if applicable. - subObj[kAlphaArg] = opts.serializeLiteral(_alpha.get(), Value(0.1)); + subObj[kAlphaArg] = Value(_alpha.get()); } - subObj[kInputArg] = _input->serialize(opts); + subObj[kInputArg] = _input->serialize(static_cast<bool>(explain)); MutableDocument outerObj; outerObj[kAccName] = subObj.freezeToValue(); return outerObj.freezeToValue(); @@ -529,15 +528,15 @@ public: return _unit; } - Value serialize(const SerializationOptions& opts) const final { + Value serialize(boost::optional<ExplainOptions::Verbosity> explain) const final { MutableDocument result; - result[_accumulatorName][kArgInput] = _input->serialize(opts); + result[_accumulatorName][kArgInput] = _input->serialize(static_cast<bool>(explain)); if (_unit) { result[_accumulatorName][kArgUnit] = Value(serializeTimeUnit(*_unit)); } MutableDocument windowField; - _bounds.serialize(windowField, opts); + _bounds.serialize(windowField); result[kWindowArg] = windowField.freezeToValue(); return result.freezeToValue(); } @@ -556,7 +555,7 @@ protected: case TimeUnit::year: case TimeUnit::quarter: case TimeUnit::month: - uasserted(5490710, "unit must be 'week' or smaller"); + uasserted(5490704, "unit must be 'week' or smaller"); // Only these time units are allowed. case TimeUnit::week: case TimeUnit::day: @@ -804,16 +803,16 @@ public: } boost::intrusive_ptr<AccumulatorState> buildAccumulatorOnly() const final { - MONGO_UNREACHABLE_TASSERT(5490704); + MONGO_UNREACHABLE_TASSERT(5490701); } std::unique_ptr<WindowFunctionState> buildRemovable() const final { - MONGO_UNREACHABLE_TASSERT(5490705); + MONGO_UNREACHABLE_TASSERT(5490702); } - Value serialize(const SerializationOptions& opts) const final { + Value serialize(boost::optional<ExplainOptions::Verbosity> explain) const final { MutableDocument args; - args.addField(_accumulatorName, Value(_input->serialize(opts))); + args.addField(_accumulatorName, Value(_input->serialize(static_cast<bool>(explain)))); return args.freezeToValue(); } }; @@ -852,11 +851,11 @@ public: } boost::intrusive_ptr<AccumulatorState> buildAccumulatorOnly() const final { - MONGO_UNREACHABLE_TASSERT(5490706); + MONGO_UNREACHABLE_TASSERT(5490701); } std::unique_ptr<WindowFunctionState> buildRemovable() const final { - MONGO_UNREACHABLE_TASSERT(5490707); + MONGO_UNREACHABLE_TASSERT(5490702); } }; @@ -874,11 +873,11 @@ public: } boost::intrusive_ptr<AccumulatorState> buildAccumulatorOnly() const final { - MONGO_UNREACHABLE_TASSERT(5490708); + MONGO_UNREACHABLE_TASSERT(5490701); } std::unique_ptr<WindowFunctionState> buildRemovable() const final { - MONGO_UNREACHABLE_TASSERT(5490709); + MONGO_UNREACHABLE_TASSERT(5490702); } }; @@ -905,7 +904,7 @@ public: nExpr(std::move(nExpr)), sortPattern(std::move(sortPattern)) {} - Value serialize(const SerializationOptions& opts) const final; + Value serialize(boost::optional<ExplainOptions::Verbosity> explain) const final; boost::intrusive_ptr<AccumulatorState> buildAccumulatorOnly() const final; diff --git a/src/mongo/db/pipeline/window_function/window_function_push.h b/src/mongo/db/pipeline/window_function/window_function_push.h index 932bbcb29dd..4ed5b083996 100644 --- a/src/mongo/db/pipeline/window_function/window_function_push.h +++ b/src/mongo/db/pipeline/window_function/window_function_push.h @@ -48,9 +48,6 @@ public: } void add(Value value) override { - if (value.missing()) { - return; - } _memUsageBytes += value.getApproximateSize(); _values.push_back(std::move(value)); } @@ -59,10 +56,6 @@ public: * This should only remove the first/lowest element in the window. */ void remove(Value value) override { - if (value.missing()) { - return; - } - tassert(5423801, "Can't remove from an empty WindowFunctionPush", _values.size() != 0); auto valToRemove = _values.front(); tassert( diff --git a/src/mongo/db/pipeline/window_function/window_function_shift.cpp b/src/mongo/db/pipeline/window_function/window_function_shift.cpp index 0ae15c4e3f0..c74424e74e7 100644 --- a/src/mongo/db/pipeline/window_function/window_function_shift.cpp +++ b/src/mongo/db/pipeline/window_function/window_function_shift.cpp @@ -119,12 +119,12 @@ boost::intrusive_ptr<Expression> ExpressionShift::parse(BSONObj obj, return shiftExpr; } -Value ExpressionShift::serialize(const SerializationOptions& opts) const { +Value ExpressionShift::serialize(boost::optional<ExplainOptions::Verbosity> explain) const { MutableDocument args; - args.addField(kByArg, opts.serializeLiteral(_offset)); - args.addField(kOutputArg, _input->serialize(opts)); - args.addField(kDefaultArg, - opts.serializeLiteral(_defaultVal.get_value_or(mongo::Value(BSONNULL)))); + args.addField(kByArg, Value(_offset)); + args.addField(kOutputArg, _input->serialize(static_cast<bool>(explain))); + args.addField(kDefaultArg, _defaultVal.get_value_or(mongo::Value(BSONNULL))); + MutableDocument windowFun; windowFun.addField(_accumulatorName, args.freezeToValue()); return windowFun.freezeToValue(); diff --git a/src/mongo/db/pipeline/window_function/window_function_shift.h b/src/mongo/db/pipeline/window_function/window_function_shift.h index 649e90ce7e4..99a45dd3f84 100644 --- a/src/mongo/db/pipeline/window_function/window_function_shift.h +++ b/src/mongo/db/pipeline/window_function/window_function_shift.h @@ -68,7 +68,7 @@ public: MONGO_UNREACHABLE_TASSERT(5424302); } - Value serialize(const SerializationOptions& opts) const final; + Value serialize(boost::optional<ExplainOptions::Verbosity> explain) const final; private: static boost::intrusive_ptr<Expression> parseShiftArgs(BSONObj obj, diff --git a/src/mongo/db/pipeline/window_function/window_function_stddev.h b/src/mongo/db/pipeline/window_function/window_function_stddev.h index 14aa1fa1260..5b4aa45b075 100644 --- a/src/mongo/db/pipeline/window_function/window_function_stddev.h +++ b/src/mongo/db/pipeline/window_function/window_function_stddev.h @@ -63,7 +63,7 @@ public: if (_nonfiniteValueCount > 0) return Value(BSONNULL); const long long adjustedCount = _isSamp ? _count - 1 : _count; - if (adjustedCount <= 0) + if (adjustedCount == 0) return getDefault(); double squaredDifferences = _m2->getValue(false).coerceToDouble(); if (squaredDifferences < 0 || (!_isSamp && _count == 1)) { diff --git a/src/mongo/db/pipeline/window_function/window_function_top_bottom_n.h b/src/mongo/db/pipeline/window_function/window_function_top_bottom_n.h index 64d3319efb9..fbf77c59fa2 100644 --- a/src/mongo/db/pipeline/window_function/window_function_top_bottom_n.h +++ b/src/mongo/db/pipeline/window_function/window_function_top_bottom_n.h @@ -56,17 +56,17 @@ public: explicit WindowFunctionTopBottomN(ExpressionContext* const expCtx, SortPattern sp, long long n) : WindowFunctionState(expCtx), _acc(expCtx, std::move(sp), true) { _acc.startNewGroup(Value(n)); - updateMemUsage(); + _memUsageBytes = sizeof(*this); } void add(Value value) final { _acc.process(value, false); - updateMemUsage(); + _memUsageBytes = _acc.getMemUsage(); } void remove(Value value) final { _acc.remove(value); - updateMemUsage(); + _memUsageBytes = _acc.getMemUsage(); } Value getValue() const final { @@ -75,14 +75,10 @@ public: void reset() final { _acc.reset(); - updateMemUsage(); + _memUsageBytes = _acc.getMemUsage(); } private: - void updateMemUsage() { - _memUsageBytes = sizeof(*this) + _acc.getMemUsage(); - } - AccumulatorTopBottomN<sense, single> _acc; }; |