Permalink
Browse files

SERVER-5936 Prevent 'special' query plans from being used for a claus…

…e iterated $or query execution.
  • Loading branch information...
1 parent cc83105 commit b2c99f16345d64b3f165d1338e484c3fcac5ff29 @astaple astaple committed Jun 6, 2012
@@ -0,0 +1,19 @@
+// The query optimizer will not generate 'special' plans inside $or clauses. SERVER-5936
+
+t = db.jstests_queryoptimizerb;
+t.drop();
+
+t.ensureIndex( { a:'2d' } );
+// Add an index to make the { b:1 } query mongo::HELPFUL but QueryPlan::Unhelpful.
+t.ensureIndex( { c:1, b:1 } );
+
+// A 'special' index is not used for an $or equality match query.
+t.save( { a:[ 0, 0 ], b:1 } );
+assert.eq( 1, t.find( { a:[ 0, 0 ], $or:[ { b:1 } ] } ).itcount() );
+assert.eq( 'BasicCursor', t.find( { a:[ 0, 0 ], $or:[ { b:1 } ] } ).explain().cursor );
+
+// A 'special' index cannot be hinted for a $or query.
+assert.throws( function() {
+ t.find( { a:[ 0, 0 ], $or:[ { a:[ 0, 0 ] } ] } ).hint( { a:'2d' } ).itcount(); } );
+assert.throws( function() {
+ t.find( { a:[ 0, 0 ], $or:[ { a:[ 0, 0 ] } ] } ).hint( { a:'2d' } ).explain(); } );
@@ -501,14 +501,16 @@ namespace mongo {
const BSONObj &hint,
RecordedPlanPolicy recordedPlanPolicy,
const BSONObj &min,
- const BSONObj &max ) :
+ const BSONObj &max,
+ bool allowSpecial ) :
_qps( qps ),
_originalFrsp( originalFrsp ),
_parsedQuery( parsedQuery ),
_hint( hint.getOwned() ),
_recordedPlanPolicy( recordedPlanPolicy ),
_min( min.getOwned() ),
- _max( max.getOwned() ) {
+ _max( max.getOwned() ),
+ _allowSpecial( allowSpecial ) {
}
void QueryPlanGenerator::addInitialPlans() {
@@ -548,12 +550,13 @@ namespace mongo {
}
break;
case QueryPlan::Helpful:
- if ( !p->special().empty() ) {
- specialPlan = p;
- }
- else {
+ if ( p->special().empty() ) {
+ // Not a 'special' plan.
plans.push_back( p );
}
+ else if ( _allowSpecial ) {
+ specialPlan = p;
+ }
break;
default:
break;
@@ -606,7 +609,7 @@ namespace mongo {
if ( !hint.eoo() ) {
IndexDetails *id = parseHint( hint, d );
if ( id ) {
- setHintedPlan( *id );
+ setHintedPlanForIndex( *id );
}
else {
uassert( 10366, "natural order cannot be specified with $min/$max",
@@ -615,14 +618,14 @@ namespace mongo {
}
return true;
}
-
+
if ( !_min.isEmpty() || !_max.isEmpty() ) {
string errmsg;
BSONObj keyPattern;
IndexDetails *idx = indexDetailsForRange( _qps.frsp().ns(), errmsg, _min, _max,
keyPattern );
uassert( 10367 , errmsg, idx );
- _qps.setSinglePlan( newPlan( d, d->idxNo( *idx ), _min, _max ) );
+ validateAndSetHintedPlan( newPlan( d, d->idxNo( *idx ), _min, _max ) );
return true;
}
@@ -640,6 +643,7 @@ namespace mongo {
const IndexSpec& spec = ii.getSpec();
if ( spec.getTypeName() == special &&
spec.suitability( _qps.originalQuery(), _qps.order() ) ) {
+ uassert( 16330, "'special' query operator not allowed", _allowSpecial );
_qps.setSinglePlan( newPlan( d, j, BSONObj(), BSONObj(), special ) );
return true;
}
@@ -692,6 +696,10 @@ namespace mongo {
return false;
}
+ if ( !_allowSpecial && !p->special().empty() ) {
+ return false;
+ }
+
_qps.setCachedPlan( p, best );
return true;
}
@@ -718,7 +726,7 @@ namespace mongo {
_qps.setSinglePlan( newPlan( d, -1 ) );
}
- void QueryPlanGenerator::setHintedPlan( IndexDetails &id ) {
+ void QueryPlanGenerator::setHintedPlanForIndex( IndexDetails& id ) {
if ( !_min.isEmpty() || !_max.isEmpty() ) {
string errmsg;
BSONObj keyPattern = id.keyPattern();
@@ -727,9 +735,15 @@ namespace mongo {
keyPattern ) );
}
NamespaceDetails *d = nsdetails( _qps.frsp().ns() );
- _qps.setSinglePlan( newPlan( d, d->idxNo( id ), _min, _max ) );
+ validateAndSetHintedPlan( newPlan( d, d->idxNo( id ), _min, _max ) );
}
-
+
+ void QueryPlanGenerator::validateAndSetHintedPlan( const shared_ptr<QueryPlan>& plan ) {
+ uassert( 16331, "'special' plan hint not allowed",
+ _allowSpecial || plan->special().empty() );
+ _qps.setSinglePlan( plan );
+ }
+
void QueryPlanGenerator::warnOnCappedIdTableScan() const {
// if we are doing a table scan on _id
// and it's a capped collection
@@ -765,10 +779,11 @@ namespace mongo {
const BSONObj& hint,
QueryPlanGenerator::RecordedPlanPolicy recordedPlanPolicy,
const BSONObj& min,
- const BSONObj& max ) {
+ const BSONObj& max,
+ bool allowSpecial ) {
auto_ptr<QueryPlanSet> ret( new QueryPlanSet( ns, frsp, originalFrsp, originalQuery, order,
parsedQuery, hint, recordedPlanPolicy, min,
- max ) );
+ max, allowSpecial ) );
ret->init();
return ret.release();
}
@@ -783,15 +798,18 @@ namespace mongo {
const BSONObj &hint,
QueryPlanGenerator::RecordedPlanPolicy recordedPlanPolicy,
const BSONObj &min,
- const BSONObj &max ) :
- _generator( *this, originalFrsp, parsedQuery, hint, recordedPlanPolicy, min, max ),
+ const BSONObj &max,
+ bool allowSpecial ) :
+ _generator( *this, originalFrsp, parsedQuery, hint, recordedPlanPolicy, min, max,
+ allowSpecial ),
_originalQuery( originalQuery ),
_frsp( frsp ),
_mayRecordPlan(),
_usingCachedPlan(),
_order( order.getOwned() ),
_oldNScanned( 0 ),
- _yieldSometimesTracker( 256, 20 ) {
+ _yieldSometimesTracker( 256, 20 ),
+ _allowSpecial( allowSpecial ) {
}
bool QueryPlanSet::hasMultiKey() const {
@@ -811,7 +829,7 @@ namespace mongo {
void QueryPlanSet::setSinglePlan( const QueryPlanPtr &plan ) {
if ( nPlans() == 0 ) {
- _plans.push_back( plan );
+ pushPlan( plan );
}
}
@@ -821,7 +839,7 @@ namespace mongo {
_usingCachedPlan = true;
_oldNScanned = cachedPlan.nScanned();
_cachedPlanCharacter = cachedPlan.planCharacter();
- _plans.push_back( plan );
+ pushPlan( plan );
}
void QueryPlanSet::addCandidatePlan( const QueryPlanPtr &plan ) {
@@ -830,15 +848,20 @@ namespace mongo {
if ( nPlans() > 0 && plan->indexKey() == firstPlan()->indexKey() ) {
return;
}
- _plans.push_back( plan );
+ pushPlan( plan );
_mayRecordPlan = true;
}
void QueryPlanSet::addFallbackPlans() {
_generator.addFallbackPlans();
_mayRecordPlan = true;
}
-
+
+ void QueryPlanSet::pushPlan( const QueryPlanSet::QueryPlanPtr& plan ) {
+ verify( _allowSpecial || plan->special().empty() );
+ _plans.push_back( plan );
+ }
+
bool QueryPlanSet::hasPossiblyExcludedPlans() const {
return
_usingCachedPlan &&
@@ -1180,7 +1203,7 @@ namespace mongo {
updateCurrentQps( QueryPlanSet::make( _ns.c_str(), frsp, auto_ptr<FieldRangeSetPair>(),
_query, order, _parsedQuery, _hint,
_recordedPlanPolicy,
- min, max ) );
+ min, max, true ) );
}
else {
BSONElement e = _query.getField( "$or" );
@@ -1206,7 +1229,9 @@ namespace mongo {
auto_ptr<FieldRangeSetPair> originalFrsp( _org->topFrspOriginal() );
updateCurrentQps( QueryPlanSet::make( _ns.c_str(), frsp, originalFrsp, _query,
BSONObj(), _parsedQuery, _hint, _recordedPlanPolicy,
- BSONObj(), BSONObj() ) );
+ BSONObj(), BSONObj(),
+ // 'Special' plans are not supported within $or.
+ false ) );
}
bool MultiPlanScanner::mayHandleBeginningOfClause() {
@@ -1561,7 +1586,7 @@ namespace mongo {
scoped_ptr<QueryPlanSet> qps( QueryPlanSet::make( ns, frsp, origFrsp, query, sort,
shared_ptr<const ParsedQuery>(), BSONObj(),
QueryPlanGenerator::UseIfInOrder,
- BSONObj(), BSONObj() ) );
+ BSONObj(), BSONObj(), true ) );
QueryPlanSet::QueryPlanPtr qpp = qps->getBestGuess();
if( ! qpp.get() ) return shared_ptr<Cursor>();
@@ -313,7 +313,8 @@ namespace mongo {
const BSONObj &hint,
RecordedPlanPolicy recordedPlanPolicy,
const BSONObj &min,
- const BSONObj &max );
+ const BSONObj &max,
+ bool allowSpecial );
/** Populate the provided QueryPlanSet with an initial set of plans. */
void addInitialPlans();
/** Supplement a cached plan provided earlier by adding additional query plans. */
@@ -333,7 +334,8 @@ namespace mongo {
const string &special = "" ) const;
bool setUnindexedPlanIf( bool set, NamespaceDetails *d );
void setSingleUnindexedPlan( NamespaceDetails *d );
- void setHintedPlan( IndexDetails &id );
+ void setHintedPlanForIndex( IndexDetails& id );
+ void validateAndSetHintedPlan( const shared_ptr<QueryPlan>& plan );
void warnOnCappedIdTableScan() const;
QueryPlanSet &_qps;
auto_ptr<FieldRangeSetPair> _originalFrsp;
@@ -342,6 +344,7 @@ namespace mongo {
RecordedPlanPolicy _recordedPlanPolicy;
BSONObj _min;
BSONObj _max;
+ bool _allowSpecial;
};
/**
@@ -366,7 +369,8 @@ namespace mongo {
const BSONObj& hint,
QueryPlanGenerator::RecordedPlanPolicy recordedPlanPolicy,
const BSONObj& min,
- const BSONObj& max );
+ const BSONObj& max,
+ bool allowSpecial );
/** @return number of candidate plans. */
int nPlans() const { return _plans.size(); }
@@ -468,10 +472,12 @@ namespace mongo {
const BSONObj &hint,
QueryPlanGenerator::RecordedPlanPolicy recordedPlanPolicy,
const BSONObj &min,
- const BSONObj &max );
+ const BSONObj &max,
+ bool allowSpecial );
void init();
void addFallbackPlans();
+ void pushPlan( const QueryPlanPtr& plan );
QueryPlanGenerator _generator;
BSONObj _originalQuery;
@@ -483,6 +489,7 @@ namespace mongo {
BSONObj _order;
long long _oldNScanned;
ElapsedTracker _yieldSometimesTracker;
+ bool _allowSpecial;
};
/** Handles $or type queries by generating a QueryPlanSet for each $or clause. */
@@ -2835,7 +2835,7 @@ namespace QueryOptimizerCursorTests {
scoped_ptr<QueryPlanSet> s( QueryPlanSet::make( ns(), frsp, frspOrig, query, order,
shared_ptr<const ParsedQuery>(),
BSONObj(), QueryPlanGenerator::Use,
- BSONObj(), BSONObj() ) );
+ BSONObj(), BSONObj(), true ) );
ASSERT_EQUALS( n, s->nPlans() );
}
static shared_ptr<QueryOptimizerCursor> getCursor( const BSONObj &query,
@@ -803,13 +803,15 @@ namespace QueryOptimizerTests {
}
shared_ptr<QueryPlanSet> makeQps( const BSONObj& query = BSONObj(),
const BSONObj& order = BSONObj(),
- const BSONObj& hint = BSONObj() ) {
+ const BSONObj& hint = BSONObj(),
+ bool allowSpecial = true ) {
auto_ptr<FieldRangeSetPair> frsp( new FieldRangeSetPair( ns(), query ) );
auto_ptr<FieldRangeSetPair> frspOrig( new FieldRangeSetPair( *frsp ) );
return shared_ptr<QueryPlanSet>
( QueryPlanSet::make( ns(), frsp, frspOrig, query, order,
shared_ptr<const ParsedQuery>(), hint,
- QueryPlanGenerator::Use, BSONObj(), BSONObj() ) );
+ QueryPlanGenerator::Use, BSONObj(), BSONObj(),
+ allowSpecial ) );
}
static const char *ns() { return "unittests.QueryPlanSetTests"; }
static NamespaceDetails *nsd() { return nsdetails( ns() ); }
@@ -1283,6 +1285,46 @@ namespace QueryOptimizerTests {
ASSERT_EQUALS( BSON( "$natural" << 1 ), qps->firstPlan()->indexKey() );
}
};
+
+ /** Special plans are only selected when allowed. */
+ class AllowSpecial : public Base {
+ public:
+ void run() {
+ BSONObj naturalIndex = BSON( "$natural" << 1 );
+ BSONObj specialIndex = BSON( "a" << "2d" );
+ BSONObj query = BSON( "a" << BSON_ARRAY( 0 << 0 ) );
+ client().ensureIndex( ns(), specialIndex );
+
+ // The special plan is chosen if allowed.
+ assertSingleIndex( specialIndex, makeQps( query ) );
+
+ // The special plan is not chosen if not allowed
+ assertSingleIndex( naturalIndex, makeQps( query, BSONObj(), BSONObj(), false ) );
+
+ // Attempting to hint a special plan when not allowed triggers an assertion.
+ ASSERT_THROWS( makeQps( query, BSONObj(), BSON( "$hint" << specialIndex ), false ),
+ UserException );
+
+ // Attempting to use a geo operator when special plans are not allowed triggers an
+ // assertion.
+ ASSERT_THROWS( makeQps( BSON( "a" << BSON( "$near" << BSON_ARRAY( 0 << 0 ) ) ),
+ BSONObj(), BSONObj(), false ),
+ UserException );
+
+ // The special plan is not chosen if not allowed, even if cached.
+ NamespaceDetailsTransient &nsdt = NamespaceDetailsTransient::get( ns() );
+ nsdt.registerCachedQueryPlanForPattern
+ ( makePattern( query, BSONObj() ),
+ CachedQueryPlan( specialIndex, 1,
+ CandidatePlanCharacter( true, false ) ) );
+ assertSingleIndex( naturalIndex, makeQps( query, BSONObj(), BSONObj(), false ) );
+ }
+ private:
+ void assertSingleIndex( const BSONObj& index, const shared_ptr<QueryPlanSet>& set ) {
+ ASSERT_EQUALS( 1, set->nPlans() );
+ ASSERT_EQUALS( index, set->firstPlan()->indexKey() );
+ }
+ };
} // namespace QueryPlanSetTests
@@ -1537,6 +1579,7 @@ namespace QueryOptimizerTests {
add<QueryPlanSetTests::PossiblePlans>();
add<QueryPlanSetTests::AvoidUnhelpfulRecordedPlan>();
add<QueryPlanSetTests::AvoidDisallowedRecordedPlan>();
+ add<QueryPlanSetTests::AllowSpecial>();
add<MultiPlanScannerTests::ToString>();
add<MultiPlanScannerTests::PossiblePlans>();
add<BestGuess>();

0 comments on commit b2c99f1

Please sign in to comment.