summaryrefslogtreecommitdiff
path: root/src/mongo/db/update
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-06 17:16:24 -0500
commit69f94d7172801725bad43dba99ca0e5400eb293c (patch)
tree6fe89c9692b68966ed1c3b20d990c0b61e4bf45c /src/mongo/db/update
parent19f93e1015bc177c5f5578a0890718d18926b7d7 (diff)
downloadmongo-69f94d7172801725bad43dba99ca0e5400eb293c.tar.gz
SERVER-32048 Ensure updates that implicitly create an array element generate new null index keys
Diffstat (limited to 'src/mongo/db/update')
-rw-r--r--src/mongo/db/update/modifier_node.cpp12
-rw-r--r--src/mongo/db/update/set_node_test.cpp58
-rw-r--r--src/mongo/db/update/update_driver.cpp20
3 files changed, 83 insertions, 7 deletions
diff --git a/src/mongo/db/update/modifier_node.cpp b/src/mongo/db/update/modifier_node.cpp
index 82b66014a51..a85b636ac0d 100644
--- a/src/mongo/db/update/modifier_node.cpp
+++ b/src/mongo/db/update/modifier_node.cpp
@@ -280,8 +280,16 @@ UpdateNode::ApplyResult ModifierNode::applyToNonexistentElement(ApplyParams appl
ApplyResult applyResult;
- // Determine if indexes are affected.
- if (!applyParams.indexData || !applyParams.indexData->mightBeIndexed(fullPath)) {
+ // 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, even though "a.1.c" does not appear to
+ // affect the index.
+ if (!applyParams.indexData ||
+ !applyParams.indexData->mightBeIndexed(applyParams.element.getType() != BSONType::Array
+ ? fullPath
+ : applyParams.pathTaken->dottedField())) {
applyResult.indexesAffected = false;
}
diff --git a/src/mongo/db/update/set_node_test.cpp b/src/mongo/db/update/set_node_test.cpp
index 5c4bcf9be3d..03b354041b8 100644
--- a/src/mongo/db/update/set_node_test.cpp
+++ b/src/mongo/db/update/set_node_test.cpp
@@ -1275,6 +1275,64 @@ TEST_F(SetNodeTest, ApplyCanCreatePrefixOfImmutablePath) {
ASSERT_EQUALS(fromjson("{$set: {a: 2}}"), getLogDoc());
}
+TEST_F(SetNodeTest, ApplySetFieldInNonExistentArrayElementAffectsIndexOnSiblingField) {
+ auto update = fromjson("{$set: {'a.1.c': 2}}");
+ boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest());
+ SetNode node;
+ ASSERT_OK(node.init(update["$set"]["a.1.c"], expCtx));
+
+ mutablebson::Document doc(fromjson("{a: [{b: 0}]}"));
+ setPathToCreate("1.c");
+ setPathTaken("a");
+ addIndexedPath("a.b");
+ auto result = node.apply(getApplyParams(doc.root()["a"]));
+ ASSERT_FALSE(result.noop);
+ ASSERT_TRUE(result.indexesAffected);
+ ASSERT_EQUALS(fromjson("{a: [{b: 0}, {c: 2}]}"), doc);
+ ASSERT_FALSE(doc.isInPlaceModeEnabled());
+ ASSERT_EQUALS(countChildren(getLogDoc().root()), 1u);
+ ASSERT_EQUALS(fromjson("{$set: {'a.1.c': 2}}"), getLogDoc());
+}
+
+TEST_F(SetNodeTest, ApplySetFieldInExistingArrayElementDoesNotAffectIndexOnSiblingField) {
+ auto update = fromjson("{$set: {'a.0.c': 2}}");
+ boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest());
+ SetNode node;
+ ASSERT_OK(node.init(update["$set"]["a.0.c"], expCtx));
+
+ mutablebson::Document doc(fromjson("{a: [{b: 0}]}"));
+ setPathToCreate("c");
+ setPathTaken("a.0");
+ addIndexedPath("a.b");
+ auto result = node.apply(getApplyParams(doc.root()["a"][0]));
+ ASSERT_FALSE(result.noop);
+ ASSERT_FALSE(result.indexesAffected);
+ ASSERT_EQUALS(fromjson("{a: [{b: 0, c: 2}]}"), doc);
+ ASSERT_FALSE(doc.isInPlaceModeEnabled());
+ ASSERT_EQUALS(countChildren(getLogDoc().root()), 1u);
+ ASSERT_EQUALS(fromjson("{$set: {'a.0.c': 2}}"), getLogDoc());
+}
+
+TEST_F(SetNodeTest, ApplySetFieldInNonExistentNumericFieldDoesNotAffectIndexOnSiblingField) {
+ auto update = fromjson("{$set: {'a.1.c': 2}}");
+ boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest());
+ SetNode node;
+ ASSERT_OK(node.init(update["$set"]["a.1.c"], expCtx));
+
+ mutablebson::Document doc(fromjson("{a: {'0': {b: 0}}}"));
+ setPathToCreate("1.c");
+ setPathTaken("a");
+ addIndexedPath("a.b");
+ addIndexedPath("a.1.b");
+ auto result = node.apply(getApplyParams(doc.root()["a"]));
+ ASSERT_FALSE(result.noop);
+ ASSERT_FALSE(result.indexesAffected);
+ ASSERT_EQUALS(fromjson("{a: {'0': {b: 0}, '1': {c: 2}}}"), doc);
+ ASSERT_FALSE(doc.isInPlaceModeEnabled());
+ ASSERT_EQUALS(countChildren(getLogDoc().root()), 1u);
+ ASSERT_EQUALS(fromjson("{$set: {'a.1.c': 2}}"), getLogDoc());
+}
+
TEST_F(SetNodeTest, ApplySetOnInsertIsNoopWhenInsertIsFalse) {
auto update = fromjson("{$setOnInsert: {a: 2}}");
boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest());
diff --git a/src/mongo/db/update/update_driver.cpp b/src/mongo/db/update/update_driver.cpp
index 7ae41480779..88d80341981 100644
--- a/src/mongo/db/update/update_driver.cpp
+++ b/src/mongo/db/update/update_driver.cpp
@@ -414,11 +414,21 @@ Status UpdateDriver::update(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();
+ }
}
}