Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

SERVER-4777 avoid 'weird case' assertion on modifier update with dupl…

…icate field names by applying updates to the first duplicate field provided by BSONObjIteratorSorted and passing through the remaining duplicates
  • Loading branch information...
commit e5d3da81edd6d094678afa6a93a761389c2b2702 1 parent 1578b86
@astaple astaple authored
View
6 jstests/update3.js
@@ -21,3 +21,9 @@ f.drop();
f.save( {'_id':0} );
f.update( {}, {$set:{'_id':5}} );
assert.eq( 0, f.findOne()._id , "D" );
+
+// Test replacement update of a field with an empty string field name.
+f.drop();
+f.save( {'':0} );
+f.update( {}, {$set:{'':'g'}} );
+assert.eq( 'g', f.findOne()[''] , "E" );
View
25 src/mongo/db/ops/update.cpp
@@ -643,6 +643,14 @@ namespace mongo {
}
+ bool ModSetState::duplicateFieldName( const BSONElement &a, const BSONElement &b ) {
+ return
+ !a.eoo() &&
+ !b.eoo() &&
+ ( a.rawdata() != b.rawdata() ) &&
+ ( a.fieldName() == string( b.fieldName() ) );
+ }
+
template< class Builder >
void ModSetState::createNewFromMods( const string& root , Builder& b , const BSONObj &obj ) {
DEBUGUPDATE( "\t\t createNewFromMods root: " << root );
@@ -655,8 +663,18 @@ namespace mongo {
ModStateHolder::iterator mend = _mods.lower_bound( buf.str() );
set<string> onedownseen;
-
+ BSONElement prevE;
while ( e.type() && m != mend ) {
+
+ if ( duplicateFieldName( prevE, e ) ) {
+ // Just copy through an element with a duplicate field name.
+ b.append( e );
+ prevE = e;
+ e = es.next();
+ continue;
+ }
+ prevE = e;
+
string field = root + e.fieldName();
FieldCompareResult cmp = compareDottedFieldNames( m->second.m->fieldName , field );
@@ -685,10 +703,7 @@ namespace mongo {
m++;
}
else {
- // this is a very weird case
- // have seen it in production, but can't reproduce
- // this assert prevents an inf. loop
- // but likely isn't the correct solution
+ // SERVER-4777
assert(0);
}
continue;
View
3  src/mongo/db/ops/update.h
@@ -642,6 +642,9 @@ namespace mongo {
}
+ /** @return true iff the elements aren't eoo(), are distinct, and share a field name. */
+ static bool duplicateFieldName( const BSONElement &a, const BSONElement &b );
+
public:
bool canApplyInPlace() const {
View
41 src/mongo/dbtests/updatetests.cpp
@@ -519,12 +519,45 @@ namespace UpdateTests {
}
};
+ /** SERVER-4777 */
class TwoModsWithinDuplicatedField : public SetBase {
public:
void run() {
- client().insert( ns(), BSON( "a" << BSONObj() << "a" << BSONObj() ) );
- client().update( ns(), BSONObj(), BSON( "$set" << BSON( "a.b" << 1 << "a.c" << 1 ) ) );
- ASSERT( error() ); // descriptive test, not sure of correct behavior yet SERVER-4777
+ client().insert( ns(), BSON( "_id" << 0 << "a" << 1
+ << "x" << BSONObj() << "x" << BSONObj()
+ << "z" << 5 ) );
+ client().update( ns(), BSONObj(), BSON( "$set" << BSON( "x.b" << 1 << "x.c" << 1 ) ) );
+ ASSERT_EQUALS( BSON( "_id" << 0 << "a" << 1
+ << "x" << BSON( "b" << 1 << "c" << 1 ) << "x" << BSONObj()
+ << "z" << 5 ),
+ client().findOne( ns(), BSONObj() ) );
+ }
+ };
+
+ /** SERVER-4777 */
+ class ThreeModsWithinDuplicatedField : public SetBase {
+ public:
+ void run() {
+ client().insert( ns(),
+ BSON( "_id" << 0
+ << "x" << BSONObj() << "x" << BSONObj() << "x" << BSONObj() ) );
+ client().update( ns(), BSONObj(),
+ BSON( "$set" << BSON( "x.b" << 1 << "x.c" << 1 << "x.d" << 1 ) ) );
+ ASSERT_EQUALS( BSON( "_id" << 0
+ << "x" << BSON( "b" << 1 << "c" << 1 << "d" << 1 )
+ << "x" << BSONObj() << "x" << BSONObj() ),
+ client().findOne( ns(), BSONObj() ) );
+ }
+ };
+
+ class TwoModsBeforeExistingField : public SetBase {
+ public:
+ void run() {
+ client().insert( ns(), BSON( "_id" << 0 << "x" << 5 ) );
+ client().update( ns(), BSONObj(),
+ BSON( "$set" << BSON( "a" << 1 << "b" << 1 << "x" << 10 ) ) );
+ ASSERT_EQUALS( BSON( "_id" << 0 << "a" << 1 << "b" << 1 << "x" << 10 ),
+ client().findOne( ns(), BSONObj() ) );
}
};
@@ -864,6 +897,8 @@ namespace UpdateTests {
add< CheckNoMods >();
add< UpdateMissingToNull >();
add< TwoModsWithinDuplicatedField >();
+ add< ThreeModsWithinDuplicatedField >();
+ add< TwoModsBeforeExistingField >();
add< ModSetTests::internal1 >();
add< ModSetTests::inc1 >();
Please sign in to comment.
Something went wrong with that request. Please try again.