diff options
-rw-r--r-- | jstests/core/rename_change_target_type.js | 15 | ||||
-rw-r--r-- | src/mongo/db/update/rename_node.cpp | 20 | ||||
-rw-r--r-- | src/mongo/db/update/rename_node_test.cpp | 18 |
3 files changed, 39 insertions, 14 deletions
diff --git a/jstests/core/rename_change_target_type.js b/jstests/core/rename_change_target_type.js new file mode 100644 index 00000000000..25fbcfb0f01 --- /dev/null +++ b/jstests/core/rename_change_target_type.js @@ -0,0 +1,15 @@ +// Test that a rename that overwrites its destination with an equivalent value of a different type +// updates the type of the destination (SERVER-32109). +(function() { + "use strict"; + + let coll = db.rename_change_target_type; + coll.drop(); + + assert.writeOK(coll.insert({to: NumberLong(100), from: 100})); + assert.writeOK(coll.update({}, {$rename: {from: "to"}})); + + let aggResult = coll.aggregate([{$project: {toType: {$type: "$to"}}}]).toArray(); + assert.eq(aggResult.length, 1); + assert.eq(aggResult[0].toType, "double", "Incorrect type resulting from $rename"); +})(); diff --git a/src/mongo/db/update/rename_node.cpp b/src/mongo/db/update/rename_node.cpp index bae93e460de..3a725218763 100644 --- a/src/mongo/db/update/rename_node.cpp +++ b/src/mongo/db/update/rename_node.cpp @@ -48,7 +48,11 @@ namespace { * SetElementNode object. We create a class for this purpose (rather than a stand-alone function) so * that it can inherit from ModifierNode. * - * Unlike SetNode, SetElementNode takes a mutablebson::Element as its input. + * Unlike SetNode, SetElementNode takes a mutablebson::Element as its input. Additionally, + * SetElementNode::updateExistingElement() does not check for the possibility that we are + * overwriting the target value with an identical source value (a no-op). That check would require + * us to convert _elemToSet from a mutablebson::Element to a BSONElement, which is not worth the + * extra time. */ class SetElementNode : public ModifierNode { public: @@ -67,18 +71,8 @@ public: protected: ModifierNode::ModifyResult updateExistingElement( mutablebson::Element* element, std::shared_ptr<FieldRef> elementPath) const final { - // In the case of a $rename where the source and destination have the same value, (e.g., we - // are applying {$rename: {a: b}} to the document {a: "foo", b: "foo"}), there's no need to - // modify the destination element. However, the source and destination values must be - // _exactly_ the same, which is why we do not use collation for this check. - StringData::ComparatorInterface* comparator = nullptr; - auto considerFieldName = false; - if (_elemToSet.compareWithElement(*element, comparator, considerFieldName) != 0) { - invariantOK(element->setValueElement(_elemToSet)); - return ModifyResult::kNormalUpdate; - } else { - return ModifyResult::kNoOp; - } + invariantOK(element->setValueElement(_elemToSet)); + return ModifyResult::kNormalUpdate; } void setValueForNewElement(mutablebson::Element* element) const final { diff --git a/src/mongo/db/update/rename_node_test.cpp b/src/mongo/db/update/rename_node_test.cpp index 103ca7f69a8..dc58e1c8174 100644 --- a/src/mongo/db/update/rename_node_test.cpp +++ b/src/mongo/db/update/rename_node_test.cpp @@ -154,7 +154,23 @@ TEST_F(RenameNodeTest, ToAndFromHaveSameValue) { ASSERT_FALSE(result.noop); ASSERT_TRUE(result.indexesAffected); ASSERT_EQUALS(fromjson("{b: 2}"), doc); - ASSERT_EQUALS(fromjson("{$unset: {a: true}}"), getLogDoc()); + ASSERT_EQUALS(fromjson("{$set: {b: 2}, $unset: {a: true}}"), getLogDoc()); +} + +TEST_F(RenameNodeTest, RenameToFieldWithSameValueButDifferentType) { + auto update = fromjson("{$rename: {a: 'b'}}"); + boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); + RenameNode node; + ASSERT_OK(node.init(update["$rename"]["a"], expCtx)); + + mutablebson::Document doc(fromjson("{a: 1, b: NumberLong(1)}")); + setPathTaken("b"); + addIndexedPath("a"); + auto result = node.apply(getApplyParams(doc.root()["b"])); + ASSERT_FALSE(result.noop); + ASSERT_TRUE(result.indexesAffected); + ASSERT_EQUALS(fromjson("{b: 1}"), doc); + ASSERT_EQUALS(fromjson("{$set: {b: 1}, $unset: {a: true}}"), getLogDoc()); } TEST_F(RenameNodeTest, FromDottedElement) { |