diff options
9 files changed, 364 insertions, 62 deletions
diff --git a/jstests/aggregation/sources/setWindowFields/derivative.js b/jstests/aggregation/sources/setWindowFields/derivative.js index d5d50caf158..fdaf2ad1ecd 100644 --- a/jstests/aggregation/sources/setWindowFields/derivative.js +++ b/jstests/aggregation/sources/setWindowFields/derivative.js @@ -126,6 +126,38 @@ assert.docEq(result, [ {time: 7, y: 118, dy: +3 / 3}, ]); +// Example with range-based bounds. +coll.drop(); +assert.commandWorked(coll.insert([ + {time: 0, y: 10}, + {time: 10, y: 12}, + {time: 11, y: 15}, + {time: 12, y: 19}, + {time: 13, y: 24}, + {time: 20, y: 30}, +])); +result = coll.aggregate([ + { + $setWindowFields: { + sortBy: {time: 1}, + output: { + dy: {$derivative: {input: "$y"}, window: {range: [-10, 0]}}, + } + } + }, + {$unset: "_id"}, + {$sort: {time: 1}}, + ]) + .toArray(); +assert.docEq(result, [ + {time: 0, y: 10, dy: null}, + {time: 10, y: 12, dy: (12 - 10) / (10 - 0)}, + {time: 11, y: 15, dy: (15 - 12) / (11 - 10)}, + {time: 12, y: 19, dy: (19 - 12) / (12 - 10)}, + {time: 13, y: 24, dy: (24 - 12) / (13 - 10)}, + {time: 20, y: 30, dy: (30 - 12) / (20 - 10)}, +]); + // 'outputUnit' only supports 'week' and smaller. coll.drop(); function explainUnit(outputUnit) { @@ -255,4 +287,39 @@ assert.sameMembers(result, [ {time: ISODate("2020-01-01T00:02:00.000Z"), y: 6, dy: +2 / (60 * 1000)}, {time: ISODate("2020-01-01T00:03:00.000Z"), y: 5, dy: -1 / (60 * 1000)}, ]); + +// Example with time-based bounds. +coll.drop(); +assert.commandWorked(coll.insert([ + {time: ISODate("2020-01-01T00:00:00"), y: 10}, + {time: ISODate("2020-01-01T00:00:10"), y: 12}, + {time: ISODate("2020-01-01T00:00:11"), y: 15}, + {time: ISODate("2020-01-01T00:00:12"), y: 19}, + {time: ISODate("2020-01-01T00:00:13"), y: 24}, + {time: ISODate("2020-01-01T00:00:20"), y: 30}, +])); +result = coll.aggregate([ + { + $setWindowFields: { + sortBy: {time: 1}, + output: { + dy: { + $derivative: {input: "$y", outputUnit: 'second'}, + window: {range: [-10, 0], unit: 'second'} + }, + } + } + }, + {$unset: "_id"}, + {$sort: {time: 1}}, + ]) + .toArray(); +assert.docEq(result, [ + {time: ISODate("2020-01-01T00:00:00.00Z"), y: 10, dy: null}, + {time: ISODate("2020-01-01T00:00:10.00Z"), y: 12, dy: (12 - 10) / (10 - 0)}, + {time: ISODate("2020-01-01T00:00:11.00Z"), y: 15, dy: (15 - 12) / (11 - 10)}, + {time: ISODate("2020-01-01T00:00:12.00Z"), y: 19, dy: (19 - 12) / (12 - 10)}, + {time: ISODate("2020-01-01T00:00:13.00Z"), y: 24, dy: (24 - 12) / (13 - 10)}, + {time: ISODate("2020-01-01T00:00:20.00Z"), y: 30, dy: (30 - 12) / (20 - 10)}, +]); })(); diff --git a/jstests/aggregation/sources/setWindowFields/parse.js b/jstests/aggregation/sources/setWindowFields/parse.js index 9adaa172464..f1e84d215a4 100644 --- a/jstests/aggregation/sources/setWindowFields/parse.js +++ b/jstests/aggregation/sources/setWindowFields/parse.js @@ -19,8 +19,6 @@ if (!featureEnabled) { const coll = db.setWindowFields_parse; coll.drop(); -assert.commandWorked(coll.insert({ts: 0})); - function run(stage, extraCommandArgs = {}) { return coll.runCommand( Object.merge({aggregate: coll.getName(), pipeline: [stage], cursor: {}}, extraCommandArgs)); @@ -87,15 +85,15 @@ assert.commandWorked( runWindowFunction({$sum: "$a", window: {range: [NumberDecimal('1.42'), NumberLong(5)]}})); // Time-based bounds: -assert.commandFailedWithCode( - runWindowFunction({"$sum": "$a", window: {range: [-3, 'unbounded'], unit: 'hour'}}), 5397902); +assert.commandWorked( + runWindowFunction({"$sum": "$a", window: {range: [-3, 'unbounded'], unit: 'hour'}})); // Numeric bounds can be a constant expression: let expr = {$add: [2, 2]}; assert.commandWorked(runWindowFunction({"$sum": "$a", window: {documents: [expr, expr]}})); assert.commandWorked(runWindowFunction({"$sum": "$a", window: {range: [expr, expr]}})); -assert.commandFailedWithCode( - runWindowFunction({"$sum": "$a", window: {range: [expr, expr], unit: 'hour'}}), 5397902); +assert.commandWorked( + runWindowFunction({"$sum": "$a", window: {range: [expr, expr], unit: 'hour'}})); // But 'current' and 'unbounded' are not expressions: they're more like keywords. assert.commandFailedWithCode( runWindowFunction({"$sum": "$a", window: {documents: [{$const: 'current'}, 3]}}), diff --git a/jstests/aggregation/sources/setWindowFields/range.js b/jstests/aggregation/sources/setWindowFields/range.js index 2d69555605d..7826662b28a 100644 --- a/jstests/aggregation/sources/setWindowFields/range.js +++ b/jstests/aggregation/sources/setWindowFields/range.js @@ -205,15 +205,13 @@ coll.insert([ {x: ''}, {x: {}}, ]); -assert.throws(() => { - run([range(+999, +999)]); -}, [], 'Invalid range: Expected the sortBy field to be a number'); -assert.throws(() => { - run([range(-999, +999)]); -}, [], 'Invalid range: Expected the sortBy field to be a number'); -assert.throws(() => { - run([range('unbounded', 'unbounded')]); -}, [], 'Invalid range: Expected the sortBy field to be a number'); +let error; +error = assert.throws(() => run([range(+999, +999)])); +assert.includes(error.message, 'Invalid range: Expected the sortBy field to be a number'); +error = assert.throws(() => run([range(-999, +999)])); +assert.includes(error.message, 'Invalid range: Expected the sortBy field to be a number'); +error = assert.throws(() => run([range('unbounded', 'unbounded')])); +assert.includes(error.message, 'Invalid range: Expected the sortBy field to be a number'); // Another case, involving ties and expiration. coll.drop(); diff --git a/jstests/aggregation/sources/setWindowFields/time.js b/jstests/aggregation/sources/setWindowFields/time.js new file mode 100644 index 00000000000..8f9ce927048 --- /dev/null +++ b/jstests/aggregation/sources/setWindowFields/time.js @@ -0,0 +1,219 @@ +/** + * Test time-based window bounds. + */ +(function() { +"use strict"; + +load("jstests/aggregation/extras/window_function_helpers.js"); + +const featureEnabled = + assert.commandWorked(db.adminCommand({getParameter: 1, featureFlagWindowFunctions: 1})) + .featureFlagWindowFunctions.value; +if (!featureEnabled) { + jsTestLog("Skipping test because the window function feature flag is disabled"); + return; +} + +const coll = db.setWindowFields_time; +coll.drop(); + +// Create an ISODate that occurs 'seconds' seconds after a fixed date. +function makeDate(seconds) { + let result = ISODate('2021-01-01T00:00:00Z'); + result.setUTCMilliseconds(seconds * 1000); + return result; +} +function makeDates(arrayOfSeconds) { + return arrayOfSeconds.map(seconds => makeDate(seconds)); +} + +assert.commandWorked(coll.insert([ + {x: makeDate(0)}, + {x: makeDate(1)}, + {x: makeDate(1.5)}, + {x: makeDate(2)}, + {x: makeDate(3)}, + {x: makeDate(42)}, + {x: makeDate(42)}, + {x: makeDate(43)}, +])); + +// Make a setWindowFields stage with the given bounds. +function range(lower, upper, unit = 'second') { + return { + $setWindowFields: { + partitionBy: "$partition", + sortBy: {x: 1}, + output: { + y: {$push: "$x", window: {range: [lower, upper], unit}}, + } + } + }; +} + +// Run the pipeline, and unset _id. +function run(pipeline) { + return coll + .aggregate([ + ...pipeline, + {$unset: '_id'}, + ]) + .toArray(); +} + +// The documents are not evenly spaced, so the window varies in size. +assert.sameMembers(run([range(-1, 0)]), [ + {x: makeDate(0), y: makeDates([0])}, + {x: makeDate(1), y: makeDates([0, 1])}, + + // Time-based bounds work like $dateAdd, which rounds down! + {x: makeDate(1.5), y: makeDates([0, 1])}, + + {x: makeDate(2), y: makeDates([1, 1.5, 2])}, + {x: makeDate(3), y: makeDates([2, 3])}, + // '0' means the current document and those that tie with it. + {x: makeDate(42), y: makeDates([42, 42])}, + {x: makeDate(42), y: makeDates([42, 42])}, + {x: makeDate(43), y: makeDates([42, 42, 43])}, +]); + +// Bounds can be specified with different units. +assert.sameMembers(run([range(-1000, 0, 'millisecond')]), [ + {x: makeDate(0), y: makeDates([0])}, + {x: makeDate(1), y: makeDates([0, 1])}, + + // Time-based bounds work like $dateAdd, which rounds down! + {x: makeDate(1.5), y: makeDates([0, 1])}, + + {x: makeDate(2), y: makeDates([1, 1.5, 2])}, + {x: makeDate(3), y: makeDates([2, 3])}, + // '0' means the current document and those that tie with it. + {x: makeDate(42), y: makeDates([42, 42])}, + {x: makeDate(42), y: makeDates([42, 42])}, + {x: makeDate(43), y: makeDates([42, 42, 43])}, +]); + +// Fractional units are not allowed. +let error; +error = assert.throws(() => run([range(-1.5, 0, 'second')])); +assert.commandFailedWithCode(error, ErrorCodes.FailedToParse); +assert.includes(error.message, 'range-based bounds must be an integer'); + +// One or both endpoints can be unbounded. +assert.sameMembers(run([range('unbounded', 0)]), [ + {x: makeDate(0), y: makeDates([0])}, + {x: makeDate(1), y: makeDates([0, 1])}, + + // Since $dateAdd rounds down, the upper bound is makeDate(1). + {x: makeDate(1.5), y: makeDates([0, 1])}, + + {x: makeDate(2), y: makeDates([0, 1, 1.5, 2])}, + {x: makeDate(3), y: makeDates([0, 1, 1.5, 2, 3])}, + // '0' means current document and those that tie with it. + {x: makeDate(42), y: makeDates([0, 1, 1.5, 2, 3, 42, 42])}, + {x: makeDate(42), y: makeDates([0, 1, 1.5, 2, 3, 42, 42])}, + {x: makeDate(43), y: makeDates([0, 1, 1.5, 2, 3, 42, 42, 43])}, +]); +assert.sameMembers(run([range(0, 'unbounded')]), [ + {x: makeDate(0), y: makeDates([0, 1, 1.5, 2, 3, 42, 42, 43])}, + {x: makeDate(1), y: makeDates([1, 1.5, 2, 3, 42, 42, 43])}, + + // Sice $dateAdd rounds down,t he lower bound is makeDate(1). + {x: makeDate(1.5), y: makeDates([1, 1.5, 2, 3, 42, 42, 43])}, + + {x: makeDate(2), y: makeDates([2, 3, 42, 42, 43])}, + {x: makeDate(3), y: makeDates([3, 42, 42, 43])}, + // '0' means current document and those that tie with it. + {x: makeDate(42), y: makeDates([42, 42, 43])}, + {x: makeDate(42), y: makeDates([42, 42, 43])}, + {x: makeDate(43), y: makeDates([43])}, +]); +assert.sameMembers(run([range('unbounded', 'unbounded')]), [ + {x: makeDate(0), y: makeDates([0, 1, 1.5, 2, 3, 42, 42, 43])}, + {x: makeDate(1), y: makeDates([0, 1, 1.5, 2, 3, 42, 42, 43])}, + {x: makeDate(1.5), y: makeDates([0, 1, 1.5, 2, 3, 42, 42, 43])}, + {x: makeDate(2), y: makeDates([0, 1, 1.5, 2, 3, 42, 42, 43])}, + {x: makeDate(3), y: makeDates([0, 1, 1.5, 2, 3, 42, 42, 43])}, + {x: makeDate(42), y: makeDates([0, 1, 1.5, 2, 3, 42, 42, 43])}, + {x: makeDate(42), y: makeDates([0, 1, 1.5, 2, 3, 42, 42, 43])}, + {x: makeDate(43), y: makeDates([0, 1, 1.5, 2, 3, 42, 42, 43])}, +]); + +// Unlike '0', 'current' always means the current document. +assert.sameMembers(run([range('current', 'current'), {$match: {x: makeDate(42)}}]), [ + {x: makeDate(42), y: makeDates([42])}, + {x: makeDate(42), y: makeDates([42])}, +]); +assert.sameMembers(run([range('current', +1), {$match: {x: makeDate(42)}}]), [ + {x: makeDate(42), y: makeDates([42, 42, 43])}, + {x: makeDate(42), y: makeDates([42, 43])}, +]); +assert.sameMembers(run([range(-40, 'current'), {$match: {x: makeDate(42)}}]), [ + {x: makeDate(42), y: makeDates([2, 3, 42])}, + {x: makeDate(42), y: makeDates([2, 3, 42, 42])}, +]); + +// The window can be empty even if it's unbounded on one side. +assert.sameMembers(run([range('unbounded', -40)]), [ + {x: makeDate(0), y: makeDates([])}, + {x: makeDate(1), y: makeDates([])}, + {x: makeDate(1.5), y: makeDates([])}, + {x: makeDate(2), y: makeDates([])}, + {x: makeDate(3), y: makeDates([])}, + {x: makeDate(42), y: makeDates([0, 1, 1.5, 2])}, + {x: makeDate(42), y: makeDates([0, 1, 1.5, 2])}, + {x: makeDate(43), y: makeDates([0, 1, 1.5, 2, 3])}, +]); +assert.sameMembers(run([range(+40, 'unbounded')]), [ + {x: makeDate(0), y: makeDates([42, 42, 43])}, + {x: makeDate(1), y: makeDates([42, 42, 43])}, + {x: makeDate(1.5), y: makeDates([42, 42, 43])}, + {x: makeDate(2), y: makeDates([42, 42, 43])}, + {x: makeDate(3), y: makeDates([43])}, + {x: makeDate(42), y: makeDates([])}, + {x: makeDate(42), y: makeDates([])}, + {x: makeDate(43), y: makeDates([])}, +]); + +// Time-based windows reset between partitions. +assert.commandWorked(coll.updateMany({}, {$set: {partition: "A"}})); +assert.commandWorked(coll.insert([ + {partition: "B", x: makeDate(43)}, + {partition: "B", x: makeDate(44)}, + {partition: "B", x: makeDate(45)}, +])); +assert.sameMembers(run([range(-5, 0)]), [ + {partition: "A", x: makeDate(0), y: makeDates([0])}, + {partition: "A", x: makeDate(1), y: makeDates([0, 1])}, + {partition: "A", x: makeDate(1.5), y: makeDates([0, 1])}, + {partition: "A", x: makeDate(2), y: makeDates([0, 1, 1.5, 2])}, + {partition: "A", x: makeDate(3), y: makeDates([0, 1, 1.5, 2, 3])}, + {partition: "A", x: makeDate(42), y: makeDates([42, 42])}, + {partition: "A", x: makeDate(42), y: makeDates([42, 42])}, + {partition: "A", x: makeDate(43), y: makeDates([42, 42, 43])}, + + {partition: "B", x: makeDate(43), y: makeDates([43])}, + {partition: "B", x: makeDate(44), y: makeDates([43, 44])}, + {partition: "B", x: makeDate(45), y: makeDates([43, 44, 45])}, +]); +assert.commandWorked(coll.deleteMany({partition: "B"})); +assert.commandWorked(coll.updateMany({}, [{$unset: 'partition'}])); + +// If the sortBy is a non-Date, we throw an error. +assert.commandWorked(coll.insert([ + {}, +])); +error = assert.throws(() => { + run([range('unbounded', 'unbounded')]); +}); +assert.commandFailedWithCode(error, 5429513); + +assert.commandWorked(coll.remove({x: {$exists: false}})); +assert.commandWorked(coll.insert([ + {x: 0}, +])); +error = assert.throws(() => { + run([range('unbounded', 'unbounded')]); +}); +assert.commandFailedWithCode(error, 5429513); +})(); diff --git a/src/mongo/db/pipeline/window_function/partition_iterator.cpp b/src/mongo/db/pipeline/window_function/partition_iterator.cpp index 32967618454..59d82a10636 100644 --- a/src/mongo/db/pipeline/window_function/partition_iterator.cpp +++ b/src/mongo/db/pipeline/window_function/partition_iterator.cpp @@ -223,27 +223,51 @@ Value decimalAdd(const Value& left, const Value& right) { // user can't observe the type. return Value(left.coerceToDecimal().add(right.coerceToDecimal())); } + + } // namespace optional<std::pair<int, int>> PartitionIterator::getEndpointsRangeBased( - const WindowBounds& bounds, const optional<std::pair<int, int>>& hint) { + const WindowBounds::RangeBased& range, const optional<std::pair<int, int>>& hint) { + tassert(5429404, "Missing _sortExpr with range-based bounds", _sortExpr != boost::none); - // TODO SERVER-54295: time-based bounds - uassert(5429402, - "Time-based bounds not supported yet", - stdx::holds_alternative<WindowBounds::RangeBased>(bounds.bounds)); - auto range = stdx::get<WindowBounds::RangeBased>(bounds.bounds); auto lessThan = _expCtx->getValueComparator().getLessThan(); Value base = (*_sortExpr)->evaluate(*(*this)[0], &_expCtx->variables); - uassert(5429413, + if (range.unit) { + uassert( + 5429513, + str::stream() << "Invalid range: Expected the sortBy field to be a Date, but it was " + << base.getType(), + base.getType() == BSONType::Date); + } else { + uassert( + 5429413, "Invalid range: For windows that involve date or time ranges, a unit must be provided.", base.getType() != BSONType::Date); - uassert(5429414, + uassert( + 5429414, str::stream() << "Invalid range: Expected the sortBy field to be a number, but it was " << base.getType(), base.numeric()); + } + auto add = [&](const Value& base, const Value& delta) -> Value { + if (range.unit) { + return Value{ + dateAdd(base.coerceToDate(), *range.unit, delta.coerceToInt(), TimeZone())}; + } else { + tassert(5429406, "Range-based bounds are specified as a number", delta.numeric()); + return decimalAdd(base, delta); + } + }; + auto hasExpectedType = [&](const Value& v) -> bool { + if (range.unit) { + return v.getType() == BSONType::Date; + } else { + return v.numeric(); + } + }; // 'lower' is the smallest offset in the partition that's within the lower bound of the window. optional<int> lower = stdx::visit( @@ -269,13 +293,13 @@ optional<std::pair<int, int>> PartitionIterator::getEndpointsRangeBased( return boost::none; } Value v = (*_sortExpr)->evaluate(*doc, &_expCtx->variables); - if (v.numeric()) { + if (hasExpectedType(v)) { return i; } } }, [&](const Value& delta) -> optional<int> { - Value threshold = decimalAdd(base, delta); + Value threshold = add(base, delta); // Start from the beginning, or the hint, whichever is higher. // Note that the hint may no longer be a valid offset, if some documents were @@ -324,7 +348,7 @@ optional<std::pair<int, int>> PartitionIterator::getEndpointsRangeBased( boost::optional<Document> doc; for (int i = start; (doc = (*this)[i]); ++i) { Value v = (*_sortExpr)->evaluate(*doc, &_expCtx->variables); - if (!v.numeric()) { + if (!hasExpectedType(v)) { // The previously scanned doc is the rightmost numeric one. Since we start // from '0', 'hint', or 'lower', which are all numeric, we should never hit // this case on the first iteration. @@ -339,8 +363,7 @@ optional<std::pair<int, int>> PartitionIterator::getEndpointsRangeBased( }, [&](const Value& delta) -> optional<int> { // Pull in documents until the sortBy value crosses 'base + delta'. - tassert(5429406, "Range-based bounds are specified as a number", delta.numeric()); - Value threshold = decimalAdd(base, delta); + Value threshold = add(base, delta); // If there's no hint, start scanning from the lower bound. // If there is a hint, start from whichever is greater: lower bound or hint. @@ -385,9 +408,8 @@ optional<std::pair<int, int>> PartitionIterator::getEndpointsRangeBased( } optional<std::pair<int, int>> PartitionIterator::getEndpointsDocumentBased( - const WindowBounds& bounds, const optional<std::pair<int, int>>& hint = boost::none) { - tassert(5423301, "getEndpoints assumes there is a current document", (*this)[0] != boost::none); - auto docBounds = stdx::get<WindowBounds::DocumentBased>(bounds.bounds); + const WindowBounds::DocumentBased& docBounds, + const optional<std::pair<int, int>>& hint = boost::none) { optional<int> lowerBound = numericBound(docBounds.lower); optional<int> upperBound = numericBound(docBounds.upper); tassert(5423302, @@ -430,10 +452,19 @@ optional<std::pair<int, int>> PartitionIterator::getEndpointsDocumentBased( optional<std::pair<int, int>> PartitionIterator::getEndpoints( const WindowBounds& bounds, const optional<std::pair<int, int>>& hint = boost::none) { - if (!stdx::holds_alternative<WindowBounds::DocumentBased>(bounds.bounds)) { - return getEndpointsRangeBased(bounds, hint); - } - return getEndpointsDocumentBased(bounds, hint); + + tassert(5423301, "getEndpoints assumes there is a current document", (*this)[0] != boost::none); + + return stdx::visit( + visit_helper::Overloaded{ + [&](const WindowBounds::DocumentBased docBounds) { + return getEndpointsDocumentBased(docBounds, hint); + }, + [&](const WindowBounds::RangeBased rangeBounds) { + return getEndpointsRangeBased(rangeBounds, hint); + }, + }, + bounds.bounds); } void PartitionIterator::getNextDocument() { diff --git a/src/mongo/db/pipeline/window_function/partition_iterator.h b/src/mongo/db/pipeline/window_function/partition_iterator.h index 1b199685238..4e14458c4b5 100644 --- a/src/mongo/db/pipeline/window_function/partition_iterator.h +++ b/src/mongo/db/pipeline/window_function/partition_iterator.h @@ -225,9 +225,10 @@ private: // Internal helpers for 'getEndpoints()'. boost::optional<std::pair<int, int>> getEndpointsRangeBased( - const WindowBounds& bounds, const boost::optional<std::pair<int, int>>& hint); + const WindowBounds::RangeBased& bounds, const boost::optional<std::pair<int, int>>& hint); boost::optional<std::pair<int, int>> getEndpointsDocumentBased( - const WindowBounds& bounds, const boost::optional<std::pair<int, int>>& hint); + const WindowBounds::DocumentBased& bounds, + const boost::optional<std::pair<int, int>>& hint); ExpressionContext* _expCtx; DocumentSource* _source; diff --git a/src/mongo/db/pipeline/window_function/window_bounds.cpp b/src/mongo/db/pipeline/window_function/window_bounds.cpp index babb0394673..dd082135e02 100644 --- a/src/mongo/db/pipeline/window_function/window_bounds.cpp +++ b/src/mongo/db/pipeline/window_function/window_bounds.cpp @@ -183,17 +183,18 @@ WindowBounds WindowBounds::parse(BSONObj args, str::stream() << "'" << kArgUnit << "' must be a string", unit.type() == BSONType::String); - auto parseInt = [](Value v) -> int { + auto parseInt = [](Value v) -> Value { uassert(ErrorCodes::FailedToParse, str::stream() << "With '" << kArgUnit << "', range-based bounds must be an integer", v.integral()); - return v.coerceToInt(); + return v; }; - auto lower = parseBound<int>(expCtx, lowerElem, parseInt); - auto upper = parseBound<int>(expCtx, upperElem, parseInt); + // Syntactically, time-based bounds can't be fractional. So parse as int. + auto lower = parseBound<Value>(expCtx, lowerElem, parseInt); + auto upper = parseBound<Value>(expCtx, upperElem, parseInt); checkBoundsForward(lower, upper); - bounds = WindowBounds{TimeBased{lower, upper, parseTimeUnit(unit.str())}}; + bounds = WindowBounds{RangeBased{lower, upper, parseTimeUnit(unit.str())}}; } else { // Parse range-based bounds. uassert(ErrorCodes::FailedToParse, @@ -231,13 +232,9 @@ void WindowBounds::serialize(MutableDocument& args) const { serializeBound(rangeBounds.lower), serializeBound(rangeBounds.upper), }}; - }, - [&](const TimeBased& timeBounds) { - args[kArgRange] = Value{std::vector<Value>{ - serializeBound(timeBounds.lower), - serializeBound(timeBounds.upper), - }}; - args[kArgUnit] = Value{serializeTimeUnit(timeBounds.unit)}; + if (rangeBounds.unit) { + args[kArgUnit] = Value{serializeTimeUnit(*rangeBounds.unit)}; + } }, }, bounds); diff --git a/src/mongo/db/pipeline/window_function/window_bounds.h b/src/mongo/db/pipeline/window_function/window_bounds.h index 523ac095975..ad6a966ecfa 100644 --- a/src/mongo/db/pipeline/window_function/window_bounds.h +++ b/src/mongo/db/pipeline/window_function/window_bounds.h @@ -75,19 +75,12 @@ struct WindowBounds { Bound<int> upper; }; struct RangeBased { - // Range-based bounds can be any numeric type: int, double, Decimal, etc. Bound<Value> lower; Bound<Value> upper; - }; - struct TimeBased { - // Although time-based bounds look similar to range-based, they are more restricted: - // the numbers must be integers, like $dateAdd / $dateDiff. - Bound<int> lower; - Bound<int> upper; - TimeUnit unit; + boost::optional<TimeUnit> unit; }; - stdx::variant<DocumentBased, RangeBased, TimeBased> bounds; + stdx::variant<DocumentBased, RangeBased> bounds; static WindowBounds defaultBounds() { return WindowBounds{DocumentBased{Unbounded{}, Unbounded{}}}; diff --git a/src/mongo/db/pipeline/window_function/window_function_exec.cpp b/src/mongo/db/pipeline/window_function/window_function_exec.cpp index ab154b720d4..0ce32211238 100644 --- a/src/mongo/db/pipeline/window_function/window_function_exec.cpp +++ b/src/mongo/db/pipeline/window_function/window_function_exec.cpp @@ -74,6 +74,7 @@ std::unique_ptr<WindowFunctionExec> translateDerivative( iter, deriv.input(), sortExpr, deriv.bounds(), deriv.outputUnit()); } + } // namespace std::unique_ptr<WindowFunctionExec> WindowFunctionExec::create( @@ -106,7 +107,6 @@ std::unique_ptr<WindowFunctionExec> WindowFunctionExec::create( part.fieldPath != boost::none && !part.expression); auto sortByExpr = ExpressionFieldPath::createPathFromString( expCtx, part.fieldPath->fullPath(), expCtx->variablesParseState); - if (stdx::holds_alternative<WindowBounds::Unbounded>(rangeBounds.lower)) { return std::make_unique<WindowFunctionExecNonRemovableRange>( iter, @@ -123,9 +123,7 @@ std::unique_ptr<WindowFunctionExec> WindowFunctionExec::create( bounds); } }, - [&](const WindowBounds::TimeBased& timeBounds) -> std::unique_ptr<WindowFunctionExec> { - uasserted(5397902, "Time based windows are not currently supported"); - }}, + }, bounds.bounds); } |