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 6477fd1d758682aadd0ef9789e449b85cd8be241 1 parent 672793c
@astaple astaple authored andy10gen committed
View
28 db/ops/update.cpp
@@ -642,6 +642,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 );
@@ -654,8 +662,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 );
@@ -684,11 +702,9 @@ 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
- assert(0);
+ massert( 16062 , "ModSet::createNewFromMods - "
+ "SERVER-4777 unhandled duplicate field" , 0 );
+
}
continue;
}
View
3  db/ops/update.h
@@ -623,6 +623,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
45 dbtests/updatetests.cpp
@@ -519,6 +519,48 @@ namespace UpdateTests {
}
};
+ /** SERVER-4777 */
+ class TwoModsWithinDuplicatedField : public SetBase {
+ public:
+ void run() {
+ 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() ) );
+ }
+ };
+
namespace ModSetTests {
class internal1 {
@@ -854,6 +896,9 @@ namespace UpdateTests {
add< PreserveIdWithIndex >();
add< CheckNoMods >();
add< UpdateMissingToNull >();
+ add< TwoModsWithinDuplicatedField >();
+ add< ThreeModsWithinDuplicatedField >();
+ add< TwoModsBeforeExistingField >();
add< ModSetTests::internal1 >();
add< ModSetTests::inc1 >();
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" );
Please sign in to comment.
Something went wrong with that request. Please try again.