Skip to content

Commit 72d8bff

Browse files
jseysterevergreen
jseyster
authored and
evergreen
committed
Revert "SERVER-42836 Fast path for sort key generation of WorkingSetMembers"
Reverting due to failure in external_sort_find.js. This reverts commit d229686.
1 parent f1a3b97 commit 72d8bff

File tree

6 files changed

+134
-144
lines changed

6 files changed

+134
-144
lines changed

‎jstests/aggregation/group_conversion_to_distinct_scan.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ assert.commandWorked(coll.insert([
4444
{_id: 3, a: 1, b: 3, c: 2},
4545
{_id: 4, a: 2, b: 2, c: 2},
4646
{_id: 5, b: 1, c: 1},
47-
{_id: 6, a: null, b: 1, c: 1.5},
47+
{_id: 6, a: null, b: 1, c: 1},
4848

4949
{_id: 7, aa: 1, mkB: 2, bb: 2},
5050
{_id: 8, aa: 1, mkB: [1, 3], bb: 1},
@@ -224,9 +224,9 @@ assert.eq(null, getAggPlanStage(explain, "SORT"), explain);
224224
// Verify that a $sort-$group pipeline can use DISTINCT_SCAN when a $first accumulator needs the
225225
// entire document.
226226
//
227-
pipeline = [{$sort: {a: -1, b: -1, c: -1}}, {$group: {_id: "$a", accum: {$first: "$$ROOT"}}}];
227+
pipeline = [{$sort: {a: -1, b: -1}}, {$group: {_id: "$a", accum: {$first: "$$ROOT"}}}];
228228
assertResultsMatchWithAndWithoutHintandIndexes(pipeline, [
229-
{_id: null, accum: {_id: 6, a: null, b: 1, c: 1.5}},
229+
{_id: null, accum: {_id: 6, a: null, b: 1, c: 1}},
230230
{_id: 1, accum: {_id: 3, a: 1, b: 3, c: 2}},
231231
{_id: 2, accum: {_id: 4, a: 2, b: 2, c: 2}}
232232
]);

‎jstests/sharding/fts_score_sort_sharded.js‎

Lines changed: 6 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,11 @@ assert.commandWorked(coll.ensureIndex({a: "text"}));
3434
var results = coll.find({$text: {$search: "pizza"}}, {s: {$meta: "textScore"}})
3535
.sort({s: {$meta: "textScore"}})
3636
.toArray();
37-
assert.eq(results.length, 4, results);
38-
assert.eq(results[0]._id, -2, results);
39-
assert.eq(results[1]._id, 2, results);
40-
assert.eq(results[2]._id, -1, results);
41-
assert.eq(results[3]._id, 1, results);
37+
assert.eq(results.length, 4);
38+
assert.eq(results[0]._id, -2);
39+
assert.eq(results[1]._id, 2);
40+
assert.eq(results[2]._id, -1);
41+
assert.eq(results[3]._id, 1);
4242

4343
//
4444
// Verify that mongos requires the text metadata sort to be specified in the projection.
@@ -68,36 +68,6 @@ assert.throws(function() {
6868
cursor.next();
6969
});
7070

71-
//
72-
// Execute query with a compound sort that includes the text score along with a multikey field.
73-
//
74-
75-
coll.drop();
76-
assert.commandWorked(coll.insert({_id: 0, a: "pizza", b: [1, 4]}));
77-
assert.commandWorked(coll.insert({_id: 1, a: "pizza pizza", b: [6, 7]}));
78-
assert.commandWorked(coll.insert({_id: 2, a: "pizza", b: [2, 3]}));
79-
assert.commandWorked(coll.insert({_id: 3, a: "pizza pizza", b: [5, 8]}));
80-
assert.commandWorked(coll.ensureIndex({a: "text"}));
81-
82-
results = coll.find({$text: {$search: "pizza"}}, {s: {$meta: "textScore"}})
83-
.sort({s: {$meta: "textScore"}, b: 1})
84-
.toArray();
85-
assert.eq(results.length, 4, results);
86-
assert.eq(results[0]._id, 3, results);
87-
assert.eq(results[1]._id, 1, results);
88-
assert.eq(results[2]._id, 0, results);
89-
assert.eq(results[3]._id, 2, results);
90-
91-
//
92-
// Repeat the query with an aggregation pipeline and verify that the result is the same.
93-
//
94-
95-
var aggResults = coll.aggregate([
96-
{$match: {$text: {$search: "pizza"}}},
97-
{$addFields: {s: {$meta: "textScore"}}},
98-
{$sort: {s: {$meta: "textScore"}, b: 1}}
99-
])
100-
.toArray();
101-
assert.eq(results, aggResults);
71+
// TODO Test sort on compound key.
10272

10373
st.stop();

‎src/mongo/db/exec/sort_key_generator.cpp‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -68,16 +68,16 @@ PlanStage::StageState SortKeyGeneratorStage::doWork(WorkingSetID* out) {
6868
if (stageState == PlanStage::ADVANCED) {
6969
WorkingSetMember* member = _ws->get(*out);
7070

71-
try {
72-
auto sortKey = _sortKeyGen.computeSortKey(*member);
73-
74-
// Add the sort key to the WSM as metadata.
75-
member->metadata().setSortKey(std::move(sortKey), _sortKeyGen.isSingleElementKey());
76-
} catch (const DBException& computeSortKeyException) {
77-
*out = WorkingSetCommon::allocateStatusMember(_ws, computeSortKeyException.toStatus());
71+
auto sortKey = _sortKeyGen.computeSortKey(*member);
72+
if (!sortKey.isOK()) {
73+
*out = WorkingSetCommon::allocateStatusMember(_ws, sortKey.getStatus());
7874
return PlanStage::FAILURE;
7975
}
8076

77+
// Add the sort key to the WSM as metadata.
78+
member->metadata().setSortKey(std::move(sortKey.getValue()),
79+
_sortKeyGen.isSingleElementKey());
80+
8181
return PlanStage::ADVANCED;
8282
}
8383

‎src/mongo/db/index/sort_key_generator.cpp‎

Lines changed: 58 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -76,15 +76,30 @@ SortKeyGenerator::SortKeyGenerator(SortPattern sortPattern, const CollatorInterf
7676
Ordering::make(_sortSpecWithoutMeta));
7777
}
7878

79-
Value SortKeyGenerator::computeSortKey(const WorkingSetMember& wsm) const {
79+
// TODO (SERVER-42836): Once WorkingSetMember objects store a Document (SERVER-42181), this function
80+
// will be able to use the Document overload of computeSortKeyFromDocument, and it will be able to
81+
// store the text score with the Document instead of in a separate SortKeyGenerator::Metadata
82+
// object.
83+
StatusWith<Value> SortKeyGenerator::computeSortKey(const WorkingSetMember& wsm) const {
8084
if (wsm.hasObj()) {
81-
return computeSortKeyFromDocument(wsm.doc.value(), wsm.metadata());
85+
SortKeyGenerator::Metadata metadata;
86+
if (_sortHasMeta && wsm.metadata().hasTextScore()) {
87+
metadata.textScore = wsm.metadata().getTextScore();
88+
}
89+
auto statusWithSortKeyObj = computeSortKeyFromDocument(wsm.doc.value().toBson(), &metadata);
90+
if (!statusWithSortKeyObj.isOK()) {
91+
return statusWithSortKeyObj.getStatus();
92+
}
93+
94+
return DocumentMetadataFields::deserializeSortKey(isSingleElementKey(),
95+
statusWithSortKeyObj.getValue());
8296
}
8397

8498
return computeSortKeyFromIndexKey(wsm);
8599
}
86100

87-
Value SortKeyGenerator::computeSortKeyFromIndexKey(const WorkingSetMember& member) const {
101+
StatusWith<Value> SortKeyGenerator::computeSortKeyFromIndexKey(
102+
const WorkingSetMember& member) const {
88103
invariant(member.getState() == WorkingSetMember::RID_AND_IDX);
89104
invariant(!_sortHasMeta);
90105

@@ -103,9 +118,16 @@ Value SortKeyGenerator::computeSortKeyFromIndexKey(const WorkingSetMember& membe
103118
return DocumentMetadataFields::deserializeSortKey(isSingleElementKey(), objBuilder.obj());
104119
}
105120

106-
BSONObj SortKeyGenerator::computeSortKeyFromDocument(const BSONObj& obj,
107-
const DocumentMetadataFields& metadata) const {
108-
auto sortKeyNoMetadata = uassertStatusOK(computeSortKeyFromDocumentWithoutMetadata(obj));
121+
StatusWith<BSONObj> SortKeyGenerator::computeSortKeyFromDocument(const BSONObj& obj,
122+
const Metadata* metadata) const {
123+
if (_sortHasMeta) {
124+
invariant(metadata);
125+
}
126+
127+
auto sortKeyNoMetadata = computeSortKeyFromDocumentWithoutMetadata(obj);
128+
if (!sortKeyNoMetadata.isOK()) {
129+
return sortKeyNoMetadata;
130+
}
109131

110132
if (!_sortHasMeta) {
111133
// We don't have to worry about $meta sort, so the index key becomes the sort key.
@@ -115,27 +137,24 @@ BSONObj SortKeyGenerator::computeSortKeyFromDocument(const BSONObj& obj,
115137
BSONObjBuilder mergedKeyBob;
116138

117139
// Merge metadata into the key.
118-
BSONObjIterator sortKeyIt(sortKeyNoMetadata);
140+
BSONObjIterator sortKeyIt(sortKeyNoMetadata.getValue());
119141
for (auto& part : _sortPattern) {
120142
if (part.fieldPath) {
121143
invariant(sortKeyIt.more());
122144
mergedKeyBob.append(sortKeyIt.next());
123145
continue;
124146
}
125-
126-
// Create a Document that represents the input object and its metadata together, so we can
127-
// use it to evaluate the ExpressionMeta for this part of the sort pattern. This operation
128-
// copies the data in 'metadata' but not any of the data in the 'obj' BSON.
129-
MutableDocument documentWithMetdata(Document{obj});
130-
documentWithMetdata.setMetadata(DocumentMetadataFields(metadata));
131-
132147
invariant(part.expression);
133-
auto value =
134-
part.expression->evaluate(documentWithMetdata.freeze(), nullptr /* variables */);
135-
if (!value.missing()) {
136-
value.addToBsonObj(&mergedKeyBob, ""_sd);
137-
} else {
138-
mergedKeyBob.appendNull("");
148+
switch (part.expression->getMetaType()) {
149+
case DocumentMetadataFields::MetaType::kTextScore: {
150+
mergedKeyBob.append("", metadata->textScore);
151+
continue;
152+
}
153+
case DocumentMetadataFields::MetaType::kRandVal: {
154+
mergedKeyBob.append("", metadata->randVal);
155+
continue;
156+
}
157+
default: { MONGO_UNREACHABLE; }
139158
}
140159
}
141160

@@ -212,9 +231,7 @@ Value SortKeyGenerator::getCollationComparisonKey(const Value& val) const {
212231
}
213232

214233
StatusWith<Value> SortKeyGenerator::extractKeyPart(
215-
const Document& doc,
216-
const DocumentMetadataFields& metadata,
217-
const SortPattern::SortPatternPart& patternPart) const {
234+
const Document& doc, const SortPattern::SortPatternPart& patternPart) const {
218235
Value plainKey;
219236
if (patternPart.fieldPath) {
220237
invariant(!patternPart.expression);
@@ -226,28 +243,22 @@ StatusWith<Value> SortKeyGenerator::extractKeyPart(
226243
plainKey = key.getValue();
227244
} else {
228245
invariant(patternPart.expression);
229-
// ExpressionMeta expects metadata to be attached to the document.
230-
MutableDocument documentWithMetadata(doc);
231-
documentWithMetadata.setMetadata(DocumentMetadataFields(metadata));
232-
233246
// ExpressionMeta does not use Variables.
234-
plainKey = patternPart.expression->evaluate(documentWithMetadata.freeze(),
235-
nullptr /* variables */);
247+
plainKey = patternPart.expression->evaluate(doc, nullptr /* variables */);
236248
}
237249

238-
return plainKey.missing() ? Value{BSONNULL} : getCollationComparisonKey(plainKey);
250+
return getCollationComparisonKey(plainKey);
239251
}
240252

241-
StatusWith<Value> SortKeyGenerator::extractKeyFast(const Document& doc,
242-
const DocumentMetadataFields& metadata) const {
253+
StatusWith<Value> SortKeyGenerator::extractKeyFast(const Document& doc) const {
243254
if (_sortPattern.isSingleElementKey()) {
244-
return extractKeyPart(doc, metadata, _sortPattern[0]);
255+
return extractKeyPart(doc, _sortPattern[0]);
245256
}
246257

247258
std::vector<Value> keys;
248259
keys.reserve(_sortPattern.size());
249260
for (auto&& keyPart : _sortPattern) {
250-
auto extractedKey = extractKeyPart(doc, metadata, keyPart);
261+
auto extractedKey = extractKeyPart(doc, keyPart);
251262
if (!extractedKey.isOK()) {
252263
// We can't use the fast path, so bail out.
253264
return extractedKey;
@@ -258,18 +269,24 @@ StatusWith<Value> SortKeyGenerator::extractKeyFast(const Document& doc,
258269
return Value{std::move(keys)};
259270
}
260271

261-
BSONObj SortKeyGenerator::extractKeyWithArray(const Document& doc,
262-
const DocumentMetadataFields& metadata) const {
272+
BSONObj SortKeyGenerator::extractKeyWithArray(const Document& doc) const {
273+
SortKeyGenerator::Metadata metadata;
274+
if (doc.metadata().hasTextScore()) {
275+
metadata.textScore = doc.metadata().getTextScore();
276+
}
277+
if (doc.metadata().hasRandVal()) {
278+
metadata.randVal = doc.metadata().getRandVal();
279+
}
280+
263281
// Convert the Document to a BSONObj, but only do the conversion for the paths we actually need.
264282
// Then run the result through the SortKeyGenerator to obtain the final sort key.
265283
auto bsonDoc = _sortPattern.documentToBsonWithSortPaths(doc);
266-
return computeSortKeyFromDocument(bsonDoc, metadata);
284+
return uassertStatusOK(computeSortKeyFromDocument(bsonDoc, &metadata));
267285
}
268286

269-
Value SortKeyGenerator::computeSortKeyFromDocument(const Document& doc,
270-
const DocumentMetadataFields& metadata) const {
287+
Value SortKeyGenerator::computeSortKeyFromDocument(const Document& doc) const {
271288
// This fast pass directly generates a Value.
272-
auto fastKey = extractKeyFast(doc, metadata);
289+
auto fastKey = extractKeyFast(doc);
273290
if (fastKey.isOK()) {
274291
return std::move(fastKey.getValue());
275292
}
@@ -278,7 +295,7 @@ Value SortKeyGenerator::computeSortKeyFromDocument(const Document& doc,
278295
// form like BSONObj {'': 1, '': [2, 3]}) and converts it to a Value (Value [1, [2, 3]] in the
279296
// earlier example).
280297
return DocumentMetadataFields::deserializeSortKey(_sortPattern.isSingleElementKey(),
281-
extractKeyWithArray(doc, metadata));
298+
extractKeyWithArray(doc));
282299
}
283300

284301
} // namespace mongo

0 commit comments

Comments
 (0)