Skip to content

Commit

Permalink
Merge #38908 #38911
Browse files Browse the repository at this point in the history
38908: exec: Fix type ambiguity error out for IN and NOT IN. r=jordanlewis a=rohany

A previous PR introduced this error, this PR fixes it and adds a regression test.

Release note: None

38911: exec: fix LIKE handling for patterns like '%a%' r=solongordon a=solongordon

GetLikeOperator was choosing the wrong operator when both the first and
last characters of the pattern were wildcards. I fixed this logic and
improved the unit tests to cover GetLikeOperator rather than just the
underlying operators.

While I was here I also added better handling for the case where there
are no wildcards, i.e. exact string matching.

Fixes #38888

Release note: None

Co-authored-by: Rohan Yadav <rohany@alumni.cmu.edu>
Co-authored-by: Solon Gordon <solon@cockroachlabs.com>
  • Loading branch information
3 people committed Jul 16, 2019
3 parents ae3f070 + 5889f11 + 5bff499 commit e1061e2
Show file tree
Hide file tree
Showing 4 changed files with 129 additions and 41 deletions.
8 changes: 8 additions & 0 deletions pkg/sql/distsqlrun/column_exec_setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,14 @@ func assertHomogeneousTypes(expr tree.TypedExpr) error {
case *tree.ComparisonExpr:
left := t.TypedLeft().ResolvedType()
right := t.TypedRight().ResolvedType()

// Special rules for IN and NOT IN expressions. The type checker
// handles invalid types for the IN and NOT IN operations at this point,
// and we allow a comparison between t and t tuple.
if t.Operator == tree.In || t.Operator == tree.NotIn {
return nil
}

if !left.Identical(right) {
return errors.Errorf("ComparisonExpr on %s and %s is unhandled", left, right)
}
Expand Down
39 changes: 32 additions & 7 deletions pkg/sql/exec/like_ops.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,33 +45,54 @@ func GetLikeOperator(
return NewNoop(input), nil
}
if len(pattern) > 1 && !strings.ContainsAny(pattern[1:len(pattern)-1], "_%") {
// Special cases for patterns which are just a prefix or suffix.
if pattern[0] == '%' {
// There are no wildcards in the middle of the string, so we only need to
// use a regular expression if both the first and last characters are
// wildcards.
firstChar := pattern[0]
lastChar := pattern[len(pattern)-1]
if !isWildcard(firstChar) && !isWildcard(lastChar) {
// No wildcards, so this is just an exact string match.
if negate {
return &selNEBytesBytesConstOp{
input: input,
colIdx: colIdx,
constArg: []byte(pattern),
}, nil
}
return &selEQBytesBytesConstOp{
input: input,
colIdx: colIdx,
constArg: []byte(pattern),
}, nil
}
if firstChar == '%' && !isWildcard(lastChar) {
suffix := []byte(pattern[1:])
if negate {
return &selNotSuffixBytesBytesConstOp{
input: input,
colIdx: colIdx,
constArg: []byte(pattern[1:]),
constArg: suffix,
}, nil
}
return &selSuffixBytesBytesConstOp{
input: input,
colIdx: colIdx,
constArg: []byte(pattern[1:]),
constArg: suffix,
}, nil
}
if pattern[len(pattern)-1] == '%' {
if lastChar == '%' && !isWildcard(firstChar) {
prefix := []byte(pattern[:len(pattern)-1])
if negate {
return &selNotPrefixBytesBytesConstOp{
input: input,
colIdx: colIdx,
constArg: []byte(pattern[:len(pattern)-1]),
constArg: prefix,
}, nil
}
return &selPrefixBytesBytesConstOp{
input: input,
colIdx: colIdx,
constArg: []byte(pattern[:len(pattern)-1]),
constArg: prefix,
}, nil
}
}
Expand All @@ -93,3 +114,7 @@ func GetLikeOperator(
constArg: re,
}, nil
}

func isWildcard(c byte) bool {
return c == '%' || c == '_'
}
105 changes: 71 additions & 34 deletions pkg/sql/exec/like_ops_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,46 +16,83 @@ import (
"regexp"
"testing"

"github.com/cockroachdb/cockroach/pkg/settings/cluster"
"github.com/cockroachdb/cockroach/pkg/sql/exec/coldata"
"github.com/cockroachdb/cockroach/pkg/sql/exec/types"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/util/randutil"
)

func TestSelPrefixBytesBytesConstOp(t *testing.T) {
tups := tuples{{"abc"}, {"def"}, {"ghi"}}
runTests(t, []tuples{tups}, tuples{{"def"}}, orderedVerifier, []int{0}, func(input []Operator) (Operator, error) {
return &selPrefixBytesBytesConstOp{
input: input[0],
colIdx: 0,
constArg: []byte("de"),
}, nil
})
}

func TestSelSuffixBytesBytesConstOp(t *testing.T) {
tups := tuples{{"abc"}, {"def"}, {"ghi"}}
runTests(t, []tuples{tups}, tuples{{"def"}}, orderedVerifier, []int{0}, func(input []Operator) (Operator, error) {
return &selSuffixBytesBytesConstOp{
input: input[0],
colIdx: 0,
constArg: []byte("ef"),
}, nil
})
}

func TestSelRegexpBytesBytesConstOp(t *testing.T) {
tups := tuples{{"abc"}, {"def"}, {"ghi"}}
pattern, err := regexp.Compile(".e.")
if err != nil {
t.Fatal(err)
func TestLikeOperators(t *testing.T) {
for _, tc := range []struct {
pattern string
negate bool
tups tuples
expected tuples
}{
{
pattern: "def",
tups: tuples{{"abc"}, {"def"}, {"ghi"}},
expected: tuples{{"def"}},
},
{
pattern: "def",
negate: true,
tups: tuples{{"abc"}, {"def"}, {"ghi"}},
expected: tuples{{"abc"}, {"ghi"}},
},
{
pattern: "de%",
tups: tuples{{"abc"}, {"def"}, {"ghi"}},
expected: tuples{{"def"}},
},
{
pattern: "de%",
negate: true,
tups: tuples{{"abc"}, {"def"}, {"ghi"}},
expected: tuples{{"abc"}, {"ghi"}},
},
{
pattern: "%ef",
tups: tuples{{"abc"}, {"def"}, {"ghi"}},
expected: tuples{{"def"}},
},
{
pattern: "%ef",
negate: true,
tups: tuples{{"abc"}, {"def"}, {"ghi"}},
expected: tuples{{"abc"}, {"ghi"}},
},
{
pattern: "_e_",
tups: tuples{{"abc"}, {"def"}, {"ghi"}},
expected: tuples{{"def"}},
},
{
pattern: "_e_",
negate: true,
tups: tuples{{"abc"}, {"def"}, {"ghi"}},
expected: tuples{{"abc"}, {"ghi"}},
},
{
pattern: "%e%",
tups: tuples{{"abc"}, {"def"}, {"ghi"}},
expected: tuples{{"def"}},
},
{
pattern: "%e%",
negate: true,
tups: tuples{{"abc"}, {"def"}, {"ghi"}},
expected: tuples{{"abc"}, {"ghi"}},
},
} {
runTests(
t, []tuples{tc.tups}, tc.expected, orderedVerifier, []int{0},
func(input []Operator) (Operator, error) {
ctx := tree.MakeTestingEvalContext(cluster.MakeTestingClusterSettings())
return GetLikeOperator(&ctx, input[0], 0, tc.pattern, tc.negate)
})
}
runTests(t, []tuples{tups}, tuples{{"def"}}, orderedVerifier, []int{0}, func(input []Operator) (Operator, error) {
return &selRegexpBytesBytesConstOp{
input: input[0],
colIdx: 0,
constArg: pattern,
}, nil
})
}

func BenchmarkLikeOps(b *testing.B) {
Expand Down
18 changes: 18 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/vectorize
Original file line number Diff line number Diff line change
Expand Up @@ -455,3 +455,21 @@ query I
SELECT * FROM (SELECT count(*) AS x FROM b) WHERE x > 0;
----
4

# Regression test for #38908
statement ok
CREATE TABLE t38908 (x INT)

statement ok
INSERT INTO t38908 VALUES (1)

statement ok
SET experimental_vectorize=always

query I
SELECT * FROM t38908 WHERE x IN (1, 2)
----
1

statement ok
RESET experimental_vectorize

0 comments on commit e1061e2

Please sign in to comment.