Permalink
Browse files

SERVER-3192 SERVER-2585 allow nested or clauses in queries, opaque to…

… indexing system
  • Loading branch information...
1 parent e47444f commit 0b32ad4e6d5629525001add932fe78ba517cb503 @astaple astaple committed Jun 15, 2011
Showing with 296 additions and 110 deletions.
  1. +71 −61 db/matcher.cpp
  2. +14 −20 db/matcher.h
  3. +5 −4 db/matcher_covered.cpp
  4. +14 −4 jstests/and.js
  5. +27 −0 jstests/and2.js
  6. +66 −0 jstests/and3.js
  7. +64 −18 jstests/andor.js
  8. +0 −1 jstests/or2.js
  9. +0 −2 jstests/or3.js
  10. +35 −0 jstests/orj.js
View
@@ -280,65 +280,43 @@ namespace mongo {
return true;
}
- void Matcher::parseOr( const BSONElement &e, bool nested, list< shared_ptr< Matcher > > &matchers ) {
- uassert( 13090, "nested $or/$nor not allowed", !nested );
- uassert( 13086, "$or/$nor must be a nonempty array", e.type() == Array && e.embeddedObject().nFields() > 0 );
+ void Matcher::parseExtractedClause( const BSONElement &e, list< shared_ptr< Matcher > > &matchers ) {
+ uassert( 13086, "$and/$or/$nor must be a nonempty array", e.type() == Array && e.embeddedObject().nFields() > 0 );
BSONObjIterator j( e.embeddedObject() );
while( j.more() ) {
BSONElement f = j.next();
- uassert( 13087, "$or/$nor match element must be an object", f.type() == Object );
- // until SERVER-109 this is never a covered index match, so don't constrain index key for $or matchers
- matchers.push_back( shared_ptr< Matcher >( new Matcher( f.embeddedObject(), true ) ) );
+ uassert( 13087, "$and/$or/$nor match element must be an object", f.type() == Object );
+ matchers.push_back( shared_ptr< Matcher >( new Matcher( f.embeddedObject() ) ) );
}
}
- bool Matcher::parseOrNor( const BSONElement &e, bool nested ) {
+ bool Matcher::parseClause( const BSONElement &e ) {
const char *ef = e.fieldName();
if ( ef[ 0 ] != '$' )
return false;
- if ( ef[ 1 ] == 'o' && ef[ 2 ] == 'r' && ef[ 3 ] == 0 ) {
- parseOr( e, nested, _orMatchers );
+ if ( ef[ 1 ] == 'a' && ef[ 2 ] == 'n' && ef[ 3 ] == 'd' ) {
+ parseExtractedClause( e, _andMatchers );
+ }
+ else if ( ef[ 1 ] == 'o' && ef[ 2 ] == 'r' && ef[ 3 ] == 0 ) {
+ parseExtractedClause( e, _orMatchers );
}
else if ( ef[ 1 ] == 'n' && ef[ 2 ] == 'o' && ef[ 3 ] == 'r' && ef[ 4 ] == 0 ) {
- parseOr( e, nested, _norMatchers );
+ parseExtractedClause( e, _norMatchers );
}
else {
return false;
}
return true;
}
- bool Matcher::parseAnd( const BSONElement &e, bool nested ) {
- const char *ef = e.fieldName();
- if (!( ef[ 0 ] == '$' && ef[ 1 ] == 'a' && ef[ 2 ] == 'n' && ef[ 3 ] == 'd' && ef[ 4 ] == 0 )) {
- return false;
- }
-
- uassert( 14815 , "$and expression must be a nonempty array" , e.type() == Array && e.embeddedObject().nFields() > 0 );
- BSONObjIterator i( e.embeddedObject() );
- while( i.more() ) {
- BSONElement e = i.next();
- uassert( 14818 , "$and elements must be objects" , e.type() == Object );
- BSONObjIterator j( e.embeddedObject() );
- while( j.more() ) {
- parseMatchExpressionElement( j.next(), true );
- }
- }
- return true;
- }
-
- void Matcher::parseMatchExpressionElement( const BSONElement &e, bool nested ) {
+ void Matcher::parseMatchExpressionElement( const BSONElement &e ) {
uassert( 13629 , "can't have undefined in a query expression" , e.type() != Undefined );
- if ( parseAnd( e, nested ) ) {
+ if ( parseClause( e ) ) {
return;
}
- if ( parseOrNor( e, nested ) ) {
- return;
- }
-
if ( ( e.type() == CodeWScope || e.type() == Code || e.type() == String ) && strcmp(e.fieldName(), "$where")==0 ) {
// $where: function()...
uassert( 10066 , "$where may only appear once in query", where == 0 );
@@ -437,26 +415,27 @@ namespace mongo {
/* _jsobj - the query pattern
*/
- Matcher::Matcher(const BSONObj &_jsobj, bool nested) :
+ Matcher::Matcher(const BSONObj &_jsobj) :
where(0), jsobj(_jsobj), haveSize(), all(), hasArray(0), haveNeg(), _atomic(false), nRegex(0) {
BSONObjIterator i(jsobj);
while ( i.more() ) {
- parseMatchExpressionElement( i.next(), nested );
+ parseMatchExpressionElement( i.next() );
}
}
Matcher::Matcher( const Matcher &other, const BSONObj &key ) :
where(0), constrainIndexKey_( key ), haveSize(), all(), hasArray(0), haveNeg(), _atomic(false), nRegex(0) {
- // do not include field matches which would make keyMatch() false
+ // Filter out match components that will provide an incorrect result
+ // given a key from a single key index.
for( vector< ElementMatcher >::const_iterator i = other.basics.begin(); i != other.basics.end(); ++i ) {
if ( key.hasField( i->toMatch.fieldName() ) ) {
switch( i->compareOp ) {
case BSONObj::opSIZE:
case BSONObj::opALL:
case BSONObj::NE:
case BSONObj::NIN:
- case BSONObj::opEXISTS: // Not part of keyMatch() determination, but we can't match on index in this case.
+ case BSONObj::opEXISTS: // We can't match on index in this case.
case BSONObj::opTYPE: // For $type:10 (null), a null key could be a missing field or a null value field.
break;
case BSONObj::opIN: {
@@ -487,6 +466,10 @@ namespace mongo {
regexs[ nRegex++ ] = other.regexs[ i ];
}
}
+ // Recursively filter match components for and and or matchers.
+ for( list< shared_ptr< Matcher > >::const_iterator i = other._andMatchers.begin(); i != other._andMatchers.end(); ++i ) {
+ _andMatchers.push_back( shared_ptr< Matcher >( new Matcher( **i, key ) ) );
+ }
for( list< shared_ptr< Matcher > >::const_iterator i = other._orMatchers.begin(); i != other._orMatchers.end(); ++i ) {
_orMatchers.push_back( shared_ptr< Matcher >( new Matcher( **i, key ) ) );
}
@@ -874,13 +857,32 @@ 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 ) {
+ // SERVER-3192 Track field matched using details the same as for
+ // top level fields, at least for now.
+ if ( !(*i)->matches( jsobj, details ) ) {
+ return false;
+ }
+ }
+ }
+
if ( _orMatchers.size() > 0 ) {
bool match = false;
for( list< shared_ptr< Matcher > >::const_iterator i = _orMatchers.begin();
i != _orMatchers.end(); ++i ) {
// SERVER-205 don't submit details - we don't want to track field
- // matched within $or, and at this point we've already loaded the
- // whole document
+ // matched within $or
if ( (*i)->matches( jsobj ) ) {
match = true;
break;
@@ -895,21 +897,13 @@ namespace mongo {
for( list< shared_ptr< Matcher > >::const_iterator i = _norMatchers.begin();
i != _norMatchers.end(); ++i ) {
// SERVER-205 don't submit details - we don't want to track field
- // matched within $nor, and at this point we've already loaded the
- // whole document
+ // matched within $nor
if ( (*i)->matches( jsobj ) ) {
return false;
}
}
}
- for( vector< shared_ptr< FieldRangeVector > >::const_iterator i = _orConstraints.begin();
- i != _orConstraints.end(); ++i ) {
- if ( (*i)->matches( jsobj ) ) {
- return false;
- }
- }
-
if ( where ) {
if ( where->func == 0 ) {
uassert( 10070 , "$where compile error", false);
@@ -948,39 +942,55 @@ namespace mongo {
return false;
}
- bool Matcher::sameCriteriaCount( const Matcher &other ) const {
- if ( !( basics.size() == other.basics.size() && nRegex == other.nRegex && !where == !other.where ) ) {
+ bool Matcher::keyMatch( const Matcher &docMatcher ) const {
+ // Quick check certain non key match cases.
+ if ( docMatcher.all
+ || docMatcher.haveSize
+ || docMatcher.hasArray // We can't match an array to its first indexed element using keymatch
+ || docMatcher.haveNeg ) {
+ return false;
+ }
+
+ // Check that all match components are available in the index matcher.
+ if ( !( basics.size() == docMatcher.basics.size() && nRegex == docMatcher.nRegex && !where == !docMatcher.where ) ) {
+ return false;
+ }
+ if ( _andMatchers.size() != docMatcher._andMatchers.size() ) {
return false;
}
- if ( _norMatchers.size() != other._norMatchers.size() ) {
+ if ( _orMatchers.size() != docMatcher._orMatchers.size() ) {
return false;
}
- if ( _orMatchers.size() != other._orMatchers.size() ) {
+ if ( _norMatchers.size() != docMatcher._norMatchers.size() ) {
return false;
}
- if ( _orConstraints.size() != other._orConstraints.size() ) {
+ if ( _orDedupConstraints.size() != docMatcher._orDedupConstraints.size() ) {
return false;
}
+
+ // Recursively check that all submatchers support key match.
{
- list< shared_ptr< Matcher > >::const_iterator i = _norMatchers.begin();
- list< shared_ptr< Matcher > >::const_iterator j = other._norMatchers.begin();
- while( i != _norMatchers.end() ) {
- if ( !(*i)->sameCriteriaCount( **j ) ) {
+ list< shared_ptr< Matcher > >::const_iterator i = _andMatchers.begin();
+ list< shared_ptr< Matcher > >::const_iterator j = docMatcher._andMatchers.begin();
+ while( i != _andMatchers.end() ) {
+ if ( !(*i)->keyMatch( **j ) ) {
return false;
}
++i; ++j;
}
}
{
list< shared_ptr< Matcher > >::const_iterator i = _orMatchers.begin();
- list< shared_ptr< Matcher > >::const_iterator j = other._orMatchers.begin();
+ list< shared_ptr< Matcher > >::const_iterator j = docMatcher._orMatchers.begin();
while( i != _orMatchers.end() ) {
- if ( !(*i)->sameCriteriaCount( **j ) ) {
+ if ( !(*i)->keyMatch( **j ) ) {
return false;
}
++i; ++j;
}
}
+ // Nor matchers and or dedup constraints aren't created for index matchers,
+ // so no need to check those here.
return true;
}
View
@@ -134,22 +134,12 @@ namespace mongo {
return op <= BSONObj::LTE ? -1 : 1;
}
- Matcher(const BSONObj &pattern, bool nested = false);
+ Matcher(const BSONObj &pattern);
~Matcher();
bool matches(const BSONObj& j, MatchDetails * details = 0 );
- // fast rough check to see if we must load the real doc - we also
- // compare field counts against covereed index matcher; for $or clauses
- // we just compare field counts
- bool keyMatch() const { return
- !all
- && !haveSize
- && !hasArray // We can't match an array to its first indexed element using keymatch
- && !haveNeg;
- }
-
bool atomic() const { return _atomic; }
bool hasType( BSONObj::MatchType type ) const;
@@ -158,15 +148,19 @@ namespace mongo {
return jsobj.toString();
}
- void addOrConstraint( const shared_ptr< FieldRangeVector > &frv ) {
- _orConstraints.push_back( frv );
+ void addOrDedupConstraint( const shared_ptr< FieldRangeVector > &frv ) {
+ _orDedupConstraints.push_back( frv );
}
void popOrClause() {
_orMatchers.pop_front();
}
- bool sameCriteriaCount( const Matcher &other ) const;
+ /**
+ * @return true if this key matcher will return the same true/false
+ * value as the provided doc matcher.
+ */
+ bool keyMatch( const Matcher &docMatcher ) const;
private:
// Only specify constrainIndexKey if matches() will be called with
@@ -185,11 +179,10 @@ namespace mongo {
int valuesMatch(const BSONElement& l, const BSONElement& r, int op, const ElementMatcher& bm);
- bool parseOrNor( const BSONElement &e, bool nested );
- void parseOr( const BSONElement &e, bool nested, list< shared_ptr< Matcher > > &matchers );
- bool parseAnd( const BSONElement &e, bool nested );
+ bool parseClause( const BSONElement &e );
+ void parseExtractedClause( const BSONElement &e, list< shared_ptr< Matcher > > &matchers );
- void parseMatchExpressionElement( const BSONElement &e, bool subMatcher );
+ void parseMatchExpressionElement( const BSONElement &e );
Where *where; // set if query uses $where
BSONObj jsobj; // the query pattern. e.g., { name: "joe" }
@@ -212,9 +205,10 @@ namespace mongo {
// so we delete the mem when we're done:
vector< shared_ptr< BSONObjBuilder > > _builders;
+ list< shared_ptr< Matcher > > _andMatchers;
list< shared_ptr< Matcher > > _orMatchers;
list< shared_ptr< Matcher > > _norMatchers;
- vector< shared_ptr< FieldRangeVector > > _orConstraints;
+ vector< shared_ptr< FieldRangeVector > > _orDedupConstraints;
friend class CoveredIndexMatcher;
};
@@ -238,7 +232,7 @@ namespace mongo {
// once this is called, shouldn't use this matcher for matching any more
void advanceOrClause( const shared_ptr< FieldRangeVector > &frv ) {
- _docMatcher->addOrConstraint( frv );
+ _docMatcher->addOrDedupConstraint( frv );
// TODO this is not yet optimal. Since we could skip an entire
// or clause (if a match is impossible) between calls to advanceOrClause()
// we may not pop all the clauses we can.
View
@@ -46,13 +46,15 @@ namespace mongo {
void CoveredIndexMatcher::init( bool alwaysUseRecord ) {
_needRecord =
alwaysUseRecord ||
- ! ( _docMatcher->keyMatch() &&
- _keyMatcher.sameCriteriaCount( *_docMatcher ) );
+ !_keyMatcher.keyMatch( *_docMatcher );
}
bool CoveredIndexMatcher::matchesCurrent( Cursor * cursor , MatchDetails * details ) {
// bool keyUsable = ! cursor->isMultiKey() && check for $orish like conditions in matcher SERVER-1264
- return matches( cursor->currKey() , cursor->currLoc() , details , !cursor->isMultiKey() );
+ return matches( cursor->currKey() , cursor->currLoc() , details ,
+ !cursor->indexKeyPattern().isEmpty() // unindexed cursor
+ && !cursor->isMultiKey() // multikey cursor
+ );
}
bool CoveredIndexMatcher::matches(const BSONObj &key, const DiskLoc &recLoc , MatchDetails * details , bool keyUsable ) {
@@ -77,5 +79,4 @@ namespace mongo {
return _docMatcher->matches(recLoc.obj() , details );
}
-
}
View
@@ -52,7 +52,9 @@ function check() {
assert.eq( 0, t.count( {$and:[{a:/^F/},{a:'foo'}]} ) );
assert.eq( 1, t.count( {$and:[{a:/^F/i},{a:'foo'}]} ) );
- // Check operators
+
+
+ // Check operator
assert.eq( 1, t.count( {$and:[{a:{$gt:0}}]} ) );
// Check where
@@ -63,9 +65,7 @@ function check() {
// Nested where ok
assert.eq( 1, t.count({$and:[{$where:'this.a=="foo"'}]}) );
assert.eq( 1, t.count({$and:[{a:'foo'},{$where:'this.a=="foo"'}]}) );
-
- // Only one where allowed
- assert.throws( function() { t.find( {$and:[{$where:'this.a=="foo"'}],$where:'this.a=="foo"'} ).toArray(); } );
+ assert.eq( 1, t.count({$and:[{$where:'this.a=="foo"'}],$where:'this.a=="foo"'}) );
}
check();
@@ -74,3 +74,13 @@ check();
var e = t.find( {$and:[{a:1}]} ).explain();
assert.eq( 'BtreeCursor a_1', e.cursor );
assert.eq( [[1,1]], e.indexBounds.a );
+
+function checkBounds( query ) {
+ var e = t.find( query ).explain();
+ assert.eq( 1, e.n );
+ assert.eq( [[1,1]], e.indexBounds.a );
+}
+
+// Since this is a multikey index, we get the bounds from the first constraint scanned.
+checkBounds( {a:1,$and:[{a:2}]} );
+checkBounds( {$and:[{a:1},{a:2}]} );
Oops, something went wrong.

0 comments on commit 0b32ad4

Please sign in to comment.