[SERVER-29762] Updating multiple elements in an array produces incorrect results Created: 21/Jun/17  Updated: 30/Oct/23  Resolved: 06/Jul/17

Status: Closed
Project: Core Server
Component/s: Querying
Affects Version/s: None
Fix Version/s: 3.5.10

Type: Bug Priority: Major - P3
Reporter: Max Hirschhorn Assignee: Justin Seyster
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Depends
Related
is related to SERVER-28762 Conditionally parse an update express... Closed
Backwards Compatibility: Fully Compatible
Operating System: ALL
Steps To Reproduce:

python buildscripts/resmoke.py repro_server29762.js

repro_server29762.js

// The featureCompatibilityVersion must be 3.6 in order for the following update to not produce the
// correct results beause otherwise the UpdateDriver doesn't parse expressions into UpdateNodes.
// (See SERVER-28762.)
const res = assert.commandWorked(db.adminCommand({
    getParameter: 1,
    featureCompatibilityVersion: 1,
}));
assert.eq("3.6", res.featureCompatibilityVersion);
 
db.mycoll.drop();
assert.writeOK(db.mycoll.insert({_id: 0, a: []}));
 
// Neither {$set: {"a.2": 2, "a.9": 10}} nor {$set: {"a.0": 2, "a.10": 10}} cause the resulting
// document to become [null, null, ... , null, 10, 2], but the following update does.
assert.writeOK(db.mycoll.update({_id: 0}, {$set: {"a.2": 2, "a.10": 10}}));
 
const arrValue = new Array(11).fill(null);
arrValue[2] = 2;
arrValue[arrValue.length - 1] = 10;
assert.eq({_id: 0, a: arrValue}, db.mycoll.findOne({_id: 0}));

Sprint: Query 2017-07-10
Participants:
Linked BF Score: 0

 Description   

This appears to be the result of the changes from ab165e7 as part of SERVER-28762 and therefore doesn't affect any released version of MongoDB.

Additionally, I think it is worth mentioning that this issue was only detected because the update operation produced a different result (i.e. the correct one) when applied by the secondary despite having the same $set modifier.

{  "ts" : Timestamp(1498018241, 1),  "t" : NumberLong(1),  "h" : NumberLong("-5655460550431900619"),  "v" : 2,  "op" : "c",  "ns" : "test.$cmd",  "wt" : ISODate("2017-06-21T04:10:41.164Z"),  "o" : {  "create" : "mycoll",  "idIndex" : {  "v" : 2,  "key" : {  "_id" : 1 },  "name" : "_id_",  "ns" : "test.mycoll" } } }
{  "ts" : Timestamp(1498018241, 2),  "t" : NumberLong(1),  "h" : NumberLong("8601612499795829594"),  "v" : 2,  "op" : "i",  "ns" : "test.mycoll",  "wt" : ISODate("2017-06-21T04:10:41.164Z"),  "o" : {  "_id" : 0,  "a" : [ ] } }
{  "ts" : Timestamp(1498018241, 3),  "t" : NumberLong(1),  "h" : NumberLong("7416058103814577585"),  "v" : 2,  "op" : "u",  "ns" : "test.mycoll",  "o2" : {  "_id" : 0 },  "wt" : ISODate("2017-06-21T04:10:41.166Z"),  "o" : {  "$set" : {  "a.10" : 10,  "a.2" : 2 } } }



 Comments   
Comment by Githook User [ 06/Jul/17 ]

Author:

{u'username': u'jseyster', u'name': u'Justin Seyster', u'email': u'justin.seyster@mongodb.com'}

Message: SERVER-29762 Look up arrays by index during update.

This patch moves getPositionalPathSpecification() from its location in
document_path_support.h to stringutils.h, where it is renamed to
parseUnsignedBase10Integer(). This function should now be what all
code uses that needs to use a path component to look up an element in
an array.

There are two problems with trying to look up an array element by
string, which this patch addresses (for one particular case). The
first is that it will fail if the path component has leading zeros
(e.g. "a.01.b").

The second problem is that mutable BSON does not maintain the
invariant that array elements have their index as their field name
throughout the entire lifetime of the mutable document. Array lookups
by string fail during the times that this invariant does not hold
(notably, after a mutable array has been extended in length).
Branch: master
https://github.com/mongodb/mongo/commit/3ea2d70f0260b1ec6ec8c60d42d6a6669a802ef2

Comment by Justin Seyster [ 23/Jun/17 ]

max.hirschhorn Thanks for this amazing code snippet! I was able to convert it directly to a unit test, which made it pretty simple to track down the problem.

Interestingly, the reason that we only see this happen when updating array indexes >= 10 is that the bug only triggers when updating array elements in descending numerical order. The new update code applies updates in lexicographic order, so "10" applies before "2" for this case, exposing the problem.

Our update code expects to be able to look up a BSON array element using a string representation of the index. However, the path_support code for adding elements to an array (with padding) does not give the new elements any fields names. (Presumably, each of these elements gets a field name matching its numeric index when the mutablebson gets serialized, but I haven't verified that yet.) Here, the "10" update pads the empty array so that it now contains a "2" element, but when the update attempts to perform a look up on "2" it fails and then erroneously appends to the array instead.

A related problem is that updates that index an array element with a leading zero will also fail, because a lookup on "02" will not find element "2" in an array.

Generated at Thu Feb 08 04:21:45 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.