diff --git a/sql/expression/function/instr_issue3650_test.go b/sql/expression/function/instr_issue3650_test.go new file mode 100644 index 0000000000..7e0c9ab1b4 --- /dev/null +++ b/sql/expression/function/instr_issue3650_test.go @@ -0,0 +1,66 @@ +// Copyright 2020-2021 Dolthub, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package function + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/dolthub/go-mysql-server/sql" + "github.com/dolthub/go-mysql-server/sql/expression" + "github.com/dolthub/go-mysql-server/sql/types" + "github.com/dolthub/go-mysql-server/test" +) + +// TestInstrIssue3650 covers the two defects reported in +// dolthub/go-mysql-server#3650: +// 1. INSTR was case-sensitive for nonbinary strings; MySQL's INSTR is +// case-insensitive unless one argument is a binary string. +// 2. A wrapped substring argument clobbered the haystack, so INSTR +// returned 1 unconditionally for any StringWrapper needle. +func TestInstrIssue3650(t *testing.T) { + f := NewInstr( + sql.NewEmptyContext(), + expression.NewGetField(0, types.LongText, "str", true), + expression.NewGetField(1, types.LongText, "substr", false), + ) + + testCases := []struct { + name string + row sql.Row + expected int + }{ + // Defect 1: case-insensitive for nonbinary strings. + {"case-insensitive needle", sql.NewRow("xyza", "A"), 4}, + {"case-insensitive haystack", sql.NewRow("XYZA", "a"), 4}, + {"case-insensitive mixed", sql.NewRow("Hello World", "world"), 7}, + {"case-insensitive no match", sql.NewRow("abc", "Z"), 0}, + // Defect 2: wrapped substring must NOT overwrite the haystack. + {"wrapped substr match", sql.NewRow("foobar", test.NewMockStringWrapper("bar")), 4}, + {"wrapped substr no match", sql.NewRow("foobar", test.NewMockStringWrapper("xyz")), 0}, + {"wrapped substr case-insensitive", sql.NewRow("foobar", test.NewMockStringWrapper("BAR")), 4}, + } + + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + require := require.New(t) + ctx := sql.NewEmptyContext() + v, err := f.Eval(ctx, tt.row) + require.NoError(err) + require.Equal(int64(tt.expected), v) + }) + } +} diff --git a/sql/expression/function/substring.go b/sql/expression/function/substring.go index 3b933133da..809270e7ac 100644 --- a/sql/expression/function/substring.go +++ b/sql/expression/function/substring.go @@ -18,6 +18,7 @@ import ( "fmt" "reflect" "strings" + "unicode" "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/go-mysql-server/sql/types" @@ -654,7 +655,7 @@ func (i Instr) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) { if err != nil { return nil, err } - text = []rune(s) + subtext = []rune(s) case []byte: subtext = []rune(string(substr)) case sql.BytesWrapper: @@ -676,7 +677,7 @@ func findSubsequence(text []rune, subtext []rune) int64 { for i := 0; i <= len(text)-len(subtext); i++ { var j int for j = 0; j < len(subtext); j++ { - if text[i+j] != subtext[j] { + if !runeEqualFold(text[i+j], subtext[j]) { break } } @@ -687,6 +688,13 @@ func findSubsequence(text []rune, subtext []rune) int64 { return -1 } +// runeEqualFold reports whether two runes are equal under simple case folding. +// MySQL's INSTR is case-insensitive unless one argument is a binary string, so +// the rune comparison folds case to match that behavior. +func runeEqualFold(a, b rune) bool { + return a == b || unicode.ToLower(a) == unicode.ToLower(b) +} + // IsNullable implements the Expression interface. func (i Instr) IsNullable(ctx *sql.Context) bool { return i.str.IsNullable(ctx) || i.substr.IsNullable(ctx)