From fc1707aa992d39d2a65ee070c5e55b559f31061c Mon Sep 17 00:00:00 2001 From: Aaron Date: Mon, 19 Jul 2010 00:39:16 -0700 Subject: [PATCH] SERVER-1437 update or range elimination --- db/queryoptimizer.cpp | 10 +----- db/queryutil.cpp | 7 ++++ db/queryutil.h | 64 +++++++++++++++++++++------------ dbtests/queryoptimizertests.cpp | 14 -------- jstests/or6.js | 2 +- jstests/or9.js | 54 ++++++++++++++++++++++++++++ 6 files changed, 104 insertions(+), 47 deletions(-) create mode 100644 jstests/or9.js diff --git a/db/queryoptimizer.cpp b/db/queryoptimizer.cpp index 712546ca55d..c29f9caf02a 100644 --- a/db/queryoptimizer.cpp +++ b/db/queryoptimizer.cpp @@ -670,15 +670,7 @@ namespace mongo { BSONElement hintElt = _hint.firstElement(); _currentQps.reset( new QueryPlanSet( _ns, frs, _query, BSONObj(), &hintElt, _honorRecordedPlan, BSONObj(), BSONObj(), _bestGuessOnly, _mayYield ) ); shared_ptr< QueryOp > ret( _currentQps->runOp( op ) ); - BSONObj selectedIndexKey = ret->qp().indexKey(); - const char *first = 0; - const char *second = 0; - BSONObjIterator i( selectedIndexKey ); - first = i.next().fieldName(); - if ( i.more() ) { - second = i.next().fieldName(); - } - _fros.popOrClause( first, second ); + _fros.popOrClause(); return ret; } diff --git a/db/queryutil.cpp b/db/queryutil.cpp index a065d98bc7e..1d954ec5304 100644 --- a/db/queryutil.cpp +++ b/db/queryutil.cpp @@ -492,6 +492,13 @@ namespace mongo { return *this; } + // TODO write a proper implementation that doesn't do a full copy + bool FieldRange::operator<=( const FieldRange &other ) { + FieldRange temp = *this; + temp -= other; + return temp.empty(); + } + BSONObj FieldRange::addObj( const BSONObj &o ) { _objData.push_back( o ); return o; diff --git a/db/queryutil.h b/db/queryutil.h index 8e80f354350..d9302a2c6ca 100644 --- a/db/queryutil.h +++ b/db/queryutil.h @@ -62,6 +62,8 @@ namespace mongo { // does not remove fully contained ranges (eg [1,3] - [2,2] doesn't remove anything) // in future we can change so that an or on $in:[3] combined with $in:{$gt:2} doesn't scan 3 a second time const FieldRange &operator-=( const FieldRange &other ); + // true iff other includes this + bool operator<=( const FieldRange &other ); BSONElement min() const { assert( !empty() ); return _intervals[ 0 ]._lower._bound; } BSONElement max() const { assert( !empty() ); return _intervals[ _intervals.size() - 1 ]._upper._bound; } bool minInclusive() const { assert( !empty() ); return _intervals[ 0 ]._lower._inclusive; } @@ -91,6 +93,7 @@ namespace mongo { maxKey.firstElement().woCompare( max(), false ) != 0 ); } bool empty() const { return _intervals.empty(); } + void makeEmpty() { _intervals.clear(); } const vector< FieldInterval > &intervals() const { return _intervals; } string getSpecial() const { return _special; } void setExclusiveBounds() { @@ -234,20 +237,41 @@ namespace mongo { QueryPattern pattern( const BSONObj &sort = BSONObj() ) const; string getSpecial() const; const FieldRangeSet &operator-=( const FieldRangeSet &other ) { + int nUnincluded = 0; + string unincludedKey; map< string, FieldRange >::iterator i = _ranges.begin(); map< string, FieldRange >::const_iterator j = other._ranges.begin(); - while( i != _ranges.end() && j != other._ranges.end() ) { + while( nUnincluded < 2 && i != _ranges.end() && j != other._ranges.end() ) { int cmp = i->first.compare( j->first ); if ( cmp == 0 ) { - i->second -= j->second; + if ( i->second <= j->second ) { + // nothing + } else { + ++nUnincluded; + unincludedKey = i->first; + } ++i; ++j; } else if ( cmp < 0 ) { ++i; } else { - ++j; + // other has a bound we don't, nothing can be done + return *this; } } + if ( j != other._ranges.end() ) { + // other has a bound we don't, nothing can be done + return *this; + } + if ( nUnincluded > 1 ) { + return *this; + } + if ( nUnincluded == 0 ) { + makeEmpty(); + return *this; + } + // nUnincluded == 1 + _ranges[ unincludedKey ] -= other._ranges[ unincludedKey ]; appendQueries( other ); return *this; } @@ -282,6 +306,11 @@ namespace mongo { _queries.push_back( *i ); } } + void makeEmpty() { + for( map< string, FieldRange >::iterator i = _ranges.begin(); i != _ranges.end(); ++i ) { + i->second.makeEmpty(); + } + } void processQueryField( const BSONElement &e, bool optimize ); void processOpElement( const char *fieldName, const BSONElement &f, bool isNot, bool optimize ); static FieldRange *trivialRange_; @@ -449,28 +478,17 @@ namespace mongo { bool orFinished() const { return _orFound && _orSets.empty(); } // removes first or clause, and removes the field ranges it covers from all subsequent or clauses // this could invalidate the result of the last topFrs() - void popOrClause( const char *firstField, const char *secondField ) { + void popOrClause() { massert( 13274, "no or clause to pop", !orFinished() ); const FieldRangeSet &toPop = _orSets.front(); - if ( toPop.hasRange( firstField ) ) { - if ( secondField && toPop.hasRange( secondField ) ) { - // modifying existing front is ok - this is the last time we'll use it - _orSets.front().range( firstField ).setExclusiveBounds(); - } - const FieldRange &r = toPop.range( firstField ); - list< FieldRangeSet >::iterator i = _orSets.begin(); - ++i; - while( i != _orSets.end() ) { - if ( i->hasRange( firstField ) ) { - i->range( firstField ) -= r; - if( !i->matchPossible() ) { - i = _orSets.erase( i ); - } else { - ++i; - } - } else { - ++i; - } + list< FieldRangeSet >::iterator i = _orSets.begin(); + ++i; + while( i != _orSets.end() ) { + *i -= toPop; + if( !i->matchPossible() ) { + i = _orSets.erase( i ); + } else { + ++i; } } _oldOrSets.push_front( toPop ); diff --git a/dbtests/queryoptimizertests.cpp b/dbtests/queryoptimizertests.cpp index 93719041b35..c0011b491f4 100644 --- a/dbtests/queryoptimizertests.cpp +++ b/dbtests/queryoptimizertests.cpp @@ -684,19 +684,6 @@ namespace QueryOptimizerTests { virtual BSONObj obj() const { return BSONObj(); } }; - class SetDiff { - public: - void run() { - FieldRangeSet frs1( "", fromjson( "{a:5,c:{$in:[6,7]},e:{$in:[7,8]},f:8}" ) ); - FieldRangeSet frs2( "", fromjson( "{b:5,c:6,d:7,e:7}" ) ); - frs1 -= frs2; - ASSERT_EQUALS( BSON( "a" << 5 << "c" << 7 << "e" << 8 << "f" << 8 ), frs1.simplifiedQuery() ); - FieldRangeSet frs3( "", fromjson( "{a:5}" ) ); - frs1 -= frs3; - ASSERT( !frs1.matchPossible() ); - } - }; - class SetIntersect { public: void run() { @@ -1685,7 +1672,6 @@ namespace QueryOptimizerTests { add< FieldRangeTests::Diff63 >(); add< FieldRangeTests::DiffMulti1 >(); add< FieldRangeTests::DiffMulti2 >(); - add< FieldRangeTests::SetDiff >(); add< FieldRangeTests::SetIntersect >(); add< QueryPlanTests::NoIndex >(); add< QueryPlanTests::SimpleOrder >(); diff --git a/jstests/or6.js b/jstests/or6.js index 56b3e917ab9..3800c7874e6 100644 --- a/jstests/or6.js +++ b/jstests/or6.js @@ -27,5 +27,5 @@ t.drop(); t.ensureIndex( {a:1,b:1} ); assert.eq.automsg( "2", "t.find( {$or:[{a:{$in:[1,2]},b:5}, {a:2,b:6}]} ).explain().clauses.length" ); assert.eq.automsg( "2", "t.find( {$or:[{a:{$gt:1,$lte:2},b:5}, {a:2,b:6}]} ).explain().clauses.length" ); -assert.eq.automsg( "null", "t.find( {$or:[{a:{$gt:1,$lte:3},b:5}, {a:2,b:6}]} ).explain().clauses" ); +assert.eq.automsg( "2", "t.find( {$or:[{a:{$gt:1,$lte:3},b:5}, {a:2,b:6}]} ).explain().clauses.length" ); assert.eq.automsg( "null", "t.find( {$or:[{a:{$in:[1,2]}}, {a:2}]} ).explain().clauses" ); \ No newline at end of file diff --git a/jstests/or9.js b/jstests/or9.js new file mode 100644 index 00000000000..5c03660f1e9 --- /dev/null +++ b/jstests/or9.js @@ -0,0 +1,54 @@ +// index skipping and previous index range negation + +t = db.jstests_or9; +t.drop(); + +t.ensureIndex( {a:1,b:1} ); + +t.save( {a:2,b:2} ); + +function check( a, b, q ) { + count = a; + clauses = b; + query = q; + assert.eq.automsg( "count", "t.count( query )" ); + if ( clauses == 1 ) { + assert.eq.automsg( "undefined", "t.find( query ).explain().clauses" ); + } else { + assert.eq.automsg( "clauses", "t.find( query ).explain().clauses.length" ); + } +} + +check( 1, 1, { $or: [ { a: { $gte:1,$lte:3 } }, { a: 2 } ] } ); +check( 1, 2, { $or: [ { a: { $gt:2,$lte:3 } }, { a: 2 } ] } ); + +check( 1, 1, { $or: [ { b: { $gte:1,$lte:3 } }, { b: 2 } ] } ); +check( 1, 1, { $or: [ { b: { $gte:2,$lte:3 } }, { b: 2 } ] } ); +//check( 1, 2, { $or: [ { b: { $gt:2,$lte:3 } }, { b: 2 } ] } ); + +check( 1, 1, { $or: [ { a: { $gte:1,$lte:3 } }, { a: 2, b: 2 } ] } ); +check( 1, 2, { $or: [ { a: { $gte:1,$lte:3 }, b:3 }, { a: 2 } ] } ); + +check( 1, 1, { $or: [ { b: { $gte:1,$lte:3 } }, { b: 2, a: 2 } ] } ); +check( 1, 2, { $or: [ { b: { $gte:1,$lte:3 }, a:3 }, { b: 2 } ] } ); + +check( 1, 2, { $or: [ { a: { $gte:1,$lte:3 }, b: 3 }, { a: 2, b: 2 } ] } ); +check( 1, 2, { $or: [ { a: { $gte:2,$lte:3 }, b: 3 }, { a: 2, b: 2 } ] } ); +check( 1, 1, { $or: [ { a: { $gte:1,$lte:3 }, b: 2 }, { a: 2, b: 2 } ] } ); + +check( 1, 2, { $or: [ { b: { $gte:1,$lte:3 }, a: 3 }, { a: 2, b: 2 } ] } ); +check( 1, 2, { $or: [ { b: { $gte:2,$lte:3 }, a: 3 }, { a: 2, b: 2 } ] } ); +check( 1, 1, { $or: [ { b: { $gte:1,$lte:3 }, a: 2 }, { a: 2, b: 2 } ] } ); + + + +t.remove(); + +t.save( {a:1,b:5} ); +t.save( {a:5,b:1} ); + +check( 2, 1, { $or: [ { a: { $in:[1,5] }, b: { $in:[1,5] } }, { a: { $in:[1,5] }, b: { $in:[1,5] } } ] } ); +check( 2, 2, { $or: [ { a: { $in:[1] }, b: { $in:[1,5] } }, { a: { $in:[1,5] }, b: { $in:[1,5] } } ] } ); +check( 2, 2, { $or: [ { a: { $in:[1] }, b: { $in:[1] } }, { a: { $in:[1,5] }, b: { $in:[1,5] } } ] } ); + +assert.eq.automsg( {a:[[1,1],[5,5]],b:[[1,1],[5,5]]}, "t.find( { $or: [ { a: { $in:[1] }, b: { $in:[1] } }, { a: { $in:[1,5] }, b: { $in:[1,5] } } ] } ).explain().clauses[ 1 ].indexBounds" );