Skip to content

Commit

Permalink
MB-42474 Set exact spans properly in case of multiple spans for the s…
Browse files Browse the repository at this point in the history 
…ame index

The problematic query has a predicate with OR clause and uses the same index
with multiple spans. On one side of the OR we use 2 sargable keys and on the
other side of the OR we use 1 sargable key. Both sides have exact span set
to true, however there is a different predicate on the OR arm which is not
considered (since on that side only a single sargable key is available),
but we did not detect it since when we determine on index pushdown later on
we take the max of the sargable keys from all arms of the OR clause.

To fix this, we add logic in sargForOR, when we iterate through different arms
of the OR clause, borrow logic from index pushdown determination to properly
detect whether there is a predicate that is not sargable, in which case
set exact span to be false.

Change-Id: I147a0d94e9bcc21d8d2d5643db882a866f24b304
Reviewed-on: http://review.couchbase.org/c/query/+/139700
Reviewed-by: Sitaram Vemulapalli <sitaram.vemulapalli@couchbase.com>
Tested-by: Bingjie Miao <bingjie.miao@couchbase.com>
Reviewed-on: http://review.couchbase.org/c/query/+/139745
Well-Formed: Build Bot <build@couchbase.com>
  • Loading branch information
@miaobingjie
miaobingjie committed Nov 6, 2020
1 parent 9331b16 commit a5aa02a
Show file tree
Hide file tree
Showing 8 changed files with 86 additions and 23 deletions.
2 changes: 1 addition & 1 deletion planner/build_scan_secondary.go
View file
Expand Up @@ -550,7 +550,7 @@ func (this *builder) sargIndexes(baseKeyspace *base.BaseKeyspace, underHash bool
spans, exactSpans, err = SargForFilters(baseKeyspace.Filters(), se.keys,
se.maxKeys, underHash, this.useCBO, baseKeyspace)
} else {
spans, exactSpans, err = SargFor(baseKeyspace.DnfPred(), se.keys,
spans, exactSpans, err = SargFor(baseKeyspace.DnfPred(), se, se.keys,
se.maxKeys, orIsJoin, this.useCBO, baseKeyspace)
}
if err != nil || spans.Size() == 0 {
Expand Down
9 changes: 5 additions & 4 deletions planner/build_scan_unnest.go
View file
Expand Up @@ -101,7 +101,7 @@ func (this *builder) buildUnnestScan(node *algebra.KeyspaceTerm, from algebra.Fr
for _, unnest := range primaryUnnests {
for _, index := range unnestIndexes {
arrayKey := arrayKeys[index]
op, _, _, n, err = this.matchUnnest(node, pred, unnest, index, indexes[index], arrayKey, unnests)
op, _, _, n, err = this.matchUnnest(node, pred, unnest, indexes[index], arrayKey, unnests)
if err != nil {
return nil, 0, err
}
Expand Down Expand Up @@ -272,7 +272,7 @@ func collectUnnestIndexes(pred expression.Expression, indexes map[datastore.Inde
}

func (this *builder) matchUnnest(node *algebra.KeyspaceTerm, pred expression.Expression, unnest *algebra.Unnest,
index datastore.Index, entry *indexEntry, arrayKey *expression.All, unnests []*algebra.Unnest) (
entry *indexEntry, arrayKey *expression.All, unnests []*algebra.Unnest) (
plan.SecondaryScan, *algebra.Unnest, *expression.All, int, error) {

var sargKey expression.Expression
Expand Down Expand Up @@ -332,7 +332,7 @@ func (this *builder) matchUnnest(node *algebra.KeyspaceTerm, pred expression.Exp
continue
}

op, un, nArrayKey, n, err := this.matchUnnest(node, pred, u, index, entry, nestedArrayKey, unnests)
op, un, nArrayKey, n, err := this.matchUnnest(node, pred, u, entry, nestedArrayKey, unnests)
if err != nil {
return nil, nil, nil, 0, err
}
Expand Down Expand Up @@ -368,6 +368,7 @@ func (this *builder) matchUnnest(node *algebra.KeyspaceTerm, pred expression.Exp
}
}

index := entry.index
formalizer := expression.NewSelfFormalizer(node.Alias(), nil)
sargKeys := make(expression.Expressions, 0, len(index.RangeKey()))
for i, key := range index.RangeKey() {
Expand All @@ -394,7 +395,7 @@ func (this *builder) matchUnnest(node *algebra.KeyspaceTerm, pred expression.Exp
n = max
}

spans, exactSpans, err := SargFor(pred, sargKeys, n, false, this.useCBO, baseKeyspace)
spans, exactSpans, err := SargFor(pred, entry, sargKeys, n, false, this.useCBO, baseKeyspace)
if err != nil {
return nil, nil, nil, 0, err
}
Expand Down
7 changes: 4 additions & 3 deletions planner/build_scan_unnest_cov.go
View file
Expand Up @@ -33,7 +33,7 @@ func (this *builder) buildCoveringUnnestScan(node *algebra.KeyspaceTerm, pred ex
this.restoreIndexPushDowns(indexPushDowns, true)

entry := indexes[index]
cop, cun, err := this.buildOneCoveringUnnestScan(node, pred, index, entry, arrayKeys[index], unnests)
cop, cun, err := this.buildOneCoveringUnnestScan(node, pred, entry, arrayKeys[index], unnests)
if err != nil {
return nil, 0, err
}
Expand Down Expand Up @@ -75,11 +75,11 @@ func (this *builder) buildCoveringUnnestScan(node *algebra.KeyspaceTerm, pred ex
}

func (this *builder) buildOneCoveringUnnestScan(node *algebra.KeyspaceTerm, pred expression.Expression,
index datastore.Index, entry *indexEntry, arrKey *expression.All, unnests []*algebra.Unnest) (
entry *indexEntry, arrKey *expression.All, unnests []*algebra.Unnest) (
plan.SecondaryScan, map[*algebra.Unnest]bool, error) {

// Sarg and populate spans
op, unnest, arrayKey, _, err := this.matchUnnest(node, pred, unnests[0], index, entry, arrKey, unnests)
op, unnest, arrayKey, _, err := this.matchUnnest(node, pred, unnests[0], entry, arrKey, unnests)
if op == nil || err != nil {
return nil, nil, err
}
Expand All @@ -94,6 +94,7 @@ func (this *builder) buildOneCoveringUnnestScan(node *algebra.KeyspaceTerm, pred
expression.NewMeta(expression.NewIdentifier(alias)),
expression.NewFieldName("id", false))

index := entry.index
entry = entry.Copy()
unnestIdent := expression.NewIdentifier(unnest.Alias())
unnestIdent.SetUnnestAlias(true)
Expand Down
26 changes: 19 additions & 7 deletions planner/sarg.go
View file
Expand Up @@ -14,12 +14,12 @@ import (
base "github.com/couchbase/query/plannerbase"
)

func SargFor(pred expression.Expression, keys expression.Expressions, max int, isJoin, doSelec bool,
baseKeyspace *base.BaseKeyspace) (SargSpans, bool, error) {
func SargFor(pred expression.Expression, entry *indexEntry, keys expression.Expressions, max int,
isJoin, doSelec bool, baseKeyspace *base.BaseKeyspace) (SargSpans, bool, error) {

// Optimize top-level OR predicate
if or, ok := pred.(*expression.Or); ok {
return sargForOr(or, keys, max, isJoin, doSelec, baseKeyspace)
return sargForOr(or, entry, keys, max, isJoin, doSelec, baseKeyspace)
}

sargKeys := keys[0:max]
Expand All @@ -34,20 +34,32 @@ func SargFor(pred expression.Expression, keys expression.Expressions, max int, i
return composeSargSpan(sargSpans, exactSpan)
}

func sargForOr(or *expression.Or, keys expression.Expressions, max int, isJoin, doSelec bool,
baseKeyspace *base.BaseKeyspace) (SargSpans, bool, error) {
func sargForOr(or *expression.Or, entry *indexEntry, keys expression.Expressions, max int,
isJoin, doSelec bool, baseKeyspace *base.BaseKeyspace) (SargSpans, bool, error) {

exact := true
spans := make([]SargSpans, len(or.Operands()))
for i, c := range or.Operands() {
_, max, _ = SargableFor(c, keys, false, true) // Variable length sarging
s, ex, err := SargFor(c, keys, max, isJoin, doSelec, baseKeyspace)
_, max1, _ := SargableFor(c, keys, false, true) // Variable length sarging
s, ex, err := SargFor(c, entry, keys, max1, isJoin, doSelec, baseKeyspace)
if err != nil {
return nil, false, err
}

spans[i] = s
exact = exact && ex

if exact && (max1 < max) {
// check for non-sargable key in predicate
exprs, _, err := indexCoverExpressions(entry, keys[:max1], c, nil, baseKeyspace.Name())
if err != nil {
return nil, false, err
}

if !expression.IsCovered(c, baseKeyspace.Name(), exprs) {
exact = false
}
}
}

var rv SargSpans = NewUnionSpans(spans...)
Expand Down
21 changes: 21 additions & 0 deletions test/gsi/test_cases/indexga/case_indexga_bugs.json
View file
@@ -0,0 +1,21 @@
[
{
"testcase": "MB-42774",
"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.`index_group_aggs` IS MISSING END"
},
"statements": "SELECT count(1) AS cnt FROM orders WHERE (test_id = 'indexga' AND type = 'bugs') AND c1 = 'doc' AND (EVERY v IN a1 SATISFIES v.name != 'b1' END OR a1 IS MISSING)",
"results": [
{
"cnt": 2
}
]
}
]
28 changes: 22 additions & 6 deletions test/gsi/test_cases/indexga/case_indexga_primary.json
View file
Expand Up @@ -14,7 +14,7 @@
"statements": "SELECT COUNT(1) AS cnt FROM orders",
"results": [
{
"cnt": 47
"cnt": 51
}
]
},
Expand All @@ -33,7 +33,7 @@
"statements": "SELECT COUNT(*) AS cnt FROM orders",
"results": [
{
"cnt": 47
"cnt": 51
}
]
},
Expand Down Expand Up @@ -259,8 +259,8 @@
"statements": "SELECT COUNT(META().id) AS cnt, SUM(TONUMBER(SUBSTR(META().id,2,4))) AS s FROM orders",
"results": [
{
"cnt": 47,
"s": 61757
"cnt": 51,
"s": 77767
}
]
},
Expand All @@ -279,8 +279,8 @@
"statements": "SELECT COUNT(META().id) AS a, AVG(TONUMBER(SUBSTR(META().id,2,4))) AS s FROM orders",
"results": [
{
"a": 47,
"s": 1313.9787234042553
"a": 51,
"s": 1524.8431372549019
}
]
},
Expand Down Expand Up @@ -486,6 +486,22 @@
{
"cnt": 1,
"s": 3058
},
{
"cnt": 1,
"s": 4001
},
{
"cnt": 1,
"s": 4002
},
{
"cnt": 1,
"s": 4003
},
{
"cnt": 1,
"s": 4004
}
]
},
Expand Down
10 changes: 9 additions & 1 deletion test/gsi/test_cases/indexga/indexga_test.go
View file
Expand Up @@ -46,6 +46,7 @@ func run_test(qc *gsi.MockServer, t *testing.T, prepare bool) {
"case_indexga_primary.json",
"case_indexga_unionscan.json",
"case_indexga_intersectscan.json",
"case_indexga_bugs.json",
}
indexes := []string{
"CREATE PRIMARY INDEX oprimary ON orders",
Expand All @@ -59,7 +60,10 @@ func run_test(qc *gsi.MockServer, t *testing.T, prepare bool) {
"CREATE INDEX ixgar101 ON orders(c1,c0) WHERE test_id = 'indexga' AND type = 'numeric'",
"DROP INDEX orders.ixgar101",
"CREATE INDEX ixgar102 ON orders(c10) WHERE test_id = 'indexga' AND type = 'numeric'",
"DROP INDEX orders.ixgar102"}
"DROP INDEX orders.ixgar102",
"CREATE INDEX ixga201 ON orders(c1, a1) WHERE test_id = 'indexga' AND type = 'bugs'",
"DROP INDEX orders.ixga201",
}

var primary int
var testcases []string
Expand Down Expand Up @@ -91,6 +95,10 @@ func run_test(qc *gsi.MockServer, t *testing.T, prepare bool) {
primary, testcases = buildtestcase(cases, indexes, 5, 2, 7)
run_testcase(primary, prepare, qc, t, testcases)
runStmt(qc, indexes[11])

// misc bugs
primary, testcases = buildtestcase(cases, indexes, 6, 12, 13)
run_testcase(primary, prepare, qc, t, testcases)
}

func case_delete(qc *gsi.MockServer, t *testing.T) {
Expand Down
6 changes: 5 additions & 1 deletion test/gsi/test_cases/indexga/insert.json
View file
Expand Up @@ -45,5 +45,9 @@
{"statements":"INSERT INTO orders (KEY,VALUE) VALUES('ga3053', {'type':'mix','test_id':'indexga','c0':'e', 'c1':{'c11':11},'c2':2,'c4':null, 'c10':50,'a1':[1,1,2,2,3,4], 'a2':[1,1,2,2,3,4], 'a3':[{'id':1},{'id':1},{'id':2},{'id':3},{'id':4},{'id':5}],'a4':[{'id':1},{'id':1},{'id':2},{'id':3},{'id':4},{'id':5}]})"},
{"statements":"INSERT INTO orders (KEY,VALUE) VALUES('ga3056', {'type':'mix','test_id':'indexga','c0':['c0'], 'c1':{'c11':11},'c2':2,'c4':null, 'c10':50,'a1':[1,1,2,2,3,4], 'a2':[1,1,2,2,3,4], 'a3':[{'id':1},{'id':1},{'id':2},{'id':3},{'id':4},{'id':5}],'a4':[{'id':1},{'id':1},{'id':2},{'id':3},{'id':4},{'id':5}]})"},
{"statements":"INSERT INTO orders (KEY,VALUE) VALUES('ga3057', {'type':'mix','test_id':'indexga','c4':['c4'], 'c0':{'c01':1},'c2':2,'c4':null, 'c10':50,'a1':[1,1,2,2,3,4], 'a2':[1,1,2,2,3,4], 'a3':[{'id':1},{'id':1},{'id':2},{'id':3},{'id':4},{'id':5}],'a4':[{'id':1},{'id':1},{'id':2},{'id':3},{'id':4},{'id':5}]})"},
{"statements":"INSERT INTO orders (KEY,VALUE) VALUES('ga3058', {'type':'mix','test_id':'indexga','c4':['c4'], 'c0':0,'c2':2,'c4':null, 'c10':50,'a1':[1,1,2,2,3,4], 'a2':[1,1,2,2,3,4], 'a3':[{'id':1},{'id':1},{'id':2},{'id':3},{'id':4},{'id':5}],'a4':[{'id':1},{'id':1},{'id':2},{'id':3},{'id':4},{'id':5}]})"}
{"statements":"INSERT INTO orders (KEY,VALUE) VALUES('ga3058', {'type':'mix','test_id':'indexga','c4':['c4'], 'c0':0,'c2':2,'c4':null, 'c10':50,'a1':[1,1,2,2,3,4], 'a2':[1,1,2,2,3,4], 'a3':[{'id':1},{'id':1},{'id':2},{'id':3},{'id':4},{'id':5}],'a4':[{'id':1},{'id':1},{'id':2},{'id':3},{'id':4},{'id':5}]})"},
{"statements":"INSERT INTO orders (KEY,VALUE) VALUES('ga4001', {'type':'bugs','test_id':'indexga','c1':'doc', 'a1':[{'name':'a1'}, {'name':'a2'}]})"},
{"statements":"INSERT INTO orders (KEY,VALUE) VALUES('ga4002', {'type':'bugs','test_id':'indexga','c1':'doc', 'a1':[{'name':'b1'}, {'name':'a2'}]})"},
{"statements":"INSERT INTO orders (KEY,VALUE) VALUES('ga4003', {'type':'bugs','test_id':'indexga','c1':'doc', 'a1':[{'name':'b1'}, {'name':'a2'}]})"},
{"statements":"INSERT INTO orders (KEY,VALUE) VALUES('ga4004', {'type':'bugs','test_id':'indexga','c1':'doc'})"}
]

0 comments on commit a5aa02a

Please sign in to comment.