Skip to content

Commit

Permalink
MB-53298 Do not return error for MISSING value in join processing
Browse files Browse the repository at this point in the history
Previous code for join assumes that the inner of a join should have
a valid value, since the join is being processed after a potential
match has been found. However, in this bug, we are doing the
equivalent of a cartesian join, and an explicit MISSING value is
used on the inner side of a join. This triggers a sanity check
failure.

The fix is to allow such MISSING value to proceed with join
processing. Also optimizer hints processing is fixed when joining
on expression/subquery terms.

Change-Id: I2ce8ee1759dc1547451241cfa6f19102d089639b
Reviewed-on: https://review.couchbase.org/c/query/+/178581
Reviewed-by: Sitaram Vemulapalli <sitaram.vemulapalli@couchbase.com>
Tested-by: Bingjie Miao <bingjie.miao@couchbase.com>
  • Loading branch information
miaobingjie committed Aug 8, 2022
1 parent 8b1553f commit 5fcf49b
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 10 deletions.
16 changes: 6 additions & 10 deletions execution/join_nl.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ package execution

import (
"encoding/json"
"fmt"

"github.com/couchbase/query/errors"
"github.com/couchbase/query/expression"
Expand Down Expand Up @@ -202,15 +201,12 @@ func processAnsiExec(item value.AnnotatedValue, right_item value.AnnotatedValue,
for _, alias := range aliases {
val, ok := right_item.Field(alias)
if !ok {
if len(aliases) > 1 {
// in case of HASH JOIN, when there are multiple aliases on the
// build side, value could be MISSING for an alias, if right_item
// contains the result of an outer join
continue
} else {
context.Error(errors.NewExecutionInternalError(fmt.Sprintf("processAnsiExec: annotated value not found for %s", alias)))
return false, false, nil
}
// it's possible to have a MISSING value for an alias, e.g.,
// in case of HASH JOIN, when there are multiple aliases on the
// build side, value could be MISSING for an alias, if right_item
// contains the result of an outer join;
// or if a MISSING value is specified explicitly in an expression term
continue
}

joined.SetField(alias, val)
Expand Down
30 changes: 30 additions & 0 deletions planner/build_join_ansi.go
Original file line number Diff line number Diff line change
Expand Up @@ -393,13 +393,28 @@ func (this *builder) buildAnsiJoinOp(node *algebra.AnsiJoin) (op plan.Operator,
return hjoin, err
}
}
} else {
if preferHash {
if inferJoinHint {
leftBaseKeyspace.MarkHashUnavailable()
} else {
baseKeyspace.MarkHashUnavailable()
}
}
}

scans, newOnclause, cost, cardinality, size, frCost, err := this.buildAnsiJoinSimpleFromTerm(right, node.Onclause(), node.Outer(), "join")
if err != nil {
return nil, err
}

if preferHash && !joinEnum {
if inferJoinHint {
leftBaseKeyspace.SetJoinHintError()
} else {
baseKeyspace.SetJoinHintError()
}
}
if newOnclause != nil {
node.SetOnclause(newOnclause)
}
Expand Down Expand Up @@ -697,13 +712,28 @@ func (this *builder) buildAnsiNestOp(node *algebra.AnsiNest) (op plan.Operator,
return hnest, err
}
}
} else {
if preferHash {
if inferJoinHint {
leftBaseKeyspace.MarkHashUnavailable()
} else {
baseKeyspace.MarkHashUnavailable()
}
}
}

scans, newOnclause, cost, cardinality, size, frCost, err := this.buildAnsiJoinSimpleFromTerm(right, node.Onclause(), node.Outer(), "nest")
if err != nil {
return nil, err
}

if preferHash && !joinEnum {
if inferJoinHint {
leftBaseKeyspace.SetJoinHintError()
} else {
baseKeyspace.SetJoinHintError()
}
}
if newOnclause != nil {
node.SetOnclause(newOnclause)
}
Expand Down
44 changes: 44 additions & 0 deletions test/gsi/test_cases/ansijoin/case_ansijoin_bugs.json
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,50 @@
"c11": 4
}
]
},
{
"testcase": "MB-53298, part 1",
"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` = 'HashJoin' AND v.`outer` = true END"
},
"statements":"SELECT x, y FROM [1, 2] AS x LEFT JOIN [1, MISSING] AS y ON x = y",
"results": [
{
"x": 1,
"y": 1
},
{
"x": 2
}
]
},
{
"testcase": "MB-53298, part 2",
"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` = 'NestedLoopJoin' AND v.`outer` = true END"
},
"statements":"SELECT x, y FROM [1, 2] AS x LEFT JOIN [1, MISSING] AS y USE HASH(PROBE) ON x = y",
"results": [
{
"x": 1,
"y": 1
},
{
"x": 2
}
]
}
]

0 comments on commit 5fcf49b

Please sign in to comment.