Permalink
Browse files

Move or clause deduping from Matcher to CoveredIndexMatcher and make …

…both Matcher and CoveredIndexMatcher immutable.
  • Loading branch information...
1 parent 6fc8a94 commit 31be905b79c95b13d2f1dab3450066cd456b10e3 @astaple astaple committed May 25, 2012
Showing with 92 additions and 50 deletions.
  1. +43 −0 jstests/orq.js
  2. +0 −21 src/mongo/db/matcher.cpp
  3. +15 −14 src/mongo/db/matcher.h
  4. +32 −13 src/mongo/db/matcher_covered.cpp
  5. +2 −2 src/mongo/db/queryoptimizer.cpp
View
@@ -0,0 +1,43 @@
+// $or clause deduping with result set sizes > 101 (smaller result sets are now also deduped by the
+// query optimizer cursor).
+
+t = db.jstests_orq;
+t.drop();
+
+t.ensureIndex( { a:1 } );
+t.ensureIndex( { b:1 } );
+t.ensureIndex( { c:1 } );
+
+for( i = 0; i < 200; ++i ) {
+ t.save( { a:1, b:1 } );
+}
+
+// Deduping results from the previous clause.
+assert.eq( 200, t.count( { $or:[ { a:1 }, { b:1 } ] } ) );
+
+// Deduping results from a prior clause.
+assert.eq( 200, t.count( { $or:[ { a:1 }, { c:1 }, { b:1 } ] } ) );
+t.save( { c:1 } );
+assert.eq( 201, t.count( { $or:[ { a:1 }, { c:1 }, { b:1 } ] } ) );
+
+// Deduping results that would normally be index only matches on overlapping and double scanned $or
+// field regions.
+t.drop();
+t.ensureIndex( { a:1, b:1 } );
+for( i = 0; i < 16; ++i ) {
+ for( j = 0; j < 16; ++j ) {
+ t.save( { a:i, b:j } );
+ }
+}
+assert.eq( 16 * 16,
+ t.count( { $or:[ { a:{ $gte:0 }, b:{ $gte:0 } }, { a:{ $lte:16 }, b:{ $lte:16 } } ] } ) );
+
+// Deduping results from a clause that completed before the multi cursor takeover.
+t.drop();
+t.ensureIndex( { a:1 } );
+t.ensureIndex( { b:1 } );
+t.save( { a:1,b:200 } );
+for( i = 0; i < 200; ++i ) {
+ t.save( { b:i } );
+}
+assert.eq( 201, t.count( { $or:[ { a:1 }, { b:{ $gte:0 } } ] } ) );
@@ -911,15 +911,6 @@ namespace mongo {
return false;
}
- if ( _orDedupConstraints.size() > 0 ) {
- for( vector< shared_ptr< FieldRangeVector > >::const_iterator i = _orDedupConstraints.begin();
- i != _orDedupConstraints.end(); ++i ) {
- if ( (*i)->matches( jsobj ) ) {
- return false;
- }
- }
- }
-
if ( _andMatchers.size() > 0 ) {
for( list< shared_ptr< Matcher > >::const_iterator i = _andMatchers.begin();
i != _andMatchers.end(); ++i ) {
@@ -1051,15 +1042,6 @@ namespace mongo {
return false;
}
- if ( _orDedupConstraints.size() > 0 ) {
- for( vector< shared_ptr< FieldRangeVector > >::const_iterator i = _orDedupConstraints.begin();
- i != _orDedupConstraints.end(); ++i ) {
- if ( (*i)->matches( jsobj ) ) {
- return false;
- }
- }
- }
-
if ( _andMatchers.size() > 0 ) {
for( list< shared_ptr< Matcher > >::const_iterator i = _andMatchers.begin();
i != _andMatchers.end(); ++i ) {
@@ -1150,9 +1132,6 @@ namespace mongo {
if ( docMatcher._norMatchers.size() > 0 ) {
return false;
}
- if ( docMatcher._orDedupConstraints.size() > 0 ) {
- return false;
- }
// Recursively check that all submatchers support key match.
{
@@ -188,10 +188,6 @@ namespace mongo {
return _jsobj.toString();
}
- void addOrDedupConstraint( const shared_ptr< FieldRangeVector > &frv ) {
- _orDedupConstraints.push_back( frv );
- }
-
/**
* @return true if this key matcher will return the same true/false
* value as the provided doc matcher.
@@ -256,7 +252,6 @@ namespace mongo {
list< shared_ptr< Matcher > > _andMatchers;
list< shared_ptr< Matcher > > _orMatchers;
list< shared_ptr< Matcher > > _norMatchers;
- vector< shared_ptr< FieldRangeVector > > _orDedupConstraints;
friend class CoveredIndexMatcher;
};
@@ -277,23 +272,29 @@ namespace mongo {
Matcher& docMatcher() { return *_docMatcher; }
- // once this is called, shouldn't use this matcher for matching any more
- void advanceOrClause( const shared_ptr< FieldRangeVector > &frv ) {
- _docMatcher->addOrDedupConstraint( frv );
- }
-
- CoveredIndexMatcher *nextClauseMatcher( const BSONObj &indexKeyPattern ) {
- return new CoveredIndexMatcher( _docMatcher, indexKeyPattern );
+ /**
+ * @return a matcher for a following $or clause.
+ * @param prevClauseFrs The index range scanned by the previous $or clause. May be empty.
+ * @param nextClauseIndexKeyPattern The index key of the following $or clause.
+ */
+ CoveredIndexMatcher *nextClauseMatcher( const shared_ptr<FieldRangeVector> &prevClauseFrv,
+ const BSONObj &nextClauseIndexKeyPattern ) const {
+ return new CoveredIndexMatcher( *this, prevClauseFrv, nextClauseIndexKeyPattern );
}
string toString() const;
private:
- bool matches(const BSONObj &key, const DiskLoc &recLoc , MatchDetails * details = 0 , bool keyUsable = true );
- CoveredIndexMatcher(const shared_ptr< Matcher > &docMatcher, const BSONObj &indexKeyPattern);
+ bool matches( const BSONObj &key, const DiskLoc &recLoc, MatchDetails *details = 0,
+ bool keyUsable = true );
+ bool isOrClauseDup( const BSONObj &obj ) const;
+ CoveredIndexMatcher( const CoveredIndexMatcher &prevClauseMatcher,
+ const shared_ptr<FieldRangeVector> &prevClauseFrv,
+ const BSONObj &nextClauseIndexKeyPattern );
void init();
shared_ptr< Matcher > _docMatcher;
Matcher _keyMatcher;
+ vector<shared_ptr<FieldRangeVector> > _orDedupConstraints;
bool _needRecord; // if the key itself isn't good enough to determine a positive match
};
@@ -20,30 +20,34 @@
#include "pch.h"
#include "matcher.h"
-#include "../util/goodies.h"
-#include "diskloc.h"
-#include "../scripting/engine.h"
-#include "db.h"
-#include "client.h"
-
-#include "pdfile.h"
+#include "mongo/db/cursor.h"
+#include "mongo/db/queryutil.h"
namespace mongo {
- CoveredIndexMatcher::CoveredIndexMatcher( const BSONObj &jsobj, const BSONObj &indexKeyPattern ) :
+ CoveredIndexMatcher::CoveredIndexMatcher( const BSONObj &jsobj,
+ const BSONObj &indexKeyPattern ) :
_docMatcher( new Matcher( jsobj ) ),
_keyMatcher( *_docMatcher, indexKeyPattern ) {
init();
}
- CoveredIndexMatcher::CoveredIndexMatcher( const shared_ptr< Matcher > &docMatcher, const BSONObj &indexKeyPattern ) :
- _docMatcher( docMatcher ),
- _keyMatcher( *_docMatcher, indexKeyPattern ) {
+ CoveredIndexMatcher::CoveredIndexMatcher( const CoveredIndexMatcher &prevClauseMatcher,
+ const shared_ptr<FieldRangeVector> &prevClauseFrv,
+ const BSONObj &nextClauseIndexKeyPattern ) :
+ _docMatcher( prevClauseMatcher._docMatcher ),
+ _keyMatcher( *_docMatcher, nextClauseIndexKeyPattern ),
+ _orDedupConstraints( prevClauseMatcher._orDedupConstraints ) {
+ if ( prevClauseFrv ) {
+ _orDedupConstraints.push_back( prevClauseFrv );
+ }
init();
}
void CoveredIndexMatcher::init() {
- _needRecord = !_keyMatcher.keyMatch( *_docMatcher );
+ _needRecord =
+ !_keyMatcher.keyMatch( *_docMatcher ) ||
+ !_orDedupConstraints.empty();
}
bool CoveredIndexMatcher::matchesCurrent( Cursor * cursor , MatchDetails * details ) {
@@ -76,10 +80,25 @@ namespace mongo {
if ( details )
details->setLoadedRecord( true );
- bool res = _docMatcher->matches(recLoc.obj() , details );
+ BSONObj obj = recLoc.obj();
+ bool res =
+ _docMatcher->matches( obj, details ) &&
+ !isOrClauseDup( obj );
LOG(5) << "CoveredIndexMatcher _docMatcher->matches() returns " << res << endl;
return res;
}
+
+ bool CoveredIndexMatcher::isOrClauseDup( const BSONObj &obj ) const {
+ for( vector<shared_ptr<FieldRangeVector> >::const_iterator i = _orDedupConstraints.begin();
+ i != _orDedupConstraints.end(); ++i ) {
+ if ( (*i)->matches( obj ) ) {
+ // If a document matches a prior $or clause index range, generally it would have
+ // been returned while scanning that range and so is reported as a dup.
+ return true;
+ }
+ }
+ return false;
+ }
string CoveredIndexMatcher::toString() const {
StringBuilder buf;
@@ -1236,10 +1236,10 @@ namespace mongo {
void MultiCursor::advanceClause() {
_nscanned += _c->nscanned();
if ( _explainPlanInfo ) _explainPlanInfo->noteDone( *_c );
- _matcher->advanceOrClause( _queryPlan->originalFrv() );
+ shared_ptr<FieldRangeVector> oldClauseFrv = _queryPlan->originalFrv();
_queryPlan = _mps->nextClauseBestGuessPlan( *_queryPlan );
if ( _queryPlan ) {
- _matcher.reset( _matcher->nextClauseMatcher( _queryPlan->indexKey() ) );
+ _matcher.reset( _matcher->nextClauseMatcher( oldClauseFrv, _queryPlan->indexKey() ) );
_c = _queryPlan->newCursor();
// All sub cursors must support yields.
verify( _c->supportYields() );

0 comments on commit 31be905

Please sign in to comment.