Skip to content

Commit

Permalink
MB-35392 Make sure ANY clause is properly covered by array index
Browse files Browse the repository at this point in the history
Change-Id: Ib26ae94c2b1a5bc687ec0154b9fc91a12c1ffca3
Reviewed-on: http://review.couchbase.org/112882
Reviewed-by: Sitaram Vemulapalli <sitaram.vemulapalli@couchbase.com>
Tested-by: Bingjie Miao <bingjie.miao@couchbase.com>
  • Loading branch information
miaobingjie committed Aug 3, 2019
1 parent 978d568 commit 13234e4
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 7 deletions.
1 change: 1 addition & 0 deletions expression/base.go
Expand Up @@ -53,6 +53,7 @@ const (
type CoveredOptions struct {
skip bool
trickle bool
chkvar bool
}

func (this *ExpressionBase) String() string {
Expand Down
7 changes: 6 additions & 1 deletion expression/expression.go
Expand Up @@ -349,6 +349,11 @@ Function rather than method to make sure we don't pick up
ExpressionBase.CoveredBy() in error
*/
func IsCovered(expr Expression, keyspace string, exprs Expressions) bool {
isCovered := expr.CoveredBy(keyspace, exprs, CoveredOptions{skip: false, trickle: false})
isCovered := expr.CoveredBy(keyspace, exprs, CoveredOptions{skip: false, trickle: false, chkvar: false})
return isCovered == CoveredSkip || isCovered == CoveredEquiv || isCovered == CoveredTrue
}

func IsVarCovered(expr Expression, keyspace string, exprs Expressions) bool {
isCovered := expr.CoveredBy(keyspace, exprs, CoveredOptions{skip: false, trickle: false, chkvar: true})
return isCovered == CoveredSkip || isCovered == CoveredEquiv || isCovered == CoveredTrue
}
18 changes: 13 additions & 5 deletions expression/identifier.go
Expand Up @@ -106,18 +106,26 @@ func (this *Identifier) CoveredBy(keyspace string, exprs Expressions, options Co
// MB-25317, if this is not the right keyspace, ignore the expression altogether
// MB-25370 this only applies for keyspace terms, not variables!
if (this.IsKeyspaceAlias() && this.identifier != keyspace) ||
this.IsProjectionAlias() || this.IsBindingVariable() {
this.IsProjectionAlias() || (!options.chkvar && this.IsBindingVariable()) {
return CoveredSkip
}

for _, expr := range exprs {
if this.EquivalentTo(expr) {
switch eType := expr.(type) {
case *Identifier:
if !this.IsKeyspaceAlias() {
return CoveredTrue
} else if eType.identifier != keyspace {
return CoveredSkip
if options.chkvar && this.IsBindingVariable() {
if eType.identifier == keyspace {
return CoveredTrue
} else {
return CoveredSkip
}
} else {
if !this.IsKeyspaceAlias() {
return CoveredTrue
} else if eType.identifier != keyspace {
return CoveredSkip
}
}
default:
return CoveredTrue
Expand Down
2 changes: 1 addition & 1 deletion planner/sarg_any.go
Expand Up @@ -107,7 +107,7 @@ func anySargFor(pred, key, cond expression.Expression, isJoin, doSelec bool,
}
}

if err != nil || !expression.IsCovered(pred, alias, exprs) {
if err != nil || !expression.IsVarCovered(pred, alias, exprs) {
sp.SetExact(false)
}

Expand Down
35 changes: 35 additions & 0 deletions test/gsi/test_cases/cover/case_cover2.json
@@ -0,0 +1,35 @@
[
{
"testcase": "ANY clause NOT covered",
"ignore": "index_id",
"explain": {
"disabled": false,
"results": [
{
"present": true
}
],
"statement": "SELECT true AS present FROM $explan AS p WHERE ANY v WITHIN p.plan.`~children` SATISFIES v.`#operator` LIKE 'IndexScan%' AND v.covers IS MISSING END"
},
"statements":"SELECT RAW meta().id FROM shellTest WHERE type=\"doc\" AND owner=\"xyz\" AND ANY v IN Names SATISFIES (v.fname='John' AND v.lname ='Doe') END",
"results": [
]
},
{
"testcase": "ANY clause covered",
"ignore": "index_id",
"explain": {
"disabled": false,
"results": [
{
"present": true
}
],
"statement": "SELECT true AS present FROM $explan AS p WHERE ANY v WITHIN p.plan.`~children` SATISFIES v.`#operator` LIKE 'IndexScan%' AND ARRAY_LENGTH(v.covers) > 0 END"
},
"statements":"SELECT RAW meta().id FROM shellTest WHERE type=\"doc\" AND owner=\"xyz\" AND ANY v IN Names SATISFIES (v.fname='John') END",
"results": [
]
}
]

4 changes: 4 additions & 0 deletions test/gsi/test_cases/cover/cover_test.go
Expand Up @@ -31,9 +31,12 @@ func TestCover(t *testing.T) {
runStmt(qc, "CREATE INDEX ixCover5 on shellTest (email,VMs,join_day) WHERE (10 < join_day)")
runStmt(qc, "CREATE INDEX ixCover6 on shellTest(main.status)")
runStmt(qc, "CREATE INDEX ixCover7 on shellTest(main.owner)")
runStmt(qc, "CREATE INDEX ixCover8 on shellTest(ALL ARRAY v.fname FOR v IN Names END) WHERE type=\"doc\" AND owner=\"xyz\"")

runMatch("case_cover.json", false, false, qc, t)

runMatch("case_cover2.json", false, true, qc, t)

runStmt(qc, "DROP PRIMARY INDEX on shellTest")
runStmt(qc, "DROP INDEX shellTest.ixCover")
runStmt(qc, "DROP INDEX shellTest.ixCover2")
Expand All @@ -42,4 +45,5 @@ func TestCover(t *testing.T) {
runStmt(qc, "DROP INDEX shellTest.ixCover5")
runStmt(qc, "DROP INDEX shellTest.ixCover6")
runStmt(qc, "DROP INDEX shellTest.ixCover7")
runStmt(qc, "DROP INDEX shellTest.ixCover8")
}

0 comments on commit 13234e4

Please sign in to comment.