SERVER-69952 Prevent timeseries from using _id when out of range

4 files changed, 84 insertions, 23 deletions

diff --git a/etc/backports_required_for_multiversion_tests.yml b/etc/backports_required_for_multiversion_tests.yml
index 4708be03bcf..348d1c6e923 100644
--- a/etc/backports_required_for_multiversion_tests.yml
+++ b/etc/backports_required_for_multiversion_tests.yml
@@ -102,6 +102,8 @@ last-continuous:
     ticket: SERVER-73125
   - test_file: jstests/sharding/shard_keys_with_dollar_sign.js
     ticket: SERVER-74124
+  - test_file: jstests/core/timeseries/timeseries_filter_extended_range.js
+    ticket: SERVER-69952
   suites:
     change_streams_multiversion_passthrough: null
     change_streams_sharded_collections_multiversion_passthrough: null
@@ -299,6 +301,8 @@ last-lts:
     ticket: SERVER-73125
   - test_file: jstests/sharding/shard_keys_with_dollar_sign.js
     ticket: SERVER-74124
+  - test_file: jstests/core/timeseries/timeseries_filter_extended_range.js
+    ticket: SERVER-69952
   suites:
     change_streams_multiversion_passthrough: null
     change_streams_sharded_collections_multiversion_passthrough: null
diff --git a/jstests/core/timeseries/timeseries_filter_extended_range.js b/jstests/core/timeseries/timeseries_filter_extended_range.js
index 5285a2ea47a..25b075afcec 100644
--- a/jstests/core/timeseries/timeseries_filter_extended_range.js
+++ b/jstests/core/timeseries/timeseries_filter_extended_range.js
@@ -3,12 +3,17 @@
  *  [1970-01-01 00:00:00 UTC - 2038-01-29 03:13:07 UTC].
  *
  * @tags: [
+ *   # Refusing to run a test that issues an aggregation command with explain because it may
+ *   # return incomplete results if interrupted by a stepdown.
+ *   does_not_support_stepdowns,
  *   # We need a timeseries collection.
  *   requires_timeseries,
  *   # Timeseries collections require FCV 5.0.
  *   requires_fcv_50,
  *   # Cannot insert into a time-series collection in a multi-document transaction
  *   does_not_support_transactions,
+ *   # Explain of a resolved view must be executed by mongos.
+ *   directly_against_shardsvrs_incompatible,
  *   # TODO SERVER-73641 Re-enable this test once the bug is fixed in the sharded case.
  *   assumes_unsharded_collection,
  * ]
@@ -55,11 +60,13 @@ function runTest(underflow, overflow, query, results) {
 
     const pipeline = [{$match: query}, {$project: {_id: 0, [timeFieldName]: 1}}];
 
+    const plan = tsColl.explain().aggregate(pipeline);
+
     // Verify agg pipeline. We don't want to go through a plan that encourages a sort order to
     // avoid BUS and index selection, so we sort after gathering the results.
     const aggActuals = tsColl.aggregate(pipeline).toArray();
     aggActuals.sort(cmpTimeFields);
-    assert.docEq(aggActuals, results);
+    assert.docEq(results, aggActuals, JSON.stringify(plan, null, 4));
 
     // Verify the equivalent find command. We again don't want to go through a plan that
     // encourages a sort order to avoid BUS and index selection, so we sort after gathering the
@@ -158,33 +165,25 @@ runTest(true, true, {[timeFieldName]: {$gte: new Date("1980-01-01")}}, [
     {[timeFieldName]: new Date("2040-01-01")}
 ]);
 
-// Verify ranges that straddle the lower epoch work properly
+// Verify ranges that straddle the lower and upper epoch boundaries work properly.
 runTest(
     true, false, {[timeFieldName]: {$gt: new Date("1920-01-01"), $lt: new Date("1980-01-01")}}, [
         {[timeFieldName]: new Date("1965-01-01")},
         {[timeFieldName]: new Date("1975-01-01")},
     ]);
-
 runTest(
     false, true, {[timeFieldName]: {$gt: new Date("1980-01-01"), $lt: new Date("2050-01-01")}}, [
         {[timeFieldName]: new Date("1995-01-01")},
         {[timeFieldName]: new Date("2040-01-01")},
     ]);
-
-// TODO: SERVER-69952 Literals outside the epoch are currently compared to _id, generally,
-// so we cannot match against them. This will have to be fixed in a similar manner by determining
-// whether the compared dates can be outside the epoch range and not relying on _id in that case.
-//
-// The following scenarios fail:
-// runTest(
-//    false, false, {[timeFieldName]: {$gt: new Date("1920-01-01"), $lt: new Date("1980-01-01")}}, [
-//         {[timeFieldName]: new Date("1971-01-01")},
-//         {[timeFieldName]: new Date("1975-01-01")},
-//     ]);
-// runTest(
-//     false, false, {[timeFieldName]: {$gt: new Date("1980-01-01"), $lt: new Date("2050-01-01")}},
-//     [
-//         {[timeFieldName]: new Date("1995-01-01")},
-//         {[timeFieldName]: new Date("2030-01-01")},
-//     ]);
+runTest(
+    false, false, {[timeFieldName]: {$gt: new Date("1920-01-01"), $lt: new Date("1980-01-01")}}, [
+        {[timeFieldName]: new Date("1971-01-01")},
+        {[timeFieldName]: new Date("1975-01-01")},
+    ]);
+runTest(
+    false, false, {[timeFieldName]: {$gt: new Date("1980-01-01"), $lt: new Date("2050-01-01")}}, [
+        {[timeFieldName]: new Date("1995-01-01")},
+        {[timeFieldName]: new Date("2030-01-01")},
+    ]);
 })();
diff --git a/src/mongo/db/exec/bucket_unpacker.cpp b/src/mongo/db/exec/bucket_unpacker.cpp
index 9303bf8a058..13d5295ab0a 100644
--- a/src/mongo/db/exec/bucket_unpacker.cpp
+++ b/src/mongo/db/exec/bucket_unpacker.cpp
@@ -32,6 +32,11 @@
 #include "mongo/bson/util/bsoncolumn.h"
 #include "mongo/db/exec/bucket_unpacker.h"
 #include "mongo/db/matcher/expression_algo.h"
+#include "mongo/db/matcher/expression_always_boolean.h"
+
+#define MONGO_LOGV2_DEFAULT_COMPONENT ::mongo::logv2::LogComponent::kQuery
+
+#include "mongo/logv2/log.h"
 
 namespace mongo {
 
diff --git a/src/mongo/db/pipeline/document_source_internal_unpack_bucket.cpp b/src/mongo/db/pipeline/document_source_internal_unpack_bucket.cpp
index 0f5d37b2c27..4de05fcbc0e 100644
--- a/src/mongo/db/pipeline/document_source_internal_unpack_bucket.cpp
+++ b/src/mongo/db/pipeline/document_source_internal_unpack_bucket.cpp
@@ -43,6 +43,7 @@
 #include "mongo/db/exec/document_value/document.h"
 #include "mongo/db/matcher/expression.h"
 #include "mongo/db/matcher/expression_algo.h"
+#include "mongo/db/matcher/expression_always_boolean.h"
 #include "mongo/db/matcher/expression_expr.h"
 #include "mongo/db/matcher/expression_internal_expr_comparison.h"
 #include "mongo/db/pipeline/document_source_add_fields.h"
@@ -116,6 +117,9 @@ void optimizeEndOfPipeline(Pipeline::SourceContainer::iterator itr,
     container->splice(std::next(itr), endOfPipeline);
 }
 
+constexpr long long max32BitEpochMillis =
+    static_cast<long long>(std::numeric_limits<uint32_t>::max()) * 1000;
+
 /**
  * Creates an ObjectId initialized with an appropriate timestamp corresponding to 'rhs' and
  * returns it as a Value.
@@ -135,7 +139,7 @@ auto constructObjectIdValue(const BSONElement& rhs, int bucketMaxSpanSeconds) {
         oid.init(date, maxOrMin == OIDInit::max);
         return oid;
     };
-    // Make an ObjectId cooresponding to a date value adjusted by the max bucket value for the
+    // Make an ObjectId corresponding to a date value adjusted by the max bucket value for the
     // time series view that this query operates on. This predicate can be used in a comparison
     // to gauge a max value for a given bucket, rather than a min value.
     auto makeMaxAdjustedDateOID = [&](auto&& date, auto&& maxOrMin) {
@@ -144,9 +148,16 @@ auto constructObjectIdValue(const BSONElement& rhs, int bucketMaxSpanSeconds) {
             // Subtract max bucket range.
             return makeDateOID(date - Seconds{bucketMaxSpanSeconds}, maxOrMin);
         else
-            // Since we're out of range, just make a predicate that is true for all date types.
-            return makeDateOID(Date_t::min(), OIDInit::min);
+            // Since we're out of range, just make a predicate that is true for all dates.
+            // We'll never use an OID for a date < 0 due to OID range limitations, so we set the
+            // minimum date to 0.
+            return makeDateOID(Date_t::fromMillisSinceEpoch(0LL), OIDInit::min);
     };
+
+    // Because the OID timestamp is only 4 bytes, we can't convert larger dates
+    invariant(rhs.date().toMillisSinceEpoch() >= 0LL);
+    invariant(rhs.date().toMillisSinceEpoch() <= max32BitEpochMillis);
+
     // An ObjectId consists of a 4-byte timestamp, as well as a unique value and a counter, thus
     // two ObjectIds initialized with the same date will have different values. To ensure that we
     // do not incorrectly include or exclude any buckets, depending on the operator we will
@@ -707,10 +718,16 @@ std::unique_ptr<MatchExpression> createComparisonPredicate(
 
     BSONObj minTime;
     BSONObj maxTime;
+    bool dateIsExtended = false;
     if (isTimeField) {
         auto timeField = matchExprData.Date();
         minTime = BSON("" << timeField - Seconds(bucketMaxSpanSeconds));
         maxTime = BSON("" << timeField + Seconds(bucketMaxSpanSeconds));
+
+        // The date is in the "extended" range if it doesn't fit into the bottom
+        // 32 bits.
+        long long timestamp = timeField.toMillisSinceEpoch();
+        dateIsExtended = timestamp < 0LL || timestamp > max32BitEpochMillis;
     }
 
     auto minPath = std::string{kControlMinFieldNamePrefix} + matchExprPath;
@@ -743,6 +760,10 @@ std::unique_ptr<MatchExpression> createComparisonPredicate(
                     MatchExprPredicate<InternalExprGTEMatchExpression>(maxPath, matchExprData),
                     MatchExprPredicate<InternalExprLTEMatchExpression>(maxPath,
                                                                        maxTime.firstElement()));
+            } else if (dateIsExtended) {
+                // Since by this point we know that no time value has been inserted which is
+                // outside the epoch range, we know that no document can meet this criteria
+                return std::make_unique<AlwaysFalseMatchExpression>();
             } else {
                 return makePredicate(
                     MatchExprPredicate<InternalExprLTEMatchExpression>(minPath, matchExprData),
@@ -784,6 +805,14 @@ std::unique_ptr<MatchExpression> createComparisonPredicate(
                     MatchExprPredicate<InternalExprGTMatchExpression>(maxPath, matchExprData),
                     MatchExprPredicate<InternalExprGTMatchExpression>(minPath,
                                                                       minTime.firstElement()));
+            } else if (matchExprData.Date().toMillisSinceEpoch() < 0LL) {
+                // Since by this point we know that no time value has been inserted < 0,
+                // every document must meet this criteria
+                return std::make_unique<AlwaysTrueMatchExpression>();
+            } else if (matchExprData.Date().toMillisSinceEpoch() > max32BitEpochMillis) {
+                // Since by this point we know that no time value has been inserted >
+                // max32BitEpochMillis, we know that no document can meet this criteria
+                return std::make_unique<AlwaysFalseMatchExpression>();
             } else {
                 return makePredicate(
                     MatchExprPredicate<InternalExprGTMatchExpression>(maxPath, matchExprData),
@@ -814,6 +843,14 @@ std::unique_ptr<MatchExpression> createComparisonPredicate(
                     MatchExprPredicate<InternalExprGTEMatchExpression>(maxPath, matchExprData),
                     MatchExprPredicate<InternalExprGTEMatchExpression>(minPath,
                                                                        minTime.firstElement()));
+            } else if (matchExprData.Date().toMillisSinceEpoch() < 0LL) {
+                // Since by this point we know that no time value has been inserted < 0,
+                // every document must meet this criteria
+                return std::make_unique<AlwaysTrueMatchExpression>();
+            } else if (matchExprData.Date().toMillisSinceEpoch() > max32BitEpochMillis) {
+                // Since by this point we know that no time value has been inserted > 0xffffffff,
+                // we know that no value can meet this criteria
+                return std::make_unique<AlwaysFalseMatchExpression>();
             } else {
                 return makePredicate(
                     MatchExprPredicate<InternalExprGTEMatchExpression>(maxPath, matchExprData),
@@ -845,6 +882,14 @@ std::unique_ptr<MatchExpression> createComparisonPredicate(
                     MatchExprPredicate<InternalExprLTMatchExpression>(minPath, matchExprData),
                     MatchExprPredicate<InternalExprLTMatchExpression>(maxPath,
                                                                       maxTime.firstElement()));
+            } else if (matchExprData.Date().toMillisSinceEpoch() < 0LL) {
+                // Since by this point we know that no time value has been inserted < 0,
+                // we know that no document can meet this criteria
+                return std::make_unique<AlwaysFalseMatchExpression>();
+            } else if (matchExprData.Date().toMillisSinceEpoch() > max32BitEpochMillis) {
+                // Since by this point we know that no time value has been inserted > 0xffffffff
+                // every time value must be less than this value
+                return std::make_unique<AlwaysTrueMatchExpression>();
             } else {
                 return makePredicate(
                     MatchExprPredicate<InternalExprLTMatchExpression>(minPath, matchExprData),
@@ -874,6 +919,14 @@ std::unique_ptr<MatchExpression> createComparisonPredicate(
                     MatchExprPredicate<InternalExprLTEMatchExpression>(minPath, matchExprData),
                     MatchExprPredicate<InternalExprLTEMatchExpression>(maxPath,
                                                                        maxTime.firstElement()));
+            } else if (matchExprData.Date().toMillisSinceEpoch() < 0LL) {
+                // Since by this point we know that no time value has been inserted < 0,
+                // we know that no document can meet this criteria
+                return std::make_unique<AlwaysFalseMatchExpression>();
+            } else if (matchExprData.Date().toMillisSinceEpoch() > max32BitEpochMillis) {
+                // Since by this point we know that no time value has been inserted > 0xffffffff
+                // every document must be less than this value
+                return std::make_unique<AlwaysTrueMatchExpression>();
             } else {
                 return makePredicate(
                     MatchExprPredicate<InternalExprLTEMatchExpression>(minPath, matchExprData),