From f136b4b850ae992b640e21daf3edb4f5a71d8de9 Mon Sep 17 00:00:00 2001 From: Dwight Date: Thu, 18 Jun 2009 13:30:49 -0400 Subject: [PATCH 01/10] towards update unique key fix --- db/btree.h | 10 ++++++++++ db/namespace.h | 3 +++ db/pdfile.cpp | 27 +++++++++++++++++++++++---- 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/db/btree.h b/db/btree.h index e4e8a76a1f9..02b94ccb920 100644 --- a/db/btree.h +++ b/db/btree.h @@ -163,6 +163,12 @@ namespace mongo { public: void dump(); + /* @return true if key exists in index + + order - indicates order of keys in the index. this is basically the index's key pattern, e.g.: + BSONObj order = ((IndexDetails&)idx).keyPattern(); + likewise below in bt_insert() etc. + */ bool exists(const IndexDetails& idx, DiskLoc thisLoc, const BSONObj& key, BSONObj order); static DiskLoc addHead(IndexDetails&); /* start a new index off, empty */ @@ -298,4 +304,8 @@ namespace mongo { #pragma pack() + inline bool IndexDetails::hasKey(const BSONObj& key) { + return head.btree()->exists(*this, head, key, keyPattern()); + } + } // namespace mongo; diff --git a/db/namespace.h b/db/namespace.h index 5fa4e9f30a8..c8481be773b 100644 --- a/db/namespace.h +++ b/db/namespace.h @@ -162,6 +162,9 @@ namespace mongo { return info.obj().getObjectField("key"); } + /* true if the specified key is in the index */ + bool hasKey(const BSONObj& key); + // returns name of this index's storage area // database.table.$index string indexNamespace() const { diff --git a/db/pdfile.cpp b/db/pdfile.cpp index 1806cc20959..c5a1e9a5cba 100644 --- a/db/pdfile.cpp +++ b/db/pdfile.cpp @@ -846,11 +846,18 @@ assert( !eloc.isNull() ); } } - struct IndexChanges { + struct IndexChanges/*on an update*/ { BSONObjSetDefaultOrder oldkeys; BSONObjSetDefaultOrder newkeys; vector removed; // these keys were removed as part of the change vector added; // these keys were added as part of the change + + void dupCheck(IndexDetails& idx) { + if( added.empty() || !idx.unique() ) + return; + for( vector::iterator i = added.begin(); i != added.end(); i++ ) + uassert("E11001 duplicate key on update", !idx.hasKey(**i)); + } }; inline void getIndexChanges(vector& v, NamespaceDetails& d, BSONObj newObj, BSONObj oldObj) { @@ -866,6 +873,13 @@ assert( !eloc.isNull() ); } } + inline void dupCheck(vector& v, NamespaceDetails& d) { + for ( int i = 0; i < d.nIndexes; i++ ) { + IndexDetails& idx = d.indexes[i]; + v[i].dupCheck(idx); + } + } + /** Note: if the object shrinks a lot, we don't free up space, we leave extra at end of the record. */ void DataFileMgr::update( @@ -880,19 +894,23 @@ assert( !eloc.isNull() ); BSONObj objOld(toupdate); BSONObj objNew(buf); BSONElement idOld; + + /* addID - if the udpate lacks an _id field, but the old object has one, we stick it back in. + */ int addID = 0; + { /* xxx duplicate _id check... */ BSONElement idNew; objOld.getObjectID(idOld); if( objNew.getObjectID(idNew) ) { +// /* if( idOld == idNew ) ; else { - /* check if idNew would be a duplicate. very unusual that an _id would change, - so not worried about the bit of extra work here. - */ + // check if idNew would be a duplicate. very unusual that an _id would change, + // so not worried about the bit of extra work here. if( d->findIdIndex() >= 0 ) { BSONObj result; BSONObjBuilder b; @@ -901,6 +919,7 @@ assert( !eloc.isNull() ); !Helpers::findOne(ns, b.done(), result)); } } +// */ } else { if ( !idOld.eoo() ) { /* if the old copy had the _id, force it back into the updated version. insert() adds From 64e8052961075def22e2f6feacf4c462d358c099 Mon Sep 17 00:00:00 2001 From: Dwight Date: Thu, 18 Jun 2009 13:34:01 -0400 Subject: [PATCH 02/10] update unique tests (pending) --- jstests/indexa.js | 1 + jstests/indexb.js | 31 +++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) create mode 100644 jstests/indexb.js diff --git a/jstests/indexa.js b/jstests/indexa.js index 11a332374c9..ba46d2f67d8 100644 --- a/jstests/indexa.js +++ b/jstests/indexa.js @@ -15,5 +15,6 @@ t.update( {x:'B'}, { x:'A' } ); a = t.find().toArray(); u = a.map( function(z){ return z.x } ).unique(); +print("test commented out in indexa.js..."); //assert( a.length == u.length , "unique index update is broken" ); diff --git a/jstests/indexb.js b/jstests/indexb.js new file mode 100644 index 00000000000..78f873e867c --- /dev/null +++ b/jstests/indexb.js @@ -0,0 +1,31 @@ +// unique index test for a case where the object grows +// and must move + +// see indexa.js for the test case for an update with dup id check +// when it doesn't move + + +t = db.indexb;t = db.indexb; +db.dropDatabase(); +t.drop(); +t.ensureIndex({a:1},true); + +t.insert({a:1}); + +x = { a : 2 }; +t.save(x); + +print("test commented out indexb.js"); +if( 0 ) { + +assert( t.count() == 2, "count wrong B"); + +x.a = 1; +x.filler = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; +t.save(x); // should fail, not unique. + +assert( t.count() == 2,"count wrong" ); +assert( t.find({a:1}).count() == 1,"bfail1" ); +assert( t.find({a:2}).count() == 1,"bfail2" ); + +} From 5b8ca64dbb02efe31cdc54b7870b5dc6965ef22a Mon Sep 17 00:00:00 2001 From: Dwight Date: Fri, 19 Jun 2009 13:26:58 -0400 Subject: [PATCH 03/10] fix bug where the feature that ensures _id is preserved on an update had a case that didn't work - test cases added too --- db/pdfile.cpp | 90 +++++++++++++++---------------------------- jstests/uniqueness.js | 17 +++++++- 2 files changed, 46 insertions(+), 61 deletions(-) diff --git a/db/pdfile.cpp b/db/pdfile.cpp index c5a1e9a5cba..06e0c1dad5a 100644 --- a/db/pdfile.cpp +++ b/db/pdfile.cpp @@ -885,23 +885,33 @@ assert( !eloc.isNull() ); void DataFileMgr::update( const char *ns, Record *toupdate, const DiskLoc& dl, - const char *buf, int len, stringstream& ss) + const char *_buf, int _len, stringstream& ss) { dassert( toupdate == dl.rec() ); NamespaceDetails *d = nsdetails(ns); BSONObj objOld(toupdate); - BSONObj objNew(buf); - BSONElement idOld; + BSONObj objNew(_buf); + assert( objNew.objsize() == _len ); + assert( objNew.objdata() == _buf ); - /* addID - if the udpate lacks an _id field, but the old object has one, we stick it back in. - */ - int addID = 0; + if( !objNew.hasElement("_id") && objOld.hasElement("_id") ) { + /* add back the old _id value if the update removes it. Note this implementation is slow + (copies entire object multiple times), but this shouldn't happen often, so going for simple + code, not speed. + */ + BSONObjBuilder b; + BSONElement e; + assert( objOld.getObjectID(e) ); + b.append(e); // put _id first, for best performance + b.appendElements(objNew); + objNew = b.obj(); + } { /* xxx duplicate _id check... */ - + BSONElement idOld; BSONElement idNew; objOld.getObjectID(idOld); if( objNew.getObjectID(idNew) ) { @@ -920,26 +930,27 @@ assert( !eloc.isNull() ); } } // */ - } else { + }/* else { if ( !idOld.eoo() ) { - /* if the old copy had the _id, force it back into the updated version. insert() adds + / if the old copy had the _id, force it back into the updated version. insert() adds one so old should have it unless this is a special table like system.* or local.oplog.* - */ + / addID = len; len += idOld.size(); } - } + }*/ } - if ( toupdate->netLength() < len ) { + if ( toupdate->netLength() < objNew.objsize() ) { // doesn't fit. reallocate ----------------------------------------------------- uassert("E10003 failing update: objects in a capped ns cannot grow", !(d && d->capped)); d->paddingTooSmall(); if ( database->profile ) ss << " moved "; // xxx TODO fix dup key error - old copy should not be deleted + deleteRecord(ns, toupdate, dl); - insert(ns, buf, len, false, idOld); + insert(ns, objNew.objdata(), objNew.objsize(), false); return; } @@ -952,8 +963,8 @@ assert( !eloc.isNull() ); getIndexChanges(changes, *d, objNew, objOld); for ( int x = 0; x < d->nIndexes; x++ ) { IndexDetails& idx = d->indexes[x]; - if ( addID && idx.isIdIndex() ) - continue; + //if ( addID && idx.isIdIndex() ) + // continue; for ( unsigned i = 0; i < changes[x].removed.size(); i++ ) { try { idx.head.btree()->unindex(idx.head, idx, *changes[x].removed[i], dl); @@ -984,55 +995,14 @@ assert( !eloc.isNull() ); } } -/* - BSONObj idxKey = idx.info.obj().getObjectField("key"); - BSONObjSetDefaultOrder oldkeys; - BSONObjSetDefaultOrder newkeys; - idx.getKeysFromObject(oldObj, oldkeys); - idx.getKeysFromObject(newObj, newkeys); - vector removed; - setDifference(oldkeys, newkeys, removed); - string idxns = idx.indexNamespace(); - for ( unsigned i = 0; i < removed.size(); i++ ) { - try { - idx.head.btree()->unindex(idx.head, idx, *removed[i], dl); - } - catch (AssertionException&) { - ss << " exception update unindex "; - problem() << " caught assertion update unindex " << idxns.c_str() << endl; - } - } - vector added; - setDifference(newkeys, oldkeys, added); - assert( !dl.isNull() ); - for ( unsigned i = 0; i < added.size(); i++ ) { - try { - ** TODO xxx dup keys handle ** - idx.head.btree()->bt_insert( - idx.head, - dl, *added[i], idxKey, **dupsAllowed**true, idx); - } - catch (AssertionException&) { - ss << " exception update index "; - out() << " caught assertion update index " << idxns.c_str() << '\n'; - problem() << " caught assertion update index " << idxns.c_str() << endl; - } - } - if ( database->profile ) - ss << '\n' << added.size() << " key updates "; - } - } - } -*/ - // update in place - if ( addID ) { + /*if ( addID ) { ((int&)*toupdate->data) = *((int*) buf) + idOld.size(); memcpy(toupdate->data+4, idOld.rawdata(), idOld.size()); memcpy(toupdate->data+4+idOld.size(), ((char *)buf)+4, addID-4); - } else { - memcpy(toupdate->data, buf, len); - } + } else {*/ + memcpy(toupdate->data, objNew.objdata(), objNew.objsize()); + /*}*/ } int followupExtentSize(int len, int lastExtentLen) { diff --git a/jstests/uniqueness.js b/jstests/uniqueness.js index 9bc9a18df05..456a1b6edcb 100644 --- a/jstests/uniqueness.js +++ b/jstests/uniqueness.js @@ -21,10 +21,25 @@ t.update( { _id : 4 } , { _id : 3, x : 99 } ); assert( db.getLastError() , 4); assert( t.findOne( {_id:4} ), 5 ); -/* Check for an error message when we index and there are dups */ +// Check for an error message when we index and there are dups db.bar.drop(); db.bar.insert({a:3}); db.bar.insert({a:3}); assert( db.bar.count() == 2 , 6) ; db.bar.ensureIndex({a:1}, true); assert( db.getLastError() , 7); + +/* Check that if we update and remove _id, it gets added back by the DB */ + +/* test when object grows */ +t.drop(); +t.save( { _id : 'Z' } ); +t.update( {}, { k : 2 } ); +assert( t.findOne()._id == 'Z', "uniqueness.js problem with adding back _id" ); + +/* test when doesn't grow */ +t.drop(); +t.save( { _id : 'Z', k : 3 } ); +t.update( {}, { k : 2 } ); +assert( t.findOne()._id == 'Z', "uniqueness.js problem with adding back _id (2)" ); + From eb95a15752edf7fd3d27b2427c2c58732be1c22e Mon Sep 17 00:00:00 2001 From: Mike Dirolf Date: Fri, 19 Jun 2009 15:46:53 -0400 Subject: [PATCH 04/10] whitespace --- scripting/engine_spidermonkey.cpp | 280 +++++++++++++++--------------- 1 file changed, 140 insertions(+), 140 deletions(-) diff --git a/scripting/engine_spidermonkey.cpp b/scripting/engine_spidermonkey.cpp index 481be661261..6916f9b7554 100644 --- a/scripting/engine_spidermonkey.cpp +++ b/scripting/engine_spidermonkey.cpp @@ -12,17 +12,17 @@ namespace mongo { #define GETHOLDER(x,o) ((BSONHolder*)JS_GetPrivate( x , o )) class BSONFieldIterator; - + class BSONHolder { public: - + BSONHolder( BSONObj obj ){ _obj = obj.getOwned(); _inResolve = false; _modified = false; _magic = 17; } - + void check(){ uassert( "holder magic value is wrong" , _magic == 17 ); } @@ -38,7 +38,7 @@ namespace mongo { class BSONFieldIterator { public: - + BSONFieldIterator( BSONHolder * holder ){ BSONObjIterator it( holder->_obj ); @@ -46,12 +46,12 @@ namespace mongo { BSONElement e = it.next(); _names.push_back( e.fieldName() ); } - + _names.merge( holder->_extra ); - + _it = _names.begin(); } - + bool more(){ return _it != _names.end(); } @@ -61,7 +61,7 @@ namespace mongo { _it++; return s; } - + private: list _names; list::iterator _it; @@ -77,16 +77,16 @@ namespace mongo { Convertor( JSContext * cx ){ _context = cx; } - + string toString( JSString * so ){ jschar * s = JS_GetStringChars( so ); size_t srclen = JS_GetStringLength( so ); if( srclen == 0 ) return ""; - + size_t len = srclen * 6; // we only need *3, but see note on len below char * dst = (char*)malloc( len ); - + len /= 2; // doc re weird JS_EncodeCharacters api claims len expected in 16bit // units, but experiments suggest 8bit units expected. We allocate @@ -103,7 +103,7 @@ namespace mongo { } string toString( jsval v ){ - return toString( JS_ValueToString( _context , v ) ); + return toString( JS_ValueToString( _context , v ) ); } double toNumber( jsval v ){ @@ -127,11 +127,11 @@ namespace mongo { oid.init( getString( o , "str" ) ); return oid; } - + BSONObj toObject( JSObject * o ){ if ( ! o ) return BSONObj(); - + if ( JS_InstanceOf( _context , o , &bson_ro_class , 0 ) ){ return GETHOLDER( _context , o )->_obj.getOwned(); } @@ -143,14 +143,14 @@ namespace mongo { return holder->_obj; orig = holder->_obj; } - + BSONObjBuilder b; - + jsval theid = getProperty( o , "_id" ); if ( ! JSVAL_IS_VOID( theid ) ){ append( b , "_id" , theid ); } - + JSIdArray * properties = JS_Enumerate( _context , o ); assert( properties ); @@ -161,24 +161,24 @@ namespace mongo { string name = toString( nameval ); if ( name == "_id" ) continue; - + append( b , name , getProperty( o , name.c_str() ) , orig[name].type() ); } - + JS_DestroyIdArray( _context , properties ); return b.obj(); } - + BSONObj toObject( jsval v ){ - if ( JSVAL_IS_NULL( v ) || + if ( JSVAL_IS_NULL( v ) || JSVAL_IS_VOID( v ) ) return BSONObj(); - + uassert( "not an object" , JSVAL_IS_OBJECT( v ) ); return toObject( JSVAL_TO_OBJECT( v ) ); } - + string getFunctionCode( JSFunction * func ){ return toString( JS_DecompileFunction( _context , func , 0 ) ); } @@ -191,10 +191,10 @@ namespace mongo { void append( BSONObjBuilder& b , string name , jsval val , BSONType oldType = EOO ){ //cout << "name: " << name << "\t" << typeString( val ) << " oldType: " << oldType << endl; switch ( JS_TypeOfValue( _context , val ) ){ - + case JSTYPE_VOID: b.appendUndefined( name.c_str() ); break; case JSTYPE_NULL: b.appendNull( name.c_str() ); break; - + case JSTYPE_NUMBER: { double d = toNumber( val ); if ( oldType == NumberInt && ((int)d) == d ) @@ -231,21 +231,21 @@ namespace mongo { b.appendRegex( name.c_str() , s.substr( 0 , end ).c_str() , s.substr( end + 1 ).c_str() ); } else { - b.appendCode( name.c_str() , getFunctionCode( val ).c_str() ); + b.appendCode( name.c_str() , getFunctionCode( val ).c_str() ); } break; } - + default: uassert( (string)"can't append field. name:" + name + " type: " + typeString( val ) , 0 ); } } - + // ---------- to spider monkey --------- bool hasFunctionIdentifier( const string& code ){ if ( code.size() < 9 || code.find( "function" ) != 0 ) return false; - + return code[8] == ' ' || code[8] == '('; } @@ -273,7 +273,7 @@ namespace mongo { addRoot( f ); return f; } - + JSFunction * _compileFunction( const char * code, JSObject * assoc ){ if ( ! hasFunctionIdentifier( code ) ){ string s = code; @@ -282,7 +282,7 @@ namespace mongo { } return JS_CompileFunction( _context , assoc , "anonymous" , 0 , 0 , s.c_str() , strlen( s.c_str() ) , "nofile_a" , 0 ); } - + // TODO: there must be a way in spider monkey to do this - this is a total hack string s = "return "; @@ -294,7 +294,7 @@ namespace mongo { cerr << "compile for hack failed" << endl; return 0; } - + jsval ret; if ( ! JS_CallFunction( _context , 0 , func , 0 , 0 , &ret ) ){ cerr << "call function for hack failed" << endl; @@ -307,7 +307,7 @@ namespace mongo { return JS_ValueToFunction( _context , ret ); } - + jsval toval( double d ){ jsval val; assert( JS_NewNumberValue( _context, d , &val ) ); @@ -326,14 +326,14 @@ namespace mongo { assert( JS_SetPrivate( _context , o , (void*)(new BSONHolder( obj->getOwned() ) ) ) ); return o; } - + jsval toval( const BSONObj* obj , bool readOnly=false ){ JSObject * o = toJSObject( obj , readOnly ); return OBJECT_TO_JSVAL( o ); } jsval toval( const BSONElement& e ){ - + switch( e.type() ){ case EOO: case jstNULL: @@ -352,9 +352,9 @@ namespace mongo { return toval( &embed ); } case Array:{ - + BSONObj embed = e.embeddedObject().getOwned(); - + if ( embed.isEmpty() ){ return OBJECT_TO_JSVAL( JS_NewArrayObject( _context , 0 , 0 ) ); } @@ -364,14 +364,14 @@ namespace mongo { JSObject * array = JS_NewArrayObject( _context , embed.nFields() , 0 ); assert( array ); - + jsval myarray = OBJECT_TO_JSVAL( array ); - + for ( int i=0; imore() ){ string name = it->next(); @@ -540,17 +540,17 @@ namespace mongo { } return JS_TRUE; } - + if ( enum_op == JSENUMERATE_DESTROY ){ - if ( it ) + if ( it ) delete it; return JS_TRUE; } - + uassert( "don't know what to do with this op" , 0 ); return JS_FALSE; } - + JSBool noaccess( JSContext *cx, JSObject *obj, jsval idval, jsval *vp){ BSONHolder * holder = GETHOLDER( cx , obj ); if ( holder->_inResolve ) @@ -558,9 +558,9 @@ namespace mongo { JS_ReportError( cx , "doing write op on read only operation" ); return JS_FALSE; } - + JSClass bson_ro_class = { - "bson_ro_object" , JSCLASS_HAS_PRIVATE | JSCLASS_NEW_RESOLVE | JSCLASS_NEW_ENUMERATE , + "bson_ro_object" , JSCLASS_HAS_PRIVATE | JSCLASS_NEW_RESOLVE | JSCLASS_NEW_ENUMERATE , noaccess, noaccess, JS_PropertyStub, noaccess, (JSEnumerateOp)bson_enumerate, (JSResolveOp)(&resolveBSONField) , JS_ConvertStub, bson_finalize , JSCLASS_NO_OPTIONAL_MEMBERS @@ -576,7 +576,7 @@ namespace mongo { return JS_TRUE; } - + JSBool mark_modified( JSContext *cx, JSObject *obj, jsval idval, jsval *vp){ BSONHolder * holder = GETHOLDER( cx , obj ); if ( holder->_inResolve ) @@ -586,7 +586,7 @@ namespace mongo { } JSClass bson_class = { - "bson_object" , JSCLASS_HAS_PRIVATE | JSCLASS_NEW_RESOLVE | JSCLASS_NEW_ENUMERATE , + "bson_object" , JSCLASS_HAS_PRIVATE | JSCLASS_NEW_RESOLVE | JSCLASS_NEW_ENUMERATE , bson_add_prop, mark_modified, JS_PropertyStub, mark_modified, (JSEnumerateOp)bson_enumerate, (JSResolveOp)(&resolveBSONField) , JS_ConvertStub, bson_finalize , JSCLASS_NO_OPTIONAL_MEMBERS @@ -597,7 +597,7 @@ namespace mongo { JS_PropertyStub, JS_PropertyStub, JS_PropertyStub, JS_PropertyStub, JS_EnumerateStub, JS_ResolveStub, JS_ConvertStub, JS_FinalizeStub, JSCLASS_NO_OPTIONAL_MEMBERS - }; + }; // --- global helpers --- @@ -624,7 +624,7 @@ namespace mongo { } BSONObj out = func( args.obj() ); - + if ( out.isEmpty() ){ *rval = JSVAL_VOID; } @@ -642,27 +642,27 @@ namespace mongo { return JS_TRUE; } - JSFunctionSpec globalHelpers[] = { - { "print" , &native_print , 0 , 0 , 0 } , - { "nativeHelper" , &native_helper , 1 , 0 , 0 } , - { "load" , &native_load , 1 , 0 , 0 } , - { "gc" , &native_gc , 1 , 0 , 0 } , - - { 0 , 0 , 0 , 0 , 0 } + JSFunctionSpec globalHelpers[] = { + { "print" , &native_print , 0 , 0 , 0 } , + { "nativeHelper" , &native_helper , 1 , 0 , 0 } , + { "load" , &native_load , 1 , 0 , 0 } , + { "gc" , &native_gc , 1 , 0 , 0 } , + + { 0 , 0 , 0 , 0 , 0 } }; // ----END global helpers ---- - + JSBool resolveBSONField( JSContext *cx, JSObject *obj, jsval id, uintN flags, JSObject **objp ){ assert( JS_EnterLocalRootScope( cx ) ); Convertor c( cx ); - + BSONHolder * holder = GETHOLDER( cx , obj ); holder->check(); - + string s = c.toString( id ); - + BSONElement e = holder->_obj[ s.c_str() ]; if ( e.type() == EOO ){ @@ -670,7 +670,7 @@ namespace mongo { JS_LeaveLocalRootScope( cx ); return JS_TRUE; } - + jsval val = c.toval( e ); assert( ! holder->_inResolve ); @@ -682,13 +682,13 @@ namespace mongo { JS_LeaveLocalRootScope( cx ); return JS_TRUE; } - + class SMScope; - + class SMEngine : public ScriptEngine { public: - + SMEngine(){ _runtime = JS_NewRuntime(8L * 1024L * 1024L); uassert( "JS_NewRuntime failed" , _runtime ); @@ -703,16 +703,16 @@ namespace mongo { } Scope * createScope(); - + void runTest(); - + virtual bool utf8Ok() const { return JS_CStringsAreUTF8(); } - + private: JSRuntime * _runtime; friend class SMScope; }; - + SMEngine * globalSMEngine; @@ -723,58 +723,58 @@ namespace mongo { // ------ special helpers ------- - + JSBool object_keyset(JSContext *cx, JSObject *obj, uintN argc, jsval *argv, jsval *rval){ - + JSIdArray * properties = JS_Enumerate( cx , obj ); assert( properties ); JSObject * array = JS_NewArrayObject( cx , properties->length , 0 ); assert( array ); - + for ( jsint i=0; ilength; i++ ){ jsid id = properties->vector[i]; jsval idval; assert( JS_IdToValue( cx , id , &idval ) ); assert( JS_SetElement( cx , array , i , &idval ) ); } - + JS_DestroyIdArray( cx , properties ); *rval = OBJECT_TO_JSVAL( array ); return JS_TRUE; } - + // ------ scope ------ JSBool no_gc(JSContext *cx, JSGCStatus status){ return JS_FALSE; } - + class SMScope : public Scope { public: SMScope(){ _context = JS_NewContext( globalSMEngine->_runtime , 8192 ); _convertor = new Convertor( _context ); massert( "JS_NewContext failed" , _context ); - + JS_SetOptions( _context , JSOPTION_VAROBJFIX); //JS_SetVersion( _context , JSVERSION_LATEST); TODO JS_SetErrorReporter( _context , errorReporter ); - + _global = JS_NewObject( _context , &global_class, NULL, NULL); massert( "JS_NewObject failed for global" , _global ); JS_SetGlobalObject( _context , _global ); massert( "js init failed" , JS_InitStandardClasses( _context , _global ) ); - + JS_SetOptions( _context , JS_GetOptions( _context ) | JSOPTION_VAROBJFIX ); JS_DefineFunctions( _context , _global , globalHelpers ); - + // install my special helpers - - assert( JS_DefineFunction( _context , _convertor->getGlobalPrototype( "Object" ) , + + assert( JS_DefineFunction( _context , _convertor->getGlobalPrototype( "Object" ) , "keySet" , object_keyset , 0 , JSPROP_READONLY ) ); _this = 0; @@ -787,7 +787,7 @@ namespace mongo { for ( list::iterator i=_roots.begin(); i != _roots.end(); i++ ){ JS_RemoveRoot( _context , *i ); } - + if ( _this ) JS_RemoveRoot( _context , &_this ); @@ -801,20 +801,20 @@ namespace mongo { _context = 0; } } - + void reset(){ massert( "SMScope::reset() not implemented yet" , 0 ); } - + void addRoot( void * root , const char * name ){ JS_AddNamedRoot( _context , root , name ); _roots.push_back( root ); } - + void init( BSONObj * data ){ if ( ! data ) return; - + BSONObjIterator i( *data ); while ( i.more() ){ BSONElement e = i.next(); @@ -829,18 +829,18 @@ namespace mongo { void localConnect( const char * dbName ){ initMongoJS( this , _context , _global , true ); - + exec( "_mongo = new Mongo();" ); exec( ((string)"db = _mongo.getDB( \"" + dbName + "\" ); ").c_str() ); } - + // ----- getters ------ double getNumber( const char *field ){ jsval val; assert( JS_GetProperty( _context , _global , field , &val ) ); return _convertor->toNumber( val ); } - + string getString( const char *field ){ jsval val; assert( JS_GetProperty( _context , _global , field , &val ) ); @@ -851,7 +851,7 @@ namespace mongo { bool getBoolean( const char *field ){ return _convertor->getBoolean( _global , field ); } - + BSONObj getObject( const char *field ){ return _convertor->toObject( _convertor->getProperty( _global , field ) ); } @@ -863,11 +863,11 @@ namespace mongo { int type( const char *field ){ jsval val; assert( JS_GetProperty( _context , _global , field , &val ) ); - + switch ( JS_TypeOfValue( _context , val ) ){ case JSTYPE_VOID: return Undefined; case JSTYPE_NULL: return jstNULL; - case JSTYPE_OBJECT: { + case JSTYPE_OBJECT: { JSObject * o = JSVAL_TO_OBJECT( val ); if ( JS_IsArrayObject( _context , o ) ) return Array; @@ -884,7 +884,7 @@ namespace mongo { } // ----- setters ------ - + void setNumber( const char *field , double val ){ jsval v = _convertor->toval( val ); assert( JS_SetProperty( _context , _global , field , &v ) ); @@ -902,20 +902,20 @@ namespace mongo { void setBoolean( const char *field , bool val ){ jsval v = BOOLEAN_TO_JSVAL( val ); - assert( JS_SetProperty( _context , _global , field , &v ) ); + assert( JS_SetProperty( _context , _global , field , &v ) ); } - + void setThis( const BSONObj * obj ){ if ( _this ) JS_RemoveRoot( _context , &_this ); - + _this = _convertor->toJSObject( obj ); - + JS_AddNamedRoot( _context , &_this , "scope this" ); } // ---- functions ----- - + ScriptingFunction createFunction( const char * code ){ precall(); return (ScriptingFunction)_convertor->compileFunction( code ); @@ -926,7 +926,7 @@ namespace mongo { boost::posix_time::time_duration timeout; int count; }; - + static JSBool checkTimeout( JSContext *cx, JSScript *script ) { TimeoutSpec &spec = *(TimeoutSpec *)( JS_GetContextPrivate( cx ) ); if ( ++spec.count % 1000 != 0 ) @@ -947,7 +947,7 @@ namespace mongo { spec->count = 0; JS_SetContextPrivate( _context, (void*)spec ); JS_SetBranchCallback( _context, checkTimeout ); - } + } } void uninstallCheckTimeout( int timeoutMs ) { @@ -957,32 +957,32 @@ namespace mongo { JS_SetContextPrivate( _context, 0 ); } } - + void precall(){ _error = ""; currentScope.reset( this ); } - + bool exec( const string& code , const string& name = "(anon)" , bool printResult = false , bool reportError = true , bool assertOnError = true, int timeoutMs = 0 ){ precall(); - + jsval ret = JSVAL_VOID; - + installCheckTimeout( timeoutMs ); JSBool worked = JS_EvaluateScript( _context , _global , code.c_str() , strlen( code.c_str() ) , name.c_str() , 0 , &ret ); uninstallCheckTimeout( timeoutMs ); - + if ( assertOnError ) uassert( name + " exec failed" , worked ); - + if ( reportError && ! _error.empty() ){ // cout << "exec error: " << _error << endl; // already printed in reportError, so... TODO } - + if ( worked ) _convertor->setProperty( _global , "__lastres__" , ret ); - + if ( worked && printResult && ! JSVAL_IS_VOID( ret ) ) cout << _convertor->toString( ret ) << endl; @@ -992,7 +992,7 @@ namespace mongo { int invoke( JSFunction * func , const BSONObj& args, int timeoutMs ){ precall(); jsval rval; - + int nargs = args.nFields(); auto_ptr smargsPtr( new jsval[nargs] ); jsval* smargs = smargsPtr.get(); @@ -1000,13 +1000,13 @@ namespace mongo { BSONObjIterator it( args ); for ( int i=0; itoval( it.next() ); - + setObject( "args" , args , true ); // this is for backwards compatability installCheckTimeout( timeoutMs ); JSBool ret = JS_CallFunction( _context , _this , func , nargs , smargs , &rval ); uninstallCheckTimeout( timeoutMs ); - + if ( !ret ) { return -3; } @@ -1022,7 +1022,7 @@ namespace mongo { void gotError( string s ){ _error = s; } - + string getError(){ return _error; } @@ -1030,11 +1030,11 @@ namespace mongo { void injectNative( const char *field, NativeFunction func ){ string name = field; _convertor->setProperty( _global , (name + "_").c_str() , PRIVATE_TO_JSVAL( func ) ); - + stringstream code; code << field << " = function(){ var a = [ " << field << "_ ]; for ( var i=0; ifilename << ":" << report->lineno; } - + log() << ss.str() << endl; if ( currentScope.get() ){ @@ -1077,21 +1077,21 @@ namespace mongo { for ( uintN i=0; iexecFile( filename , false , true , false ) ){ JS_ReportError( cx , ((string)"error loading file: " + filename ).c_str() ); return JS_FALSE; } } - + return JS_TRUE; } - + void SMEngine::runTest(){ SMScope s; - + s.localConnect( "foo" ); s.exec( "assert( db.getMongo() )" ); @@ -1120,13 +1120,13 @@ namespace mongo { void Convertor::addRoot( JSFunction * f ){ if ( ! f ) return; - + SMScope * scope = currentScope.get(); uassert( "need a scope" , scope ); - + scope->addRoot( f , "cf" ); - } - + } + } #include "sm_db.cpp" From 2671b91d4847bcd1e32000555f979e540c37d621 Mon Sep 17 00:00:00 2001 From: Mike Dirolf Date: Fri, 19 Jun 2009 15:48:32 -0400 Subject: [PATCH 05/10] strip leading whitespace from code before adding return --- scripting/engine_spidermonkey.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/scripting/engine_spidermonkey.cpp b/scripting/engine_spidermonkey.cpp index 6916f9b7554..7c3341f9b76 100644 --- a/scripting/engine_spidermonkey.cpp +++ b/scripting/engine_spidermonkey.cpp @@ -275,6 +275,10 @@ namespace mongo { } JSFunction * _compileFunction( const char * code, JSObject * assoc ){ + while (isspace(*code)) { + code++; + } + if ( ! hasFunctionIdentifier( code ) ){ string s = code; if ( isSimpleStatement( s ) ){ From aaa1769381674b1ae8cf09005f5949f9a78c7141 Mon Sep 17 00:00:00 2001 From: Dwight Date: Fri, 19 Jun 2009 16:03:44 -0400 Subject: [PATCH 06/10] fix bug SERVER-96 update that changes unique index field disregards unique constraint --- db/pdfile.cpp | 44 +++++++------------------------------------ jstests/indexa.js | 8 +++++--- jstests/indexb.js | 17 ++++++++--------- jstests/uniqueness.js | 4 ++-- 4 files changed, 22 insertions(+), 51 deletions(-) diff --git a/db/pdfile.cpp b/db/pdfile.cpp index 06e0c1dad5a..30f49195318 100644 --- a/db/pdfile.cpp +++ b/db/pdfile.cpp @@ -909,38 +909,13 @@ assert( !eloc.isNull() ); objNew = b.obj(); } - { - /* xxx duplicate _id check... */ - BSONElement idOld; - BSONElement idNew; - objOld.getObjectID(idOld); - if( objNew.getObjectID(idNew) ) { -// /* - if( idOld == idNew ) - ; - else { - // check if idNew would be a duplicate. very unusual that an _id would change, - // so not worried about the bit of extra work here. - if( d->findIdIndex() >= 0 ) { - BSONObj result; - BSONObjBuilder b; - b.append(idNew); - uassert("duplicate _id key on update", - !Helpers::findOne(ns, b.done(), result)); - } - } -// */ - }/* else { - if ( !idOld.eoo() ) { - / if the old copy had the _id, force it back into the updated version. insert() adds - one so old should have it unless this is a special table like system.* or - local.oplog.* - / - addID = len; - len += idOld.size(); - } - }*/ - } + /* duplicate key check. we descend the btree twice - once for this check, and once for the actual inserts, further + below. that is suboptimal, but it's pretty complicated to do it the other way without rollbacks... + */ + vector changes; + getIndexChanges(changes, *d, objNew, objOld); + dupCheck(changes, *d); + if ( toupdate->netLength() < objNew.objsize() ) { // doesn't fit. reallocate ----------------------------------------------------- uassert("E10003 failing update: objects in a capped ns cannot grow", !(d && d->capped)); @@ -959,12 +934,8 @@ assert( !eloc.isNull() ); /* have any index keys changed? */ if( d->nIndexes ) { - vector changes; - getIndexChanges(changes, *d, objNew, objOld); for ( int x = 0; x < d->nIndexes; x++ ) { IndexDetails& idx = d->indexes[x]; - //if ( addID && idx.isIdIndex() ) - // continue; for ( unsigned i = 0; i < changes[x].removed.size(); i++ ) { try { idx.head.btree()->unindex(idx.head, idx, *changes[x].removed[i], dl); @@ -1002,7 +973,6 @@ assert( !eloc.isNull() ); memcpy(toupdate->data+4+idOld.size(), ((char *)buf)+4, addID-4); } else {*/ memcpy(toupdate->data, objNew.objdata(), objNew.objsize()); - /*}*/ } int followupExtentSize(int len, int lastExtentLen) { diff --git a/jstests/indexa.js b/jstests/indexa.js index ba46d2f67d8..8e5a31c8062 100644 --- a/jstests/indexa.js +++ b/jstests/indexa.js @@ -1,3 +1,5 @@ +// unique index constraint test for updates +// case where object doesn't grow tested here t = db.indexa; t.drop(); @@ -8,13 +10,13 @@ t.insert( { 'x':'A' } ); t.insert( { 'x':'B' } ); t.insert( { 'x':'A' } ); -assert.eq( 2 , t.count() , "A" ); +assert.eq( 2 , t.count() , "indexa 1" ); t.update( {x:'B'}, { x:'A' } ); a = t.find().toArray(); u = a.map( function(z){ return z.x } ).unique(); +assert.eq( 2 , t.count() , "indexa 2" ); -print("test commented out in indexa.js..."); -//assert( a.length == u.length , "unique index update is broken" ); +assert( a.length == u.length , "unique index update is broken" ); diff --git a/jstests/indexb.js b/jstests/indexb.js index 78f873e867c..5507fee8460 100644 --- a/jstests/indexb.js +++ b/jstests/indexb.js @@ -15,17 +15,16 @@ t.insert({a:1}); x = { a : 2 }; t.save(x); -print("test commented out indexb.js"); -if( 0 ) { +{ -assert( t.count() == 2, "count wrong B"); + assert( t.count() == 2, "count wrong B"); -x.a = 1; -x.filler = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; -t.save(x); // should fail, not unique. + x.a = 1; + x.filler = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + t.save(x); // should fail, not unique. -assert( t.count() == 2,"count wrong" ); -assert( t.find({a:1}).count() == 1,"bfail1" ); -assert( t.find({a:2}).count() == 1,"bfail2" ); + assert( t.count() == 2,"count wrong" ); + assert( t.find({a:1}).count() == 1,"bfail1" ); + assert( t.find({a:2}).count() == 1,"bfail2" ); } diff --git a/jstests/uniqueness.js b/jstests/uniqueness.js index 456a1b6edcb..6d69e4b9d04 100644 --- a/jstests/uniqueness.js +++ b/jstests/uniqueness.js @@ -31,13 +31,13 @@ assert( db.getLastError() , 7); /* Check that if we update and remove _id, it gets added back by the DB */ -/* test when object grows */ +/* - test when object grows */ t.drop(); t.save( { _id : 'Z' } ); t.update( {}, { k : 2 } ); assert( t.findOne()._id == 'Z', "uniqueness.js problem with adding back _id" ); -/* test when doesn't grow */ +/* - test when doesn't grow */ t.drop(); t.save( { _id : 'Z', k : 3 } ); t.update( {}, { k : 2 } ); From 0cd354e6760a9e1911d59282668b533a9b3ae725 Mon Sep 17 00:00:00 2001 From: Eliot Horowitz Date: Sat, 20 Jun 2009 20:19:34 -0400 Subject: [PATCH 07/10] add more logging in verbose mode --- db/instance.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/db/instance.cpp b/db/instance.cpp index 90264303846..817f4f6063a 100644 --- a/db/instance.cpp +++ b/db/instance.cpp @@ -152,7 +152,7 @@ namespace mongo { database = 0; int ms; - bool log = false; + bool log = logLevel >= 1; currentOp.op = curOp = m.data->operation(); #if 0 @@ -523,7 +523,7 @@ namespace mongo { { Timer t; - bool log = false; + bool log = logLevel >= 1; curOp = m.data->operation(); if ( m.data->operation() == dbQuery ) { @@ -575,7 +575,7 @@ namespace mongo { log = log || ctr++ % 128 == 0; if ( log || ms > 100 ) { ss << ' ' << t.millis() << "ms"; - mongo::out() << ss.str().c_str() << endl; + mongo::out() << ss.str().c_str() << endl; } if ( database && database->profile >= 1 ) { if ( database->profile >= 2 || ms >= 100 ) { From 285b43af2503e5ee81cf9d88cc8d40393295333f Mon Sep 17 00:00:00 2001 From: Eliot Horowitz Date: Sat, 20 Jun 2009 21:36:35 -0400 Subject: [PATCH 08/10] DBQuery.map - aka. db.foo.find().map() --- shell/query.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/shell/query.js b/shell/query.js index 3ba17467f8b..1933c68ef98 100644 --- a/shell/query.js +++ b/shell/query.js @@ -173,6 +173,13 @@ DBQuery.prototype.forEach = function( func ){ func( this.next() ); } +DBQuery.prototype.map = function( func ){ + var a = []; + while ( this.hasNext() ) + a.push( func( this.next() ) ); + return a; +} + DBQuery.prototype.arrayAccess = function( idx ){ return this.toArray()[idx]; } From 7fdff9e044891bf09530579d3d8946b87dc29cb5 Mon Sep 17 00:00:00 2001 From: Eliot Horowitz Date: Sat, 20 Jun 2009 21:38:45 -0400 Subject: [PATCH 09/10] test for field filters MINOR --- jstests/find4.js | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 jstests/find4.js diff --git a/jstests/find4.js b/jstests/find4.js new file mode 100644 index 00000000000..17639d3a684 --- /dev/null +++ b/jstests/find4.js @@ -0,0 +1,26 @@ + +t = db.find4; +t.drop(); + +t.save( { a : 1123 , b : 54332 } ); + +o = t.find( {} , {} )[0]; +assert.eq( 1123 , o.a , "A" ); +assert.eq( 54332 , o.b , "B" ); +assert( o._id.str , "C" ); + +o = t.find( {} , { a : 1 } )[0]; +assert.eq( 1123 , o.a , "D" ); +assert( o._id.str , "E" ); +assert( ! o.b , "F" ); + +o = t.find( {} , { b : 1 } )[0]; +assert.eq( 54332 , o.b , "G" ); +assert( o._id.str , "H" ); +assert( ! o.a , "I" ); + +t.drop(); +t.save( { a : 1 , b : 1 } ); +t.save( { a : 2 , b : 2 } ); +assert.eq( "1-1,2-2" , t.find().map( function(z){ return z.a + "-" + z.b } ).toString() ); +assert.eq( "1-undefined,2-undefined" , t.find( {} , { a : 1 }).map( function(z){ return z.a + "-" + z.b } ).toString() ); From d220c42a9908451e7b40e83c90256f22f8cda871 Mon Sep 17 00:00:00 2001 From: Eliot Horowitz Date: Sat, 20 Jun 2009 23:15:31 -0400 Subject: [PATCH 10/10] optimize using field filter --- db/scanandorder.h | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/db/scanandorder.h b/db/scanandorder.h index 061c86ea9a4..9c08e358f16 100644 --- a/db/scanandorder.h +++ b/db/scanandorder.h @@ -50,19 +50,36 @@ namespace mongo { _ response size limit from runquery; push it up a bit. */ - inline bool fillQueryResultFromObj(BufBuilder& b, set *filter, BSONObj& js) { + inline bool fillQueryResultFromObj(BufBuilder& bb, set *filter, BSONObj& js) { if ( filter ) { - BSONObj x; - bool ok = x.addFields(js, *filter) > 0; - if ( ok ) - b.append((void*) x.objdata(), x.objsize()); - return ok; + BSONObjBuilder b( bb ); + BSONObjIterator i( js ); + int N = filter->size(); + int n=0; + bool gotId = false; + while ( i.more() ){ + BSONElement e = i.next(); + const char * fname = e.fieldName(); + + if ( strcmp( fname , "_id" ) == 0 ){ + b.append( e ); + gotId = true; + } + else if ( filter->count( fname ) ){ + b.append( e ); + n++; + if ( n == N && gotId ) + break; + } + } + b.done(); + return n; } - - b.append((void*) js.objdata(), js.objsize()); + + bb.append((void*) js.objdata(), js.objsize()); return true; } - + typedef multimap BestMap; class ScanAndOrder { BestMap best; // key -> full object