SERVER-81013 Fix resolveCollator to return 'kNo' when query has colla…

…tor and collection does not
babiel · Sep 13, 2023 · fa81699 · fa81699
1 parent 53e1c5b
commit fa81699
Show file tree

Hide file tree

Showing 5 changed files with 59 additions and 29 deletions.
diff --git a/etc/backports_required_for_multiversion_tests.yml b/etc/backports_required_for_multiversion_tests.yml
@@ -433,6 +433,12 @@ last-continuous:
     ticket: SERVER-78530
   - test_file: jstests/sharding/delete_range_deletion_tasks_on_dropped_hashed_shard_key_index.js
     ticket: SERVER-78530
+  - test_file: jstests/core/timeseries/nondefault_collation.js
+    ticket: SERVER-81013
+  - test_file: jstests/core/timeseries/timeseries_delete_collation.js
+    ticket: SERVER-81013
+  - test_file: jstests/core/timeseries/timeseries_update_collation.js
+    ticket: SERVER-81013
   suites: null
 last-lts:
   all:
@@ -932,4 +938,10 @@ last-lts:
     ticket: SERVER-78530
   - test_file: jstests/sharding/delete_range_deletion_tasks_on_dropped_hashed_shard_key_index.js
     ticket: SERVER-78530
+  - test_file: jstests/core/timeseries/nondefault_collation.js
+    ticket: SERVER-81013
+  - test_file: jstests/core/timeseries/timeseries_delete_collation.js
+    ticket: SERVER-81013
+  - test_file: jstests/core/timeseries/timeseries_update_collation.js
+    ticket: SERVER-81013
   suites: null
diff --git a/jstests/core/timeseries/nondefault_collation.js b/jstests/core/timeseries/nondefault_collation.js
@@ -93,6 +93,36 @@ const insensitive = {
     assert.eq(1, res2.itcount(), res2.toArray());  // should match only "5"
 }());
 
+(function testFind_OnlyQueryHasCollation() {
+    coll.drop();
+
+    assert.commandWorked(
+        db.createCollection(coll.getName(), {timeseries: {timeField: 'time', metaField: 'meta'}}));
+
+    // This should generate a bucket with control.min.value = 'C' and control.max.value = 'c'.
+    assert.commandWorked(coll.insert({time: ISODate(), meta: 42, value: "C"}));
+    assert.commandWorked(coll.insert({time: ISODate(), meta: 42, value: "b"}));
+    assert.commandWorked(coll.insert({time: ISODate(), meta: 42, value: "c"}));
+
+    // A query with default collation would use the bucket's min/max and find the two matches.
+    const resWithNoCollation = coll.find({value: {$lt: "c"}});
+    assert.eq(2,
+              resWithNoCollation.itcount(),
+              resWithNoCollation.toArray());  // should match "C" and "b".
+
+    // If a query with 'insensitive' collation used the bucket's min/max it would miss the bucket.
+    // Check, that it doesn't.
+    const resWithCollation_find = coll.find({value: {$lt: "c"}}).collation(insensitive);
+    assert.eq(1,
+              resWithCollation_find.itcount(),
+              resWithCollation_find.toArray());  // should match only "b".
+
+    // Run the same test with aggregate command.
+    const resWithCollation_agg =
+        coll.aggregate([{$match: {value: {$lt: "c"}}}], {collation: insensitive}).toArray();
+    assert.eq(1, resWithCollation_agg.length, resWithCollation_agg);
+}());
+
 (function testAgg_GroupByMetaField() {
     coll.drop();
 

diff --git a/jstests/core/timeseries/timeseries_delete_collation.js b/jstests/core/timeseries/timeseries_delete_collation.js
@@ -136,32 +136,23 @@ function runTest({
     });
 })();
 
+// Since the query collation does not match the collection collation, there should be no predicates
+// on control fields. The predicates on meta field are okay because buckets are grouped based on
+// real values, ignoring their collation.
 (function testQueryLevelCollation() {
     // Residual filter.
     runTest({
         deleteFilter: {str: "Hello"},
         queryCollation: caseSensitive,
         nDeleted: 0,
-        expectedBucketQuery: {
-            $and: [
-                closedBucketFilter,
-                {"control.max.str": {$_internalExprGte: "Hello"}},
-                {"control.min.str": {$_internalExprLte: "Hello"}}
-            ]
-        },
+        expectedBucketQuery: closedBucketFilter,
         expectedDeleteStage: "TS_MODIFY"
     });
     runTest({
         deleteFilter: {str: "Hello"},
         queryCollation: caseInsensitive,
         nDeleted: 6,
-        expectedBucketQuery: {
-            $and: [
-                closedBucketFilter,
-                {"control.max.str": {$_internalExprGte: "Hello"}},
-                {"control.min.str": {$_internalExprLte: "Hello"}}
-            ]
-        },
+        expectedBucketQuery: closedBucketFilter,
         expectedDeleteStage: "TS_MODIFY"
     });
 

diff --git a/jstests/core/timeseries/timeseries_update_collation.js b/jstests/core/timeseries/timeseries_update_collation.js
@@ -133,31 +133,22 @@ function runTest({
     });
 })();
 
+// Since the query collation does not match the collection collation, there should be no predicates
+// on control fields. The predicates on meta field are okay because buckets are grouped based on
+// real values, ignoring their collation.
 (function testQueryLevelCollation() {
     // Residual filter.
     runTest({
         updateFilter: {str: "Hello"},
         queryCollation: caseSensitive,
         nModified: 0,
-        expectedBucketQuery: {
-            $and: [
-                closedBucketFilter,
-                {"control.max.str": {$_internalExprGte: "Hello"}},
-                {"control.min.str": {$_internalExprLte: "Hello"}}
-            ]
-        },
+        expectedBucketQuery: closedBucketFilter,
     });
     runTest({
         updateFilter: {str: "Hello"},
         queryCollation: caseInsensitive,
         nModified: 6,
-        expectedBucketQuery: {
-            $and: [
-                closedBucketFilter,
-                {"control.max.str": {$_internalExprGte: "Hello"}},
-                {"control.min.str": {$_internalExprLte: "Hello"}}
-            ]
-        },
+        expectedBucketQuery: closedBucketFilter,
     });
 
     // Bucket filter.

diff --git a/src/mongo/db/catalog/collection.cpp b/src/mongo/db/catalog/collection.cpp
@@ -105,7 +105,13 @@ resolveCollator(OperationContext* opCtx, BSONObj userCollation, const Collection
             return {nullptr, ExpressionContext::CollationMatchesDefault::kYes};
         } else {
             return {getUserCollator(opCtx, userCollation),
-                    ExpressionContext::CollationMatchesDefault::kYes};
+                    // If the user explicitly provided a simple collation, we can still treat it as
+                    // 'CollationMatchesDefault::kYes', as no collation and simple collation are
+                    // functionally equivalent in the query code.
+                    (SimpleBSONObjComparator::kInstance.evaluate(userCollation ==
+                                                                 CollationSpec::kSimpleSpec))
+                        ? ExpressionContext::CollationMatchesDefault::kYes
+                        : ExpressionContext::CollationMatchesDefault::kNo};
         }
     }