SERVER-4150 SERVER-5198 Don't advance a duplicate entry on takeover w…

…hen mixed in order and out of order plans are possible.
mongodb · Mar 22, 2012 · 589cd70 · 589cd70
1 parent a836080
commit 589cd70
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 2 deletions.
diff --git a/src/mongo/db/queryoptimizercursorimpl.cpp b/src/mongo/db/queryoptimizercursorimpl.cpp
@@ -585,8 +585,14 @@ namespace mongo {
             }
             else if ( op->stopRequested() ) {
                 if ( qocop->cursor() ) {
-                    // Ensure that prepareToTouchEarlierIterate() may be called safely when a BasicCursor takes over.
-                    if ( !prevLoc.isNull() && prevLoc == qocop->currLoc() ) {
+                    // Ensure that prepareToTouchEarlierIterate() may be called safely when a
+                    // BasicCursor takes over.
+                    if ( !prevLoc.isNull() && prevLoc == qocop->currLoc() &&
+                        // If there is an out of order plan, advancing may be incorrect because
+                        // in orer plans must return all results.  And advancing is unnecessary,
+                        // because _mps will not traverse $or clauses.
+                        // TODO Clean this as part of SERVER-5198.
+                        !_mps->possibleOutOfOrderPlan() ) {
                         qocop->cursor()->advance();
                     }
                     _takeover.reset( new MultiCursor( _mps,

diff --git a/src/mongo/dbtests/queryoptimizercursortests.cpp b/src/mongo/dbtests/queryoptimizercursortests.cpp
@@ -3193,6 +3193,35 @@ namespace QueryOptimizerCursorTests {
         }
     };
 
+    /**
+     * An ordered plan returns all results, including when it takes over, even when it duplicates an
+     * entry of an out of order plan.
+     */
+    class TakeoverOrderedPlanDupsOutOfOrderPlan : public PlanChecking {
+    public:
+        void run() {
+            _cli.ensureIndex( ns(), BSON( "a" << 1 ) );
+            for( int i = 1; i < 200; ++i ) {
+                _cli.insert( ns(), BSON( "a" << i << "b" << 0 ) );
+            }
+            // Insert this document last, so that most documents are read from the $natural cursor
+            // before the a:1 cursor.
+            _cli.insert( ns(), BSON( "a" << 0 << "b" << 0 ) );
+
+            Client::ReadContext ctx( ns() );
+            shared_ptr<QueryOptimizerCursor> cursor =
+                    getCursor( BSON( "a" << GTE << 0 << "b" << 0 ), BSON( "a" << 1 ) );
+            int nextA = 0;
+            for( ; cursor->ok(); cursor->advance() ) {
+                if ( cursor->indexKeyPattern() == BSON( "a" << 1 ) ) {
+                    // Check that the expected 'a' value is present and in order.
+                    ASSERT_EQUALS( nextA++, cursor->current()[ "a" ].number() );
+                }
+            }
+            ASSERT_EQUALS( 200, nextA );
+        }
+    };
+
     namespace GetCursor {
 
         class Base : public QueryOptimizerCursorTests::Base {
@@ -4354,6 +4383,7 @@ namespace QueryOptimizerCursorTests {
             add<AbortOutOfOrderPlanOnLastMatch>();
             add<TakeoverOrRangeElimination>();
             add<TakeoverOrDedups>();
+            add<TakeoverOrderedPlanDupsOutOfOrderPlan>();
             add<GetCursor::NoConstraints>();
             add<GetCursor::SimpleId>();
             add<GetCursor::OptimalIndex>();