SERVER-4665 backport 'ConstraintPresent' constraint type and exclusion of unhelpful recorded plans

5 files changed, 51 insertions, 7 deletions

diff --git a/db/queryoptimizer.cpp b/db/queryoptimizer.cpp
index 692e9fd9b98..71ca6576fc4 100644
--- a/db/queryoptimizer.cpp
+++ b/db/queryoptimizer.cpp
@@ -482,7 +482,7 @@ doneCheckOrder:
                 }
 
                 massert( 10368 ,  "Unable to locate previously recorded index", p.get() );
-                if ( !( _bestGuessOnly && p->scanAndOrderRequired() ) ) {
+                if ( !p->unhelpful() && !( _bestGuessOnly && p->scanAndOrderRequired() ) ) {
                     _usingPrerecordedPlan = true;
                     _mayRecordPlan = false;
                     _plans.push_back( p );
diff --git a/db/querypattern.h b/db/querypattern.h
index d87cc64a5bc..2f7450e5ec2 100644
--- a/db/querypattern.h
+++ b/db/querypattern.h
@@ -36,7 +36,8 @@ namespace mongo {
             Equality,
             LowerBound,
             UpperBound,
-            UpperAndLowerBound
+            UpperAndLowerBound,
+            ConstraintPresent
         };
         bool operator<( const QueryPattern &other ) const;
         /** for testing only */
diff --git a/db/queryutil.cpp b/db/queryutil.cpp
index 717eac816b8..ef5ae3a6fdb 100644
--- a/db/queryutil.cpp
+++ b/db/queryutil.cpp
@@ -1007,6 +1007,8 @@ namespace mongo {
                     qp._fieldTypes[ i->first ] = QueryPattern::UpperBound;
                 else if ( lower )
                     qp._fieldTypes[ i->first ] = QueryPattern::LowerBound;
+                else
+                    qp._fieldTypes[ i->first ] = QueryPattern::ConstraintPresent;
             }
         }
         qp.setSort( sort );
diff --git a/dbtests/queryutiltests.cpp b/dbtests/queryutiltests.cpp
index e825b4f8a9b..c3dd64d12dd 100644
--- a/dbtests/queryutiltests.cpp
+++ b/dbtests/queryutiltests.cpp
@@ -238,7 +238,14 @@ namespace QueryUtilTests {
             }
         };
 
-        class QueryPatternTest {
+        class QueryPatternBase {
+        protected:
+            static QueryPattern p( const BSONObj &query, const BSONObj &sort = BSONObj() ) {
+                return FieldRangeSet( "", query, true ).pattern( sort );
+            }
+        };
+        
+        class QueryPatternTest : public QueryPatternBase {
         public:
             void run() {
                 ASSERT( p( BSON( "a" << 1 ) ) == p( BSON( "a" << 1 ) ) );
@@ -258,12 +265,17 @@ namespace QueryUtilTests {
                 ASSERT( p( BSON( "a" << 1 ), BSON( "b" << 1 << "c" << 1 ) ) != p( BSON( "a" << 4 ), BSON( "b" << 1 ) ) );
                 ASSERT( p( BSON( "a" << 1 ), BSON( "b" << 1 ) ) != p( BSON( "a" << 4 ), BSON( "b" << 1 << "c" << 1 ) ) );
             }
-        private:
-            static QueryPattern p( const BSONObj &query, const BSONObj &sort = BSONObj() ) {
-                return FieldRangeSet( "", query, true ).pattern( sort );
+        };
+        
+        class QueryPatternNeConstraint : public QueryPatternBase {
+        public:
+            void run() {
+                ASSERT( p( BSON( "a" << NE << 5 ) ) != p( BSON( "a" << GT << 1 ) ) );
+                ASSERT( p( BSON( "a" << NE << 5 ) ) != p( BSONObj() ) );
+                ASSERT( p( BSON( "a" << NE << 5 ) ) == p( BSON( "a" << NE << "a" ) ) );
             }
         };
-
+        
         class NoWhere {
         public:
             void run() {
@@ -902,6 +914,7 @@ namespace QueryUtilTests {
             add< FieldRangeTests::Equality >();
             add< FieldRangeTests::SimplifiedQuery >();
             add< FieldRangeTests::QueryPatternTest >();
+            add< FieldRangeTests::QueryPatternNeConstraint >();
             add< FieldRangeTests::NoWhere >();
             add< FieldRangeTests::Numeric >();
             add< FieldRangeTests::InLowerBound >();
diff --git a/jstests/queryoptimizer6.js b/jstests/queryoptimizer6.js
new file mode 100644
index 00000000000..fce92d76d6b
--- /dev/null
+++ b/jstests/queryoptimizer6.js
@@ -0,0 +1,28 @@
+// Test that $ne constraints are accounted for in QueryPattern. SERVER-4665
+
+t = db.jstests_queryoptimizer6;
+
+function reset() {
+    t.drop();
+    t.save( {a:1} );
+    t.ensureIndex( {b:1}, {sparse:true} );
+}
+
+reset();
+// The sparse index will be used, and recorded for this query pattern.
+assert.eq( 0, t.find( {a:1,b:{$ne:1}} ).itcount() );
+// The query pattern should be different, and the sparse index should not be used.
+assert.eq( 1, t.find( {a:1} ).itcount() );
+
+reset();
+// The sparse index will be used, and (for better or worse) recorded for this query pattern.
+assert.eq( 0, t.find( {a:1} ).min({b:1}).itcount() );
+// The sparse index should not be used, even though the query patterns match.
+assert.eq( 1, t.find( {a:1} ).itcount() );
+
+reset();
+t.ensureIndex( {a:1,b:1} );
+// The sparse index will be used, and (for better or worse) recorded for this query pattern.
+assert.eq( 0, t.find( {a:1,b:null} ).min({b:1}).itcount() );
+// Descriptive test - the recorded {b:1} index is used, because it is not useless.
+assert.eq( 0, t.find( {a:1,b:null} ).itcount() );