summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTess Avitabile <tess.avitabile@mongodb.com>2017-11-29 19:16:50 -0500
committerTess Avitabile <tess.avitabile@mongodb.com>2017-12-15 11:54:09 -0500
commit59af713ebb0004c73a2891759ac28ed580e8b4b1 (patch)
treedf15c818a1936e828de4575f74a8a2b0b0900359
parentb4172b971d6223ab22e070e738ca69530ff79277 (diff)
downloadmongo-59af713ebb0004c73a2891759ac28ed580e8b4b1.tar.gz
SERVER-32048 Ensure updates that implicitly create an array element generate new null index keys
(cherry picked from commit 69f94d7172801725bad43dba99ca0e5400eb293c) (cherry picked from commit 80b311e0a8537a47eb723ed4b6c010b5d1006551) (cherry picked from commit 5e58d9f0b6e6b42a0c9f4817e88ee1bfb7527d81) (cherry picked from commit 752daa306095fb1610bb5db13b7b106ac87ec6cb)
-rw-r--r--jstests/core/update_affects_indexes.js104
-rw-r--r--src/mongo/db/ops/modifier_add_to_set.cpp8
-rw-r--r--src/mongo/db/ops/modifier_add_to_set_test.cpp36
-rw-r--r--src/mongo/db/ops/modifier_bit.cpp8
-rw-r--r--src/mongo/db/ops/modifier_bit_test.cpp37
-rw-r--r--src/mongo/db/ops/modifier_compare.cpp7
-rw-r--r--src/mongo/db/ops/modifier_compare_test.cpp36
-rw-r--r--src/mongo/db/ops/modifier_current_date.cpp10
-rw-r--r--src/mongo/db/ops/modifier_current_date_test.cpp37
-rw-r--r--src/mongo/db/ops/modifier_inc.cpp7
-rw-r--r--src/mongo/db/ops/modifier_inc_test.cpp37
-rw-r--r--src/mongo/db/ops/modifier_interface.h8
-rw-r--r--src/mongo/db/ops/modifier_push.cpp9
-rw-r--r--src/mongo/db/ops/modifier_push_test.cpp83
-rw-r--r--src/mongo/db/ops/modifier_set.cpp6
-rw-r--r--src/mongo/db/ops/modifier_set_test.cpp37
-rw-r--r--src/mongo/db/ops/update_driver.cpp20
17 files changed, 485 insertions, 5 deletions
diff --git a/jstests/core/update_affects_indexes.js b/jstests/core/update_affects_indexes.js
new file mode 100644
index 00000000000..6bc259d3a24
--- /dev/null
+++ b/jstests/core/update_affects_indexes.js
@@ -0,0 +1,104 @@
+// This is a regression test for SERVER-32048. It checks that index keys are correctly updated when
+// an update modifier implicitly creates a new array element.
+(function() {
+ "use strict";
+
+ var coll = db.update_affects_indexes;
+ coll.drop();
+ var indexKeyPattern = {
+ "a.b": 1
+ };
+ assert.commandWorked(coll.createIndex(indexKeyPattern));
+
+ // Tests that the document 'docId' has all the index keys in 'expectedKeys' and none of the
+ // index keys in 'unexpectedKeys'.
+ function assertExpectedIndexKeys(docId, expectedKeys, unexpectedKeys) {
+ for (var i = 0; i < expectedKeys.length; i++) {
+ var key = expectedKeys[i];
+ var res = coll.find(docId).hint(indexKeyPattern).min(key)._addSpecial("$returnKey", true).toArray();
+ assert.eq(1, res.length, tojson(res));
+ assert.eq(0, bsonWoCompare(key, res[0]), tojson(res[0]));
+ }
+
+ for (var i = 0; i < unexpectedKeys.length; i++) {
+ var key = unexpectedKeys[i];
+ var res = coll.find(docId).hint(indexKeyPattern).min(key)._addSpecial("$returnKey", true).toArray();
+ if (res.length > 0) {
+ assert.eq(1, res.length, tojson(res));
+ assert.neq(0, bsonWoCompare(key, res[0]), tojson(res[0]));
+ }
+ }
+ }
+
+ // $set implicitly creates array element at end of array.
+ assert.writeOK(coll.insert({_id: 0, a: [{b: 0}]}));
+ assertExpectedIndexKeys({_id: 0}, [{"a.b": 0}], [{"a.b": null}]);
+ assert.writeOK(coll.update({_id: 0}, {$set: {"a.1.c": 0}}));
+ assertExpectedIndexKeys({_id: 0}, [{"a.b": 0}, {"a.b": null}], []);
+
+ // $set implicitly creates array element beyond end of array.
+ assert.writeOK(coll.insert({_id: 1, a: [{b: 0}]}));
+ assertExpectedIndexKeys({_id: 1}, [{"a.b": 0}], [{"a.b": null}]);
+ assert.writeOK(coll.update({_id: 1}, {$set: {"a.3.c": 0}}));
+ assertExpectedIndexKeys({_id: 1}, [{"a.b": 0}, {"a.b": null}], []);
+
+ // $set implicitly creates array element in empty array (no index key changes needed).
+ assert.writeOK(coll.insert({_id: 2, a: []}));
+ assertExpectedIndexKeys({_id: 2}, [{"a.b": null}], []);
+ assert.writeOK(coll.update({_id: 2}, {$set: {"a.0.c": 0}}));
+ assertExpectedIndexKeys({_id: 2}, [{"a.b": null}], []);
+
+ // $inc implicitly creates array element at end of array.
+ assert.writeOK(coll.insert({_id: 3, a: [{b: 0}]}));
+ assertExpectedIndexKeys({_id: 3}, [{"a.b": 0}], [{"a.b": null}]);
+ assert.writeOK(coll.update({_id: 3}, {$inc: {"a.1.c": 0}}));
+ assertExpectedIndexKeys({_id: 3}, [{"a.b": 0}, {"a.b": null}], []);
+
+ // $mul implicitly creates array element at end of array.
+ assert.writeOK(coll.insert({_id: 4, a: [{b: 0}]}));
+ assertExpectedIndexKeys({_id: 4}, [{"a.b": 0}], [{"a.b": null}]);
+ assert.writeOK(coll.update({_id: 4}, {$mul: {"a.1.c": 0}}));
+ assertExpectedIndexKeys({_id: 4}, [{"a.b": 0}, {"a.b": null}], []);
+
+ // $addToSet implicitly creates array element at end of array.
+ assert.writeOK(coll.insert({_id: 5, a: [{b: 0}]}));
+ assertExpectedIndexKeys({_id: 5}, [{"a.b": 0}], [{"a.b": null}]);
+ assert.writeOK(coll.update({_id: 5}, {$addToSet: {"a.1.c": 0}}));
+ assertExpectedIndexKeys({_id: 5}, [{"a.b": 0}, {"a.b": null}], []);
+
+ // $bit implicitly creates array element at end of array.
+ assert.writeOK(coll.insert({_id: 6, a: [{b: 0}]}));
+ assertExpectedIndexKeys({_id: 6}, [{"a.b": 0}], [{"a.b": null}]);
+ assert.writeOK(coll.update({_id: 6}, {$bit: {"a.1.c": {and: NumberInt(1)}}}));
+ assertExpectedIndexKeys({_id: 6}, [{"a.b": 0}, {"a.b": null}], []);
+
+ // $min implicitly creates array element at end of array.
+ assert.writeOK(coll.insert({_id: 7, a: [{b: 0}]}));
+ assertExpectedIndexKeys({_id: 7}, [{"a.b": 0}], [{"a.b": null}]);
+ assert.writeOK(coll.update({_id: 7}, {$min: {"a.1.c": 0}}));
+ assertExpectedIndexKeys({_id: 7}, [{"a.b": 0}, {"a.b": null}], []);
+
+ // $max implicitly creates array element at end of array.
+ assert.writeOK(coll.insert({_id: 8, a: [{b: 0}]}));
+ assertExpectedIndexKeys({_id: 8}, [{"a.b": 0}], [{"a.b": null}]);
+ assert.writeOK(coll.update({_id: 8}, {$max: {"a.1.c": 0}}));
+ assertExpectedIndexKeys({_id: 8}, [{"a.b": 0}, {"a.b": null}], []);
+
+ // $currentDate implicitly creates array element at end of array.
+ assert.writeOK(coll.insert({_id: 9, a: [{b: 0}]}));
+ assertExpectedIndexKeys({_id: 9}, [{"a.b": 0}], [{"a.b": null}]);
+ assert.writeOK(coll.update({_id: 9}, {$currentDate: {"a.1.c": true}}));
+ assertExpectedIndexKeys({_id: 9}, [{"a.b": 0}, {"a.b": null}], []);
+
+ // $push implicitly creates array element at end of array.
+ assert.writeOK(coll.insert({_id: 10, a: [{b: 0}]}));
+ assertExpectedIndexKeys({_id: 10}, [{"a.b": 0}], [{"a.b": null}]);
+ assert.writeOK(coll.update({_id: 10}, {$push: {"a.1.c": 0}}));
+ assertExpectedIndexKeys({_id: 10}, [{"a.b": 0}, {"a.b": null}], []);
+
+ // $pushAll implicitly creates array element at end of array.
+ assert.writeOK(coll.insert({_id: 11, a: [{b: 0}]}));
+ assertExpectedIndexKeys({_id: 11}, [{"a.b": 0}], [{"a.b": null}]);
+ assert.writeOK(coll.update({_id: 11}, {$pushAll: {"a.1.c": [0]}}));
+ assertExpectedIndexKeys({_id: 11}, [{"a.b": 0}, {"a.b": null}], []);
+}());
diff --git a/src/mongo/db/ops/modifier_add_to_set.cpp b/src/mongo/db/ops/modifier_add_to_set.cpp
index 417ceeccf82..ce76fe28192 100644
--- a/src/mongo/db/ops/modifier_add_to_set.cpp
+++ b/src/mongo/db/ops/modifier_add_to_set.cpp
@@ -219,6 +219,8 @@ Status ModifierAddToSet::prepare(mb::Element root,
// Locate the field name in 'root'.
Status status = pathsupport::findLongestPrefix(
_fieldRef, root, &_preparedState->idxFound, &_preparedState->elemFound);
+ const auto elemFoundIsArray =
+ _preparedState->elemFound.ok() && _preparedState->elemFound.getType() == BSONType::Array;
// FindLongestPrefix may say the path does not exist at all, which is fine here, or
// that the path was not viable or otherwise wrong, in which case, the mod cannot
@@ -242,6 +244,12 @@ Status ModifierAddToSet::prepare(mb::Element root,
if (!_preparedState->elemFound.ok() || _preparedState->idxFound < (_fieldRef.numParts() - 1)) {
// If no target element exists, we will simply be creating a new array.
_preparedState->addAll = true;
+
+ if (elemFoundIsArray) {
+ // Report that an existing array will gain a new element as a result of this mod.
+ execInfo->indexOfArrayWithNewElement[0] = _preparedState->idxFound;
+ }
+
return Status::OK();
}
diff --git a/src/mongo/db/ops/modifier_add_to_set_test.cpp b/src/mongo/db/ops/modifier_add_to_set_test.cpp
index ad3c61eec7d..0bd774b719d 100644
--- a/src/mongo/db/ops/modifier_add_to_set_test.cpp
+++ b/src/mongo/db/ops/modifier_add_to_set_test.cpp
@@ -388,4 +388,40 @@ TEST(Regressions, SERVER_12848) {
ASSERT_EQUALS(fromjson("{ $set : { 'a.1' : [ 1 ] } }"), logDoc);
}
+TEST(IndexedMod, PrepareReportCreatedArrayElement) {
+ Document doc(fromjson("{a: [{b: 0}]}"));
+ Mod mod(fromjson("{$addToSet: {'a.1.c': 2}}"));
+
+ ModifierInterface::ExecInfo execInfo;
+ ASSERT_OK(mod.prepare(doc.root(), "", &execInfo));
+
+ ASSERT_EQUALS(execInfo.fieldRef[0]->dottedField(), "a.1.c");
+ ASSERT_TRUE(execInfo.indexOfArrayWithNewElement[0]);
+ ASSERT_EQUALS(*execInfo.indexOfArrayWithNewElement[0], 0u);
+ ASSERT_FALSE(execInfo.noOp);
+}
+
+TEST(IndexedMod, PrepareDoNotReportModifiedArrayElement) {
+ Document doc(fromjson("{a: [{b: 0}]}"));
+ Mod mod(fromjson("{$addToSet: {'a.0.c': 2}}"));
+
+ ModifierInterface::ExecInfo execInfo;
+ ASSERT_OK(mod.prepare(doc.root(), "", &execInfo));
+
+ ASSERT_EQUALS(execInfo.fieldRef[0]->dottedField(), "a.0.c");
+ ASSERT_FALSE(execInfo.indexOfArrayWithNewElement[0]);
+ ASSERT_FALSE(execInfo.noOp);
+}
+
+TEST(IndexedMod, PrepareDoNotReportCreatedNumericObjectField) {
+ Document doc(fromjson("{a: {'0': {b: 0}}}"));
+ Mod mod(fromjson("{$addToSet: {'a.1.c': 2}}"));
+
+ ModifierInterface::ExecInfo execInfo;
+ ASSERT_OK(mod.prepare(doc.root(), "", &execInfo));
+
+ ASSERT_EQUALS(execInfo.fieldRef[0]->dottedField(), "a.1.c");
+ ASSERT_FALSE(execInfo.indexOfArrayWithNewElement[0]);
+ ASSERT_FALSE(execInfo.noOp);
+}
} // namespace
diff --git a/src/mongo/db/ops/modifier_bit.cpp b/src/mongo/db/ops/modifier_bit.cpp
index dddc2a7a61c..1e19884baab 100644
--- a/src/mongo/db/ops/modifier_bit.cpp
+++ b/src/mongo/db/ops/modifier_bit.cpp
@@ -152,6 +152,8 @@ Status ModifierBit::prepare(mutablebson::Element root,
// Locate the field name in 'root'.
Status status = pathsupport::findLongestPrefix(
_fieldRef, root, &_preparedState->idxFound, &_preparedState->elemFound);
+ const auto elemFoundIsArray =
+ _preparedState->elemFound.ok() && _preparedState->elemFound.getType() == BSONType::Array;
// FindLongestPrefix may say the path does not exist at all, which is fine here, or
@@ -177,6 +179,12 @@ Status ModifierBit::prepare(mutablebson::Element root,
// If no target element exists, the value we will write is the result of applying
// the operation to a zero-initialized integer element.
_preparedState->newValue = apply(SafeNum(static_cast<int>(0)));
+
+ if (elemFoundIsArray) {
+ // Report that an existing array will gain a new element as a result of this mod.
+ execInfo->indexOfArrayWithNewElement[0] = _preparedState->idxFound;
+ }
+
return Status::OK();
}
diff --git a/src/mongo/db/ops/modifier_bit_test.cpp b/src/mongo/db/ops/modifier_bit_test.cpp
index 743693d5bf5..a7cca4cb9e5 100644
--- a/src/mongo/db/ops/modifier_bit_test.cpp
+++ b/src/mongo/db/ops/modifier_bit_test.cpp
@@ -731,4 +731,41 @@ TEST(DbUpdateTests, Bit1_4_Combined) {
ASSERT_EQUALS(BSON("$set" << BSON("x" << ((3 | 2) & 8))), logDoc);
}
+TEST(IndexedMod, PrepareReportCreatedArrayElement) {
+ Document doc(fromjson("{a: [{b: 0}]}"));
+ Mod mod(fromjson("{$bit: {'a.1.c': {and: NumberInt(1)}}}"));
+
+ ModifierInterface::ExecInfo execInfo;
+ ASSERT_OK(mod.prepare(doc.root(), "", &execInfo));
+
+ ASSERT_EQUALS(execInfo.fieldRef[0]->dottedField(), "a.1.c");
+ ASSERT_TRUE(execInfo.indexOfArrayWithNewElement[0]);
+ ASSERT_EQUALS(*execInfo.indexOfArrayWithNewElement[0], 0u);
+ ASSERT_FALSE(execInfo.noOp);
+}
+
+TEST(IndexedMod, PrepareDoNotReportModifiedArrayElement) {
+ Document doc(fromjson("{a: [{b: NumberInt(0)}]}"));
+ Mod mod(fromjson("{$bit: {'a.0.c': {or: NumberInt(1)}}}"));
+
+ ModifierInterface::ExecInfo execInfo;
+ ASSERT_OK(mod.prepare(doc.root(), "", &execInfo));
+
+ ASSERT_EQUALS(execInfo.fieldRef[0]->dottedField(), "a.0.c");
+ ASSERT_FALSE(execInfo.indexOfArrayWithNewElement[0]);
+ ASSERT_FALSE(execInfo.noOp);
+}
+
+TEST(IndexedMod, PrepareDoNotReportCreatedNumericObjectField) {
+ Document doc(fromjson("{a: {'0': {b: 0}}}"));
+ Mod mod(fromjson("{$bit: {'a.1.c': {and: NumberInt(1)}}}"));
+
+ ModifierInterface::ExecInfo execInfo;
+ ASSERT_OK(mod.prepare(doc.root(), "", &execInfo));
+
+ ASSERT_EQUALS(execInfo.fieldRef[0]->dottedField(), "a.1.c");
+ ASSERT_FALSE(execInfo.indexOfArrayWithNewElement[0]);
+ ASSERT_FALSE(execInfo.noOp);
+}
+
} // namespace
diff --git a/src/mongo/db/ops/modifier_compare.cpp b/src/mongo/db/ops/modifier_compare.cpp
index 2c9a48ac238..3be7c669428 100644
--- a/src/mongo/db/ops/modifier_compare.cpp
+++ b/src/mongo/db/ops/modifier_compare.cpp
@@ -107,6 +107,8 @@ Status ModifierCompare::prepare(mutablebson::Element root,
// be created during the apply.
Status status = pathsupport::findLongestPrefix(
_updatePath, root, &_preparedState->idxFound, &_preparedState->elemFound);
+ const auto elemFoundIsArray =
+ _preparedState->elemFound.ok() && _preparedState->elemFound.getType() == BSONType::Array;
// FindLongestPrefix may say the path does not exist at all, which is fine here, or
// that the path was not viable or otherwise wrong, in which case, the mod cannot
@@ -125,6 +127,11 @@ Status ModifierCompare::prepare(mutablebson::Element root,
_preparedState->idxFound == (_updatePath.numParts() - 1));
if (!destExists) {
execInfo->noOp = false;
+
+ if (elemFoundIsArray) {
+ // Report that an existing array will gain a new element as a result of this mod.
+ execInfo->indexOfArrayWithNewElement[0] = _preparedState->idxFound;
+ }
} else {
const int compareVal = _preparedState->elemFound.compareWithBSONElement(_val, false);
execInfo->noOp = (compareVal == 0) ||
diff --git a/src/mongo/db/ops/modifier_compare_test.cpp b/src/mongo/db/ops/modifier_compare_test.cpp
index 479abedbade..993be74ee5a 100644
--- a/src/mongo/db/ops/modifier_compare_test.cpp
+++ b/src/mongo/db/ops/modifier_compare_test.cpp
@@ -293,4 +293,40 @@ TEST(ExistingEmbeddedDoc, MaxNumber) {
ASSERT_EQUALS("a", execInfo.fieldRef[0]->dottedField());
}
+TEST(IndexedMod, PrepareReportCreatedArrayElement) {
+ Document doc(fromjson("{a: [{b: 0}]}"));
+ Mod mod(fromjson("{$min: {'a.1.c': 2}}"));
+
+ ModifierInterface::ExecInfo execInfo;
+ ASSERT_OK(mod.prepare(doc.root(), "", &execInfo));
+
+ ASSERT_EQUALS(execInfo.fieldRef[0]->dottedField(), "a.1.c");
+ ASSERT_TRUE(execInfo.indexOfArrayWithNewElement[0]);
+ ASSERT_EQUALS(*execInfo.indexOfArrayWithNewElement[0], 0u);
+ ASSERT_FALSE(execInfo.noOp);
+}
+
+TEST(IndexedMod, PrepareDoNotReportModifiedArrayElement) {
+ Document doc(fromjson("{a: [{b: 0}]}"));
+ Mod mod(fromjson("{$min: {'a.0.c': 2}}"));
+
+ ModifierInterface::ExecInfo execInfo;
+ ASSERT_OK(mod.prepare(doc.root(), "", &execInfo));
+
+ ASSERT_EQUALS(execInfo.fieldRef[0]->dottedField(), "a.0.c");
+ ASSERT_FALSE(execInfo.indexOfArrayWithNewElement[0]);
+ ASSERT_FALSE(execInfo.noOp);
+}
+
+TEST(IndexedMod, PrepareDoNotReportCreatedNumericObjectField) {
+ Document doc(fromjson("{a: {'0': {b: 0}}}"));
+ Mod mod(fromjson("{$min: {'a.1.c': 2}}"));
+
+ ModifierInterface::ExecInfo execInfo;
+ ASSERT_OK(mod.prepare(doc.root(), "", &execInfo));
+
+ ASSERT_EQUALS(execInfo.fieldRef[0]->dottedField(), "a.1.c");
+ ASSERT_FALSE(execInfo.indexOfArrayWithNewElement[0]);
+ ASSERT_FALSE(execInfo.noOp);
+}
} // namespace
diff --git a/src/mongo/db/ops/modifier_current_date.cpp b/src/mongo/db/ops/modifier_current_date.cpp
index 11041be98ff..ed37f5e6298 100644
--- a/src/mongo/db/ops/modifier_current_date.cpp
+++ b/src/mongo/db/ops/modifier_current_date.cpp
@@ -159,6 +159,8 @@ Status ModifierCurrentDate::prepare(mutablebson::Element root,
// be created during the apply.
Status status = pathsupport::findLongestPrefix(
_updatePath, root, &_preparedState->idxFound, &_preparedState->elemFound);
+ const auto elemFoundIsArray =
+ _preparedState->elemFound.ok() && _preparedState->elemFound.getType() == BSONType::Array;
// FindLongestPrefix may say the path does not exist at all, which is fine here, or
// that the path was not viable or otherwise wrong, in which case, the mod cannot
@@ -173,6 +175,14 @@ Status ModifierCurrentDate::prepare(mutablebson::Element root,
// there is any conflict among mods.
execInfo->fieldRef[0] = &_updatePath;
+ if (!_preparedState->elemFound.ok() ||
+ _preparedState->idxFound < (_updatePath.numParts() - 1)) {
+ if (elemFoundIsArray) {
+ // Report that an existing array will gain a new element as a result of this mod.
+ execInfo->indexOfArrayWithNewElement[0] = _preparedState->idxFound;
+ }
+ }
+
return Status::OK();
}
diff --git a/src/mongo/db/ops/modifier_current_date_test.cpp b/src/mongo/db/ops/modifier_current_date_test.cpp
index e5d21f79883..75586db56cc 100644
--- a/src/mongo/db/ops/modifier_current_date_test.cpp
+++ b/src/mongo/db/ops/modifier_current_date_test.cpp
@@ -360,4 +360,41 @@ TEST(DottedTimestampInput, EmptyStartDoc) {
validateOplogEntry(oplogFormat, logDoc);
}
+TEST(BoolInput, PrepareReportCreatedArrayElement) {
+ Document doc(fromjson("{a: [{b: 0}]}"));
+ Mod mod(fromjson("{$currentDate: {'a.1.c': true}}"));
+
+ ModifierInterface::ExecInfo execInfo;
+ ASSERT_OK(mod.prepare(doc.root(), "", &execInfo));
+
+ ASSERT_EQUALS(execInfo.fieldRef[0]->dottedField(), "a.1.c");
+ ASSERT_TRUE(execInfo.indexOfArrayWithNewElement[0]);
+ ASSERT_EQUALS(*execInfo.indexOfArrayWithNewElement[0], 0u);
+ ASSERT_FALSE(execInfo.noOp);
+}
+
+TEST(BoolInput, PrepareDoNotReportModifiedArrayElement) {
+ Document doc(fromjson("{a: [{b: 0}]}"));
+ Mod mod(fromjson("{$currentDate: {'a.0.c': true}}"));
+
+ ModifierInterface::ExecInfo execInfo;
+ ASSERT_OK(mod.prepare(doc.root(), "", &execInfo));
+
+ ASSERT_EQUALS(execInfo.fieldRef[0]->dottedField(), "a.0.c");
+ ASSERT_FALSE(execInfo.indexOfArrayWithNewElement[0]);
+ ASSERT_FALSE(execInfo.noOp);
+}
+
+TEST(BoolInput, PrepareDoNotReportCreatedNumericObjectField) {
+ Document doc(fromjson("{a: {'0': {b: 0}}}"));
+ Mod mod(fromjson("{$currentDate: {'a.1.c': true}}"));
+
+ ModifierInterface::ExecInfo execInfo;
+ ASSERT_OK(mod.prepare(doc.root(), "", &execInfo));
+
+ ASSERT_EQUALS(execInfo.fieldRef[0]->dottedField(), "a.1.c");
+ ASSERT_FALSE(execInfo.indexOfArrayWithNewElement[0]);
+ ASSERT_FALSE(execInfo.noOp);
+}
+
} // namespace
diff --git a/src/mongo/db/ops/modifier_inc.cpp b/src/mongo/db/ops/modifier_inc.cpp
index ae24613d576..532246bfeef 100644
--- a/src/mongo/db/ops/modifier_inc.cpp
+++ b/src/mongo/db/ops/modifier_inc.cpp
@@ -132,6 +132,8 @@ Status ModifierInc::prepare(mutablebson::Element root,
// be created during the apply.
Status status = pathsupport::findLongestPrefix(
_fieldRef, root, &_preparedState->idxFound, &_preparedState->elemFound);
+ const auto elemFoundIsArray =
+ _preparedState->elemFound.ok() && _preparedState->elemFound.getType() == BSONType::Array;
// FindLongestPrefix may say the path does not exist at all, which is fine here, or
// that the path was not viable or otherwise wrong, in which case, the mod cannot
@@ -163,6 +165,11 @@ Status ModifierInc::prepare(mutablebson::Element root,
if (_mode == MODE_MUL)
_preparedState->newValue *= SafeNum(static_cast<int>(0));
+ if (elemFoundIsArray) {
+ // Report that an existing array will gain a new element as a result of this mod.
+ execInfo->indexOfArrayWithNewElement[0] = _preparedState->idxFound;
+ }
+
return Status::OK();
}
diff --git a/src/mongo/db/ops/modifier_inc_test.cpp b/src/mongo/db/ops/modifier_inc_test.cpp
index c14bf54eacc..65a424878dc 100644
--- a/src/mongo/db/ops/modifier_inc_test.cpp
+++ b/src/mongo/db/ops/modifier_inc_test.cpp
@@ -522,4 +522,41 @@ TEST(Multiplication, ApplyMissingElementDouble) {
ASSERT_EQUALS(mongo::NumberDouble, doc.root().rightChild().getType());
}
+TEST(IndexedMod, PrepareReportCreatedArrayElement) {
+ Document doc(fromjson("{a: [{b: 0}]}"));
+ Mod mod(fromjson("{$inc: {'a.1.c': 2}}"));
+
+ ModifierInterface::ExecInfo execInfo;
+ ASSERT_OK(mod.prepare(doc.root(), "", &execInfo));
+
+ ASSERT_EQUALS(execInfo.fieldRef[0]->dottedField(), "a.1.c");
+ ASSERT_TRUE(execInfo.indexOfArrayWithNewElement[0]);
+ ASSERT_EQUALS(*execInfo.indexOfArrayWithNewElement[0], 0u);
+ ASSERT_FALSE(execInfo.noOp);
+}
+
+TEST(IndexedMod, PrepareDoNotReportModifiedArrayElement) {
+ Document doc(fromjson("{a: [{b: 0}]}"));
+ Mod mod(fromjson("{$inc: {'a.0.c': 2}}"));
+
+ ModifierInterface::ExecInfo execInfo;
+ ASSERT_OK(mod.prepare(doc.root(), "", &execInfo));
+
+ ASSERT_EQUALS(execInfo.fieldRef[0]->dottedField(), "a.0.c");
+ ASSERT_FALSE(execInfo.indexOfArrayWithNewElement[0]);
+ ASSERT_FALSE(execInfo.noOp);
+}
+
+TEST(IndexedMod, PrepareDoNotReportCreatedNumericObjectField) {
+ Document doc(fromjson("{a: {'0': {b: 0}}}"));
+ Mod mod(fromjson("{$inc: {'a.1.c': 2}}"));
+
+ ModifierInterface::ExecInfo execInfo;
+ ASSERT_OK(mod.prepare(doc.root(), "", &execInfo));
+
+ ASSERT_EQUALS(execInfo.fieldRef[0]->dottedField(), "a.1.c");
+ ASSERT_FALSE(execInfo.indexOfArrayWithNewElement[0]);
+ ASSERT_FALSE(execInfo.noOp);
+}
+
} // namespace
diff --git a/src/mongo/db/ops/modifier_interface.h b/src/mongo/db/ops/modifier_interface.h
index 5920acdc798..841d37606d3 100644
--- a/src/mongo/db/ops/modifier_interface.h
+++ b/src/mongo/db/ops/modifier_interface.h
@@ -28,6 +28,8 @@
#pragma once
+#include <boost/optional.hpp>
+
#include "mongo/base/status.h"
#include "mongo/base/string_data.h"
#include "mongo/bson/mutable/element.h"
@@ -190,11 +192,17 @@ struct ModifierInterface::ExecInfo {
ExecInfo() : noOp(false), context(ANY_CONTEXT) {
for (int i = 0; i < MAX_NUM_FIELDS; i++) {
fieldRef[i] = NULL;
+ indexOfArrayWithNewElement[i] = boost::none;
}
}
// The fields of concern to the driver: no other op may modify the fields listed here.
FieldRef* fieldRef[MAX_NUM_FIELDS]; // not owned here
+
+ // For each modified field ref, the index of the path component representing an existing array
+ // that gained a new element.
+ boost::optional<size_t> indexOfArrayWithNewElement[MAX_NUM_FIELDS];
+
bool noOp;
UpdateContext context;
};
diff --git a/src/mongo/db/ops/modifier_push.cpp b/src/mongo/db/ops/modifier_push.cpp
index 31a2bd7f881..c9859dcbee7 100644
--- a/src/mongo/db/ops/modifier_push.cpp
+++ b/src/mongo/db/ops/modifier_push.cpp
@@ -421,6 +421,8 @@ Status ModifierPush::prepare(mutablebson::Element root,
// be created during the apply.
Status status = pathsupport::findLongestPrefix(
_fieldRef, root, &_preparedState->idxFound, &_preparedState->elemFound);
+ const auto elemFoundIsArray =
+ _preparedState->elemFound.ok() && _preparedState->elemFound.getType() == BSONType::Array;
// FindLongestPrefix may say the path does not exist at all, which is fine here, or
// that the path was not viable or otherwise wrong, in which case, the mod cannot
@@ -448,6 +450,13 @@ Status ModifierPush::prepare(mutablebson::Element root,
// there is any conflict among mods.
execInfo->fieldRef[0] = &_fieldRef;
+ if (!_preparedState->elemFound.ok() || _preparedState->idxFound < (_fieldRef.numParts() - 1)) {
+ if (elemFoundIsArray) {
+ // Report that an existing array will gain a new element as a result of this mod.
+ execInfo->indexOfArrayWithNewElement[0] = _preparedState->idxFound;
+ }
+ }
+
return Status::OK();
}
diff --git a/src/mongo/db/ops/modifier_push_test.cpp b/src/mongo/db/ops/modifier_push_test.cpp
index 71424a83b6f..c11c37d72db 100644
--- a/src/mongo/db/ops/modifier_push_test.cpp
+++ b/src/mongo/db/ops/modifier_push_test.cpp
@@ -1439,4 +1439,87 @@ TEST(ToPosition, Back) {
ASSERT_EQUALS(fromjson("{$set: {'a.1':1}}"), logDoc);
}
+TEST(IndexedMod, PrepareReportCreatedArrayElement) {
+ Document doc(fromjson("{a: [{b: 0}]}"));
+ Mod mod(fromjson("{$push: {'a.1.c': 2}}"));
+
+ ModifierInterface::ExecInfo execInfo;
+ ASSERT_OK(mod.prepare(doc.root(), "", &execInfo));
+
+ ASSERT_EQUALS(execInfo.fieldRef[0]->dottedField(), "a.1.c");
+ ASSERT_TRUE(execInfo.indexOfArrayWithNewElement[0]);
+ ASSERT_EQUALS(*execInfo.indexOfArrayWithNewElement[0], 0u);
+ ASSERT_FALSE(execInfo.noOp);
+}
+
+TEST(IndexedMod, PrepareReportCreatedArrayElementPushAll) {
+ Document doc(fromjson("{a: [{b: 0}]}"));
+ auto modObj = fromjson("{$pushAll: {'a.1.c': [2]}}");
+ ModifierPush mod(ModifierPush::PUSH_ALL);
+ ASSERT_OK(mod.init(modObj["$pushAll"].embeddedObject().firstElement(),
+ ModifierInterface::Options::normal()));
+
+ ModifierInterface::ExecInfo execInfo;
+ ASSERT_OK(mod.prepare(doc.root(), "", &execInfo));
+
+ ASSERT_EQUALS(execInfo.fieldRef[0]->dottedField(), "a.1.c");
+ ASSERT_TRUE(execInfo.indexOfArrayWithNewElement[0]);
+ ASSERT_EQUALS(*execInfo.indexOfArrayWithNewElement[0], 0u);
+ ASSERT_FALSE(execInfo.noOp);
+}
+
+TEST(IndexedMod, PrepareDoNotReportModifiedArrayElement) {
+ Document doc(fromjson("{a: [{b: 0}]}"));
+ Mod mod(fromjson("{$push: {'a.0.c': 2}}"));
+
+ ModifierInterface::ExecInfo execInfo;
+ ASSERT_OK(mod.prepare(doc.root(), "", &execInfo));
+
+ ASSERT_EQUALS(execInfo.fieldRef[0]->dottedField(), "a.0.c");
+ ASSERT_FALSE(execInfo.indexOfArrayWithNewElement[0]);
+ ASSERT_FALSE(execInfo.noOp);
+}
+
+TEST(IndexedMod, PrepareDoNotReportModifiedArrayElementPushAll) {
+ Document doc(fromjson("{a: [{b: 0}]}"));
+ auto modObj = fromjson("{$pushAll: {'a.0.c': [2]}}");
+ ModifierPush mod(ModifierPush::PUSH_ALL);
+ ASSERT_OK(mod.init(modObj["$pushAll"].embeddedObject().firstElement(),
+ ModifierInterface::Options::normal()));
+
+ ModifierInterface::ExecInfo execInfo;
+ ASSERT_OK(mod.prepare(doc.root(), "", &execInfo));
+
+ ASSERT_EQUALS(execInfo.fieldRef[0]->dottedField(), "a.0.c");
+ ASSERT_FALSE(execInfo.indexOfArrayWithNewElement[0]);
+ ASSERT_FALSE(execInfo.noOp);
+}
+
+TEST(IndexedMod, PrepareDoNotReportCreatedNumericObjectField) {
+ Document doc(fromjson("{a: {'0': {b: 0}}}"));
+ Mod mod(fromjson("{$push: {'a.1.c': 2}}"));
+
+ ModifierInterface::ExecInfo execInfo;
+ ASSERT_OK(mod.prepare(doc.root(), "", &execInfo));
+
+ ASSERT_EQUALS(execInfo.fieldRef[0]->dottedField(), "a.1.c");
+ ASSERT_FALSE(execInfo.indexOfArrayWithNewElement[0]);
+ ASSERT_FALSE(execInfo.noOp);
+}
+
+TEST(IndexedMod, PrepareDoNotReportCreatedNumericObjectFieldPushAll) {
+ Document doc(fromjson("{a: {'0': {b: 0}}}"));
+ auto modObj = fromjson("{$pushAll: {'a.1.c': [2]}}");
+ ModifierPush mod(ModifierPush::PUSH_ALL);
+ ASSERT_OK(mod.init(modObj["$pushAll"].embeddedObject().firstElement(),
+ ModifierInterface::Options::normal()));
+
+ ModifierInterface::ExecInfo execInfo;
+ ASSERT_OK(mod.prepare(doc.root(), "", &execInfo));
+
+ ASSERT_EQUALS(execInfo.fieldRef[0]->dottedField(), "a.1.c");
+ ASSERT_FALSE(execInfo.indexOfArrayWithNewElement[0]);
+ ASSERT_FALSE(execInfo.noOp);
+}
+
} // unnamed namespace
diff --git a/src/mongo/db/ops/modifier_set.cpp b/src/mongo/db/ops/modifier_set.cpp
index 93f3214a023..0fdc0bdcf44 100644
--- a/src/mongo/db/ops/modifier_set.cpp
+++ b/src/mongo/db/ops/modifier_set.cpp
@@ -126,6 +126,8 @@ Status ModifierSet::prepare(mutablebson::Element root,
// be created during the apply.
Status status = pathsupport::findLongestPrefix(
_fieldRef, root, &_preparedState->idxFound, &_preparedState->elemFound);
+ const auto elemFoundIsArray =
+ _preparedState->elemFound.ok() && _preparedState->elemFound.getType() == BSONType::Array;
// FindLongestPrefix may say the path does not exist at all, which is fine here, or
// that the path was not viable or otherwise wrong, in which case, the mod cannot
@@ -155,6 +157,10 @@ Status ModifierSet::prepare(mutablebson::Element root,
// If the field path is not fully present, then this mod cannot be in place, nor is a
// noOp.
if (!_preparedState->elemFound.ok() || _preparedState->idxFound < (_fieldRef.numParts() - 1)) {
+ if (elemFoundIsArray) {
+ // Report that an existing array will gain a new element as a result of this mod.
+ execInfo->indexOfArrayWithNewElement[0] = _preparedState->idxFound;
+ }
return Status::OK();
}
diff --git a/src/mongo/db/ops/modifier_set_test.cpp b/src/mongo/db/ops/modifier_set_test.cpp
index d5991b52080..ee0df93d76c 100644
--- a/src/mongo/db/ops/modifier_set_test.cpp
+++ b/src/mongo/db/ops/modifier_set_test.cpp
@@ -381,6 +381,43 @@ TEST(IndexedMod, PrepareNonViablePath) {
ASSERT_NOT_OK(setMod.prepare(doc.root(), "", &execInfo));
}
+TEST(IndexedMod, PrepareReportCreatedArrayElement) {
+ Document doc(fromjson("{a: [{b: 0}]}"));
+ Mod setMod(fromjson("{$set: {'a.1.c': 2}}"));
+
+ ModifierInterface::ExecInfo execInfo;
+ ASSERT_OK(setMod.prepare(doc.root(), "", &execInfo));
+
+ ASSERT_EQUALS(execInfo.fieldRef[0]->dottedField(), "a.1.c");
+ ASSERT_TRUE(execInfo.indexOfArrayWithNewElement[0]);
+ ASSERT_EQUALS(*execInfo.indexOfArrayWithNewElement[0], 0u);
+ ASSERT_FALSE(execInfo.noOp);
+}
+
+TEST(IndexedMod, PrepareDoNotReportModifiedArrayElement) {
+ Document doc(fromjson("{a: [{b: 0}]}"));
+ Mod setMod(fromjson("{$set: {'a.0.c': 2}}"));
+
+ ModifierInterface::ExecInfo execInfo;
+ ASSERT_OK(setMod.prepare(doc.root(), "", &execInfo));
+
+ ASSERT_EQUALS(execInfo.fieldRef[0]->dottedField(), "a.0.c");
+ ASSERT_FALSE(execInfo.indexOfArrayWithNewElement[0]);
+ ASSERT_FALSE(execInfo.noOp);
+}
+
+TEST(IndexedMod, PrepareDoNotReportCreatedNumericObjectField) {
+ Document doc(fromjson("{a: {'0': {b: 0}}}"));
+ Mod setMod(fromjson("{$set: {'a.1.c': 2}}"));
+
+ ModifierInterface::ExecInfo execInfo;
+ ASSERT_OK(setMod.prepare(doc.root(), "", &execInfo));
+
+ ASSERT_EQUALS(execInfo.fieldRef[0]->dottedField(), "a.1.c");
+ ASSERT_FALSE(execInfo.indexOfArrayWithNewElement[0]);
+ ASSERT_FALSE(execInfo.noOp);
+}
+
TEST(IndexedMod, PrepareApplyInPlace) {
Document doc(fromjson("{a: [{b: 0},{b: 1},{b: 1}]}"));
Mod setMod(fromjson("{$set: {'a.2.b': 2}}"));
diff --git a/src/mongo/db/ops/update_driver.cpp b/src/mongo/db/ops/update_driver.cpp
index 733bf51a9c8..8edce5a0c2d 100644
--- a/src/mongo/db/ops/update_driver.cpp
+++ b/src/mongo/db/ops/update_driver.cpp
@@ -281,11 +281,21 @@ Status UpdateDriver::update(const StringData& matchedField,
// is not a no-op and it is in a valid context -- then we switch back to a
// non-in-place mode.
//
- // TODO: make mightBeIndexed and fieldRef like each other.
- if (!_affectIndices && !execInfo.noOp && _indexedFields &&
- _indexedFields->mightBeIndexed(execInfo.fieldRef[i]->dottedField())) {
- _affectIndices = true;
- doc->disableInPlaceUpdates();
+ // To determine if indexes are affected: If we did not create a new element in an array,
+ // check whether the full path affects indexes. If we did create a new element in an
+ // array, check whether the array itself might affect any indexes. This is necessary
+ // because if there is an index {"a.b": 1}, and we set "a.1.c" and implicitly create an
+ // array element in "a", then we may need to add a null key to the index {"a.b": 1},
+ // even though "a.1.c" does not appear to affect the index.
+ if (!_affectIndices && !execInfo.noOp && _indexedFields) {
+ auto pathLengthForIndexCheck = execInfo.indexOfArrayWithNewElement[i]
+ ? *execInfo.indexOfArrayWithNewElement[i] + 1
+ : execInfo.fieldRef[i]->numParts();
+ if (_indexedFields->mightBeIndexed(
+ execInfo.fieldRef[i]->dottedSubstring(0, pathLengthForIndexCheck))) {
+ _affectIndices = true;
+ doc->disableInPlaceUpdates();
+ }
}
}