SERVER-4150 reimplement query using query optimizer cursor one checkpoint

12 files changed, 450 insertions, 54 deletions

diff --git a/src/mongo/db/clientcursor.h b/src/mongo/db/clientcursor.h
index 0cba294be35..0022a9e6190 100644
--- a/src/mongo/db/clientcursor.h
+++ b/src/mongo/db/clientcursor.h
@@ -139,6 +139,10 @@ namespace mongo {
             }
             operator bool() { return _c; }
             ClientCursor * operator-> () { return _c; }
+            void release() {
+                _c = 0;
+                _id = -1;
+            }
         private:
             ClientCursor *_c;
             CursorId _id;
diff --git a/src/mongo/db/namespace.h b/src/mongo/db/namespace.h
index e54a5313d26..49fd48d873f 100644
--- a/src/mongo/db/namespace.h
+++ b/src/mongo/db/namespace.h
@@ -472,7 +472,8 @@ namespace mongo {
                                             const BSONObj &order = BSONObj(),
                                             const QueryPlanSelectionPolicy &planPolicy =
                                             QueryPlanSelectionPolicy::any(),
-                                            bool *simpleEqualityMatch = 0 );
+                                            bool *simpleEqualityMatch = 0,
+                                            const ParsedQuery *parsedQuery = 0 );
 
         /**
          * @return a single cursor that may work well for the given query.
@@ -481,7 +482,7 @@ namespace mongo {
          * no suitable indices exist.
          */
         static shared_ptr<Cursor> bestGuessCursor( const char *ns, const BSONObj &query, const BSONObj &sort );
-        
+
         /* indexKeys() cache ---------------------------------------------------- */
         /* assumed to be in write lock for this */
     private:
diff --git a/src/mongo/db/oplog.cpp b/src/mongo/db/oplog.cpp
index 1332887b58f..9f3c064f568 100644
--- a/src/mongo/db/oplog.cpp
+++ b/src/mongo/db/oplog.cpp
@@ -536,6 +536,24 @@ namespace mongo {
         _findingStartMode = Initial;
     }
     
+    shared_ptr<Cursor> FindingStartCursor::getCursor( const char *ns, const BSONObj &query, const BSONObj &order ) {
+        NamespaceDetails *d = nsdetails(ns);
+        assert(d); // !!! what if ns not present
+        FieldRangeSetPair frsp = * new FieldRangeSetPair( ns, query );
+        QueryPlan &oplogPlan = * new QueryPlan( d, -1, frsp, 0, query, order, false ); // cursor isn't going to own this, make memory ownership ok/clearer
+        FindingStartCursor finder( oplogPlan );
+        while( !finder.done() ) {
+//            RARELY { // !!! want multiple to yield simultaneously
+//                if ( finder.prepareToYield() ) {
+//                    ClientCursor::staticYield( -1, ns, 0 );
+//                    finder.recoverFromYield();
+//                }
+//            }
+            finder.next();
+        }
+        return finder.cursor();
+    }
+    
     // -------------------------------------
 
     struct TestOpTime : public UnitTest {
diff --git a/src/mongo/db/oplog.h b/src/mongo/db/oplog.h
index 80ec2004f3c..bdf6f0c79be 100644
--- a/src/mongo/db/oplog.h
+++ b/src/mongo/db/oplog.h
@@ -101,6 +101,9 @@ namespace mongo {
                 }
             }
         }
+        
+        static shared_ptr<Cursor> getCursor( const char *ns, const BSONObj &query, const BSONObj &order );
+
     private:
         enum FindingStartMode { Initial, FindExtent, InExtent };
         const QueryPlan &_qp;
diff --git a/src/mongo/db/ops/query.cpp b/src/mongo/db/ops/query.cpp
index d46bb7076b0..2a1aedd20ed 100644
--- a/src/mongo/db/ops/query.cpp
+++ b/src/mongo/db/ops/query.cpp
@@ -646,6 +646,32 @@ namespace mongo {
         bool _yieldRecoveryFailed;
     };
 
+    class QueryResponseBuilder {
+    public:
+        QueryResponseBuilder( const ParsedQuery &parsedQuery ) : _buf(32768), _parsedQuery( parsedQuery ), _n() {
+            _buf.skip( sizeof( QueryResult ) );
+        }
+        void addResult( const DiskLoc &loc ) {
+            fillQueryResultFromObj( _buf, _parsedQuery.getFields(), loc.obj(), ( _parsedQuery.showDiskLoc() ? &loc : 0 ) );
+        }
+        bool enoughTotalResults() {
+            return ( _parsedQuery.enough( _n ) || _buf.len() >= MaxBytesToReturnToClientAtOnce );
+        }
+        bool enoughForFirstBatch() {
+            return _parsedQuery.enoughForFirstBatch( _n, _buf.len() );
+        }
+        void handoff( Message &result ) {
+            if ( _buf.len() > 0 ) {
+                result.appendData( _buf.buf(), _buf.len() );
+                _buf.decouple();
+            }
+        }
+    private:
+        BufBuilder _buf;
+        const ParsedQuery &_parsedQuery;
+        long long _n;
+    };
+    
     /* run a query -- includes checking for and running a Command \
        @return points to ns if exhaust mode. 0=normal mode
     */
@@ -782,6 +808,98 @@ namespace mongo {
                 return NULL;
             }
         }
+        
+        {
+            shared_ptr<Cursor> cursor;
+            if ( pq.hasOption( QueryOption_OplogReplay ) ) {
+//                cursor = FindingStartCursor::getCursor( ns, query, order );
+            } else if ( order.isEmpty() && !pq.getFields() && !pq.isExplain() && !pq.returnKey() ) {
+                cursor = NamespaceDetailsTransient::getCursor( ns, query, BSONObj(), &pq );
+            }
+            if ( cursor ) {
+                QueryResponseBuilder queryResponseBuilder( pq );
+//                long long nscanned = 0;
+                long long skip = pq.getSkip();
+                long long cursorid = 0;
+                OpTime slaveReadTill;
+                ClientCursor::CleanupPointer ccPointer;
+                ccPointer.reset( new ClientCursor( QueryOption_NoCursorTimeout, cursor, ns ) );
+                while( cursor->ok() ) {
+//                    if ( !ccPointer->yieldSometimes( ClientCursor::MaybeCovered ) || !cursor->ok() ) {
+//                        break;
+//                    }
+
+                    if ( pq.getMaxScan() && cursor->nscanned() > pq.getMaxScan() ) {
+                        break;
+                    }
+                    
+                    if ( cursor->matcher() && !cursor->matcher()->matchesCurrent( cursor.get() ) ) {
+                        cursor->advance();
+                        continue;
+                    }
+                    
+                    DiskLoc currLoc = cursor->currLoc();
+                    if ( cursor->getsetdup( currLoc ) ) {
+                        cursor->advance();
+                        continue;
+                    }
+                    
+                    if ( skip > 0 ) {
+                        --skip;
+                        cursor->advance();
+                        continue;
+                    }
+
+                    BSONObj js = cursor->current();
+                    assert( js.isValid() );
+                    
+                    if ( pq.hasOption( QueryOption_OplogReplay ) ) {
+                        BSONElement e = js["ts"];
+                        if ( e.type() == Date || e.type() == Timestamp ) {
+                            slaveReadTill = e._opTime();
+                        }
+                    }
+                    
+                    queryResponseBuilder.addResult( currLoc );
+                    if ( !cursor->supportGetMore() ) {
+                        if ( queryResponseBuilder.enoughTotalResults() ) {
+                            log() << "enough n:" << n << endl;
+                            break;
+                        }
+                    }
+                    else if ( queryResponseBuilder.enoughForFirstBatch() ) {
+                        log() << "enough for first" << endl;
+                        /* if only 1 requested, no cursor saved for efficiency...we assume it is findOne() */
+                        if ( pq.wantMore() && pq.getNumToReturn() != 1 && useCursors ) {
+                            log() << "setting cursorid" << endl;
+                            cursor->advance();
+                            cursorid = ccPointer->cursorid();
+                        }
+                        break;
+                    }
+                    
+                    cursor->advance();
+                }
+                if ( cursorid == 0 ) {
+//                    ccPointer.reset();
+                    ccPointer.release();
+                } else {
+                    log() << "updating location" << endl;
+                    ccPointer->updateLocation();
+                    ccPointer.release();
+                }
+                queryResponseBuilder.handoff( result );
+                QueryResult *qr = (QueryResult *) result.header();
+                qr->cursorId = cursorid;
+                qr->setResultFlagsToOk();
+                // qr->len is updated automatically by appendData()
+                curop.debug().responseLength = qr->len;
+                qr->setOperation(opReply);
+                qr->startingFrom = 0;
+                qr->nReturned = n;
+                return 0;
+            }
+        }
 
         // regular, not QO bypass query
 
diff --git a/src/mongo/db/ops/query.h b/src/mongo/db/ops/query.h
index 8097133c6fc..8080fcd9d78 100644
--- a/src/mongo/db/ops/query.h
+++ b/src/mongo/db/ops/query.h
@@ -23,15 +23,12 @@
 #include "../dbmessage.h"
 #include "../jsobj.h"
 #include "../diskloc.h"
-#include "../projection.h"
 
 // struct QueryOptions, QueryResult, QueryResultFlags in:
 #include "../../client/dbclient.h"
 
 namespace mongo {
 
-    extern const int MaxBytesToReturnToClientAtOnce;
-
     QueryResult* processGetMore(const char *ns, int ntoreturn, long long cursorid , CurOp& op, int pass, bool& exhaust);
 
     const char * runQuery(Message& m, QueryMessage& q, CurOp& curop, Message &result);
diff --git a/src/mongo/db/queryoptimizer.cpp b/src/mongo/db/queryoptimizer.cpp
index 08089e5d6bd..9c5fae5d050 100644
--- a/src/mongo/db/queryoptimizer.cpp
+++ b/src/mongo/db/queryoptimizer.cpp
@@ -579,6 +579,7 @@ doneCheckOrder:
 
         // Table scan plan
         addPlan( QueryPlanPtr( new QueryPlan( d, -1, *_frsp, _originalFrsp.get(), _originalQuery, _order, _mustAssertOnYieldFailure ) ), checkFirst );
+
         _mayRecordPlan = true;
     }
 
@@ -1039,7 +1040,10 @@ doneCheckOrder:
     }
 
     const QueryPlan *MultiPlanScanner::singlePlan() const {
-        if ( _or || _currentQps->nPlans() != 1 || _currentQps->firstPlan()->scanAndOrderRequired() || _currentQps->usingCachedPlan() ) {
+        if ( _or ||
+            _currentQps->nPlans() != 1 ||
+            _currentQps->firstPlan()->scanAndOrderRequired() ||
+            _currentQps->usingCachedPlan() ) {
             return 0;
         }
         return _currentQps->firstPlan().get();
diff --git a/src/mongo/db/queryoptimizercursor.cpp b/src/mongo/db/queryoptimizercursor.cpp
index a2d191a741e..ef42b2453e1 100644
--- a/src/mongo/db/queryoptimizercursor.cpp
+++ b/src/mongo/db/queryoptimizercursor.cpp
@@ -25,6 +25,8 @@
 
 namespace mongo {
     
+    extern bool useHints;
+    
     static const int OutOfOrderDocumentsAssertionCode = 14810;
         
     QueryPlanSelectionPolicy::Any QueryPlanSelectionPolicy::__any;
@@ -149,6 +151,7 @@ namespace mongo {
         }
         
         virtual void next() {
+
             mayAdvance();
             
             if ( _matchCounter.enoughCumulativeMatchesToChooseAPlan() ) {
@@ -307,7 +310,7 @@ namespace mongo {
             return _currOp->cursor()->indexKeyPattern();
         }
         
-        virtual bool supportGetMore() { return false; }
+        virtual bool supportGetMore() { return true; }
 
         virtual bool supportYields() { return _takeover ? _takeover->supportYields() : true; }
         
@@ -453,6 +456,7 @@ namespace mongo {
                 _currOp = qocop;
             }
             else if ( op->stopRequested() ) {
+                log() << "stop requested" << endl;
                 if ( qocop->cursor() ) {
                     // Ensure that prepareToTouchEarlierIterate() may be called safely when a BasicCursor takes over.
                     if ( !prevLoc.isNull() && prevLoc == qocop->currLoc() ) {
@@ -520,7 +524,8 @@ namespace mongo {
                                                             const BSONObj &order,
                                                             const QueryPlanSelectionPolicy
                                                             &planPolicy,
-                                                            bool *simpleEqualityMatch ) {
+                                                            bool *simpleEqualityMatch,
+                                                            const ParsedQuery *parsedQuery ) {
         if ( simpleEqualityMatch ) {
             *simpleEqualityMatch = false;
         }
@@ -528,22 +533,54 @@ namespace mongo {
             // TODO This will not use a covered index currently.
             return theDataFileMgr.findAll( ns );
         }
-        if ( planPolicy.permitOptimalIdPlan() && isSimpleIdQuery( query ) ) {
-            Database *database = cc().database();
-            verify( 15985, database );
-            NamespaceDetails *d = database->namespaceIndex.details(ns);
+
+        BSONElement hint = (useHints && parsedQuery) ? parsedQuery->getHint() : BSONElement();
+        bool snapshot = parsedQuery && parsedQuery->isSnapshot();
+
+        BSONObj snapshotHint; // put here to keep the data in scope
+        if( snapshot ) {
+            NamespaceDetails *d = nsdetails(ns);
             if ( d ) {
-                int idxNo = d->findIdIndex();
-                if ( idxNo >= 0 ) {
-                    IndexDetails& i = d->idx( idxNo );
-                    BSONObj key = i.getKeyFromQuery( query );
-                    return shared_ptr<Cursor>( BtreeCursor::make( d, idxNo, i, key, key, true, 1 ) );
+                int i = d->findIdIndex();
+                if( i < 0 ) {
+                    if ( strstr( ns , ".system." ) == 0 )
+                        log() << "warning: no _id index on $snapshot query, ns:" << ns << endl;
+                }
+                else {
+                    /* [dm] the name of an _id index tends to vary, so we build the hint the hard way here.
+                     probably need a better way to specify "use the _id index" as a hint.  if someone is
+                     in the query optimizer please fix this then!
+                     */
+                    BSONObjBuilder b;
+                    b.append("$hint", d->idx(i).indexName());
+                    snapshotHint = b.obj();
+                    hint = snapshotHint.firstElement();
+                }
+            }
+        }
+
+        bool mayShortcutQueryOptimizer = !parsedQuery || ( parsedQuery->getMin().isEmpty() && parsedQuery->getMax().isEmpty() );
+        if ( mayShortcutQueryOptimizer ) {
+            if ( query.isEmpty() && order.isEmpty() ) {
+                // TODO This will not use a covered index currently.
+                return theDataFileMgr.findAll( ns );
+            }
+            if ( planPolicy.permitOptimalIdPlan() && isSimpleIdQuery( query ) ) {
+                Database *database = cc().database();
+                verify( 15985, database );
+                NamespaceDetails *d = database->namespaceIndex.details(ns);
+                if ( d ) {
+                    int idxNo = d->findIdIndex();
+                    if ( idxNo >= 0 ) {
+                        IndexDetails& i = d->idx( idxNo );
+                        BSONObj key = i.getKeyFromQuery( query );
+                        return shared_ptr<Cursor>( BtreeCursor::make( d, idxNo, i, key, key, true, 1 ) );
+                    }
                 }
             }
         }
-        auto_ptr<MultiPlanScanner> mps( new MultiPlanScanner( ns, query, order,
-                                                             planPolicy.planHint( ns ) ) ); // mayYield == false
-        const QueryPlan *singlePlan = mps->singlePlan();
+        auto_ptr<MultiPlanScanner> mps( new MultiPlanScanner( ns, query, order, &hint, false, parsedQuery ? parsedQuery->getMin() : BSONObj(), parsedQuery ? parsedQuery->getMax() : BSONObj() ) ); // mayYield == false
+        const QueryPlan *singlePlan = mps->singleCursor();
         if ( singlePlan ) {
             if ( planPolicy.permitPlan( *singlePlan ) ) {
                 shared_ptr<Cursor> single = singlePlan->newCursor();
diff --git a/src/mongo/db/queryutil.cpp b/src/mongo/db/queryutil.cpp
index 079895ebc6c..3c312847835 100644
--- a/src/mongo/db/queryutil.cpp
+++ b/src/mongo/db/queryutil.cpp
@@ -27,8 +27,15 @@
 #include "../util/mongoutils/str.h"
 
 namespace mongo {
+
     static const unsigned maxCombinations = 4000000;
 
+    ParsedQuery::ParsedQuery( QueryMessage& qm )
+    : _ns( qm.ns ) , _ntoskip( qm.ntoskip ) , _ntoreturn( qm.ntoreturn ) , _options( qm.queryOptions ) {
+        init( qm.query );
+        initFields( qm.fields );
+    }
+    
     extern BSONObj staticNull;
     extern BSONObj staticUndefined;
 
diff --git a/src/mongo/db/queryutil.h b/src/mongo/db/queryutil.h
index 2f12f7f4c5e..ebac4bf54f0 100644
--- a/src/mongo/db/queryutil.h
+++ b/src/mongo/db/queryutil.h
@@ -19,9 +19,216 @@
 
 #include "jsobj.h"
 #include "indexkey.h"
+#include "projection.h"
 
 namespace mongo {
+    
+    extern const int MaxBytesToReturnToClientAtOnce;
 
+    /* This is for languages whose "objects" are not well ordered (JSON is well ordered).
+     [ { a : ... } , { b : ... } ] -> { a : ..., b : ... }
+     */
+    inline BSONObj transformOrderFromArrayFormat(BSONObj order) {
+        /* note: this is slow, but that is ok as order will have very few pieces */
+        BSONObjBuilder b;
+        char p[2] = "0";
+        
+        while ( 1 ) {
+            BSONObj j = order.getObjectField(p);
+            if ( j.isEmpty() )
+                break;
+            BSONElement e = j.firstElement();
+            uassert( 10102 , "bad order array", !e.eoo());
+            uassert( 10103 , "bad order array [2]", e.isNumber());
+            b.append(e);
+            (*p)++;
+            uassert( 10104 , "too many ordering elements", *p <= '9');
+        }
+        
+        return b.obj();
+    }
+    
+    class QueryMessage;
+    
+    /**
+     * this represents a total user query
+     * includes fields from the query message, both possible query levels
+     * parses everything up front
+     */
+    class ParsedQuery : boost::noncopyable {
+    public:
+        ParsedQuery( QueryMessage& qm );
+        ParsedQuery( const char* ns , int ntoskip , int ntoreturn , int queryoptions , const BSONObj& query , const BSONObj& fields )
+        : _ns( ns ) , _ntoskip( ntoskip ) , _ntoreturn( ntoreturn ) , _options( queryoptions ) {
+            init( query );
+            initFields( fields );
+        }
+        
+        const char * ns() const { return _ns; }
+        bool isLocalDB() const { return strncmp(_ns, "local.", 6) == 0; }
+        
+        const BSONObj& getFilter() const { return _filter; }
+        Projection* getFields() const { return _fields.get(); }
+        shared_ptr<Projection> getFieldPtr() const { return _fields; }
+        
+        int getSkip() const { return _ntoskip; }
+        int getNumToReturn() const { return _ntoreturn; }
+        bool wantMore() const { return _wantMore; }
+        int getOptions() const { return _options; }
+        bool hasOption( int x ) const { return x & _options; }
+        
+        bool isExplain() const { return _explain; }
+        bool isSnapshot() const { return _snapshot; }
+        bool returnKey() const { return _returnKey; }
+        bool showDiskLoc() const { return _showDiskLoc; }
+        
+        const BSONObj& getMin() const { return _min; }
+        const BSONObj& getMax() const { return _max; }
+        const BSONObj& getOrder() const { return _order; }
+        const BSONElement& getHint() const { return _hint; }
+        int getMaxScan() const { return _maxScan; }
+        
+        bool couldBeCommand() const {
+            /* we assume you are using findOne() for running a cmd... */
+            return _ntoreturn == 1 && strstr( _ns , ".$cmd" );
+        }
+        
+        bool hasIndexSpecifier() const {
+            return ! _hint.eoo() || ! _min.isEmpty() || ! _max.isEmpty();
+        }
+        
+        /* if ntoreturn is zero, we return up to 101 objects.  on the subsequent getmore, there
+         is only a size limit.  The idea is that on a find() where one doesn't use much results,
+         we don't return much, but once getmore kicks in, we start pushing significant quantities.
+         
+         The n limit (vs. size) is important when someone fetches only one small field from big
+         objects, which causes massive scanning server-side.
+         */
+        bool enoughForFirstBatch( int n , int len ) const {
+            if ( _ntoreturn == 0 )
+                return ( len > 1024 * 1024 ) || n >= 101;
+            return n >= _ntoreturn || len > MaxBytesToReturnToClientAtOnce;
+        }
+        
+        bool enough( int n ) const {
+            if ( _ntoreturn == 0 )
+                return false;
+            return n >= _ntoreturn;
+        }
+        
+    private:
+        void init( const BSONObj& q ) {
+            _reset();
+            uassert( 10105 , "bad skip value in query", _ntoskip >= 0);
+            
+            if ( _ntoreturn < 0 ) {
+                /* _ntoreturn greater than zero is simply a hint on how many objects to send back per
+                 "cursor batch".
+                 A negative number indicates a hard limit.
+                 */
+                _wantMore = false;
+                _ntoreturn = -_ntoreturn;
+            }
+            
+            
+            BSONElement e = q["query"];
+            if ( ! e.isABSONObj() )
+                e = q["$query"];
+            
+            if ( e.isABSONObj() ) {
+                _filter = e.embeddedObject();
+                _initTop( q );
+            }
+            else {
+                _filter = q;
+            }
+        }
+        
+        void _reset() {
+            _wantMore = true;
+            _explain = false;
+            _snapshot = false;
+            _returnKey = false;
+            _showDiskLoc = false;
+            _maxScan = 0;
+        }
+        
+        void _initTop( const BSONObj& top ) {
+            BSONObjIterator i( top );
+            while ( i.more() ) {
+                BSONElement e = i.next();
+                const char * name = e.fieldName();
+                
+                if ( strcmp( "$orderby" , name ) == 0 ||
+                    strcmp( "orderby" , name ) == 0 ) {
+                    if ( e.type() == Object ) {
+                        _order = e.embeddedObject();
+                    }
+                    else if ( e.type() == Array ) {
+                        _order = transformOrderFromArrayFormat( _order );
+                    }
+                    else {
+                        uasserted(13513, "sort must be an object or array");
+                    }
+                    continue;
+                }
+                
+                if( *name == '$' ) {
+                    name++;
+                    if ( strcmp( "explain" , name ) == 0 )
+                        _explain = e.trueValue();
+                    else if ( strcmp( "snapshot" , name ) == 0 )
+                        _snapshot = e.trueValue();
+                    else if ( strcmp( "min" , name ) == 0 )
+                        _min = e.embeddedObject();
+                    else if ( strcmp( "max" , name ) == 0 )
+                        _max = e.embeddedObject();
+                    else if ( strcmp( "hint" , name ) == 0 )
+                        _hint = e;
+                    else if ( strcmp( "returnKey" , name ) == 0 )
+                        _returnKey = e.trueValue();
+                    else if ( strcmp( "maxScan" , name ) == 0 )
+                        _maxScan = e.numberInt();
+                    else if ( strcmp( "showDiskLoc" , name ) == 0 )
+                        _showDiskLoc = e.trueValue();
+                    else if ( strcmp( "comment" , name ) == 0 ) {
+                        ; // no-op
+                    }
+                }
+            }
+            
+            if ( _snapshot ) {
+                uassert( 12001 , "E12001 can't sort with $snapshot", _order.isEmpty() );
+                uassert( 12002 , "E12002 can't use hint with $snapshot", _hint.eoo() );
+            }
+            
+        }
+        
+        void initFields( const BSONObj& fields ) {
+            if ( fields.isEmpty() )
+                return;
+            _fields.reset( new Projection() );
+            _fields->init( fields );
+        }
+        
+        const char * const _ns;
+        const int _ntoskip;
+        int _ntoreturn;
+        BSONObj _filter;
+        BSONObj _order;
+        const int _options;
+        shared_ptr< Projection > _fields;
+        bool _wantMore;
+        bool _explain;
+        bool _snapshot;
+        bool _returnKey;
+        bool _showDiskLoc;
+        BSONObj _min;
+        BSONObj _max;
+        BSONElement _hint;
+        int _maxScan;
+    };
+    
     /**
      * One side of an interval of valid BSONElements, specified by a value and a
      * boolean indicating whether the interval includes the value.
diff --git a/src/mongo/dbtests/queryoptimizercursortests.cpp b/src/mongo/dbtests/queryoptimizercursortests.cpp
index ea915a63077..fdc21e90b66 100644
--- a/src/mongo/dbtests/queryoptimizercursortests.cpp
+++ b/src/mongo/dbtests/queryoptimizercursortests.cpp
@@ -2387,6 +2387,40 @@ namespace QueryOptimizerCursorTests {
             }
         };
         
+        class BestSavedOptimal : public QueryOptimizerCursorTests::Base {
+        public:
+            void run() {
+                _cli.insert( ns(), BSON( "_id" << 1 ) );
+                _cli.ensureIndex( ns(), BSON( "_id" << 1 << "q" << 1 ) );
+                // {_id:1} index not recorded for these queries since it is an optimal index.
+                ASSERT( _cli.query( ns(), QUERY( "_id" << GT << 0 ) )->more() );
+                ASSERT( _cli.query( ns(), QUERY( "$or" << BSON_ARRAY( BSON( "_id" << GT << 0 ) ) ) )->more() );
+                dblock lk;
+                Client::Context ctx( ns() );
+                // Check that no plan was recorded for this query.
+                ASSERT( BSONObj().woCompare( NamespaceDetailsTransient::get_inlock( ns() ).indexForPattern( FieldRangeSet( ns(), BSON( "_id" << GT << 0 ), true ).pattern() ) ) == 0 );
+                shared_ptr<Cursor> c = NamespaceDetailsTransient::getCursor( ns(), BSON( "_id" << GT << 0 ) );
+                // No need for query optimizer cursor since the plan is optimal.
+                ASSERT_EQUALS( "BtreeCursor _id_", c->toString() );
+            }
+        };
+        
+        class BestSavedNotOptimal : public QueryOptimizerCursorTests::Base {
+        public:
+            void run() {
+                _cli.insert( ns(), BSON( "_id" << 1 << "q" << 1 ) );
+                _cli.ensureIndex( ns(), BSON( "q" << 1 ) );
+                // Record {_id:1} index for this query
+                ASSERT( _cli.query( ns(), QUERY( "q" << 1 << "_id" << 1 ) )->more() );
+                dblock lk;
+                Client::Context ctx( ns() );
+                ASSERT( BSON( "_id" << 1 ).woCompare( NamespaceDetailsTransient::get_inlock( ns() ).indexForPattern( FieldRangeSet( ns(), BSON( "q" << 1 << "_id" << 1 ), true ).pattern() ) ) == 0 );
+                shared_ptr<Cursor> c = NamespaceDetailsTransient::getCursor( ns(), BSON( "q" << 1 << "_id" << 1 ) );
+                // Need query optimizer cursor since the cached plan is not optimal.
+                ASSERT_EQUALS( "QueryOptimizerCursor", c->toString() );
+            }
+        };
+        
         class MultiIndex : public Base {
         public:
             MultiIndex() {
diff --git a/src/mongo/dbtests/queryoptimizertests.cpp b/src/mongo/dbtests/queryoptimizertests.cpp
index 59479d57231..ea57aa8896c 100644
--- a/src/mongo/dbtests/queryoptimizertests.cpp
+++ b/src/mongo/dbtests/queryoptimizertests.cpp
@@ -829,39 +829,6 @@ namespace QueryOptimizerTests {
             }
         };
 
-        class TryOtherPlansBeforeFinish : public Base {
-        public:
-            void run() {
-                Helpers::ensureIndex( ns(), BSON( "a" << 1 ), false, "a_1" );
-                for( int i = 0; i < 100; ++i ) {
-                    for( int j = 0; j < 2; ++j ) {
-                        BSONObj temp = BSON( "a" << 100 - i - 1 << "b" << i );
-                        theDataFileMgr.insertWithObjMod( ns(), temp );
-                    }
-                }
-                Message m;
-                // Need to return at least 2 records to cause plan to be recorded.
-                assembleRequest( ns(), QUERY( "b" << 0 << "a" << GTE << 0 ).obj, 2, 0, 0, 0, m );
-                stringstream ss;
-                {
-                    DbMessage d(m);
-                    QueryMessage q(d);
-                    runQuery( m, q);
-                }
-                ASSERT( BSON( "$natural" << 1 ).woCompare( NamespaceDetailsTransient::get_inlock( ns() ).indexForPattern( FieldRangeSet( ns(), BSON( "b" << 0 << "a" << GTE << 0 ), true ).pattern() ) ) == 0 );
-
-                Message m2;
-                assembleRequest( ns(), QUERY( "b" << 99 << "a" << GTE << 0 ).obj, 2, 0, 0, 0, m2 );
-                {
-                    DbMessage d(m2);
-                    QueryMessage q(d);
-                    runQuery( m2, q);
-                }
-                ASSERT( BSON( "a" << 1 ).woCompare( NamespaceDetailsTransient::get_inlock( ns() ).indexForPattern( FieldRangeSet( ns(), BSON( "b" << 0 << "a" << GTE << 0 ), true ).pattern() ) ) == 0 );
-                ASSERT_EQUALS( 3, NamespaceDetailsTransient::get_inlock( ns() ).nScannedForPattern( FieldRangeSet( ns(), BSON( "b" << 0 << "a" << GTE << 0 ), true ).pattern() ) );
-            }
-        };
-
         class InQueryIntervals : public Base {
         public:
             void run() {
@@ -1085,7 +1052,6 @@ namespace QueryOptimizerTests {
             add<QueryPlanSetTests::Delete>();
             add<QueryPlanSetTests::DeleteOneScan>();
             add<QueryPlanSetTests::DeleteOneIndex>();
-            add<QueryPlanSetTests::TryOtherPlansBeforeFinish>();
             add<QueryPlanSetTests::InQueryIntervals>();
             add<QueryPlanSetTests::EqualityThenIn>();
             add<QueryPlanSetTests::NotEqualityThenIn>();