From 13f6f04fb178450c351fdddd90c9a065555279fb Mon Sep 17 00:00:00 2001 From: Daylon Wilkins Date: Fri, 28 Feb 2025 05:05:45 -0800 Subject: [PATCH 1/2] Added regexp_instr and regexp_substr --- enginetest/queries/regex_queries.go | 48 +++++ go.mod | 2 +- go.sum | 8 +- sql/expression/function/regexp_instr.go | 255 +++++++++++++++++++++++ sql/expression/function/regexp_substr.go | 232 +++++++++++++++++++++ sql/expression/function/registry.go | 2 + 6 files changed, 540 insertions(+), 7 deletions(-) create mode 100644 sql/expression/function/regexp_instr.go create mode 100644 sql/expression/function/regexp_substr.go diff --git a/enginetest/queries/regex_queries.go b/enginetest/queries/regex_queries.go index cb82dfeaaa..830ba6fd3f 100644 --- a/enginetest/queries/regex_queries.go +++ b/enginetest/queries/regex_queries.go @@ -2107,4 +2107,52 @@ var RegexTests = []RegexTest{ Query: `SELECT REGEXP_LIKE("abc", "^([ab]*?)(? Date: Mon, 3 Mar 2025 05:18:20 -0800 Subject: [PATCH 2/2] PR feedback --- enginetest/engine_only_test.go | 41 +++++++++++++ go.mod | 2 +- go.sum | 4 +- sql/expression/function/regexp_instr.go | 43 +++++++++----- sql/expression/function/regexp_like.go | 65 ++++++++++++++------- sql/expression/function/regexp_like_test.go | 16 ++--- sql/expression/function/regexp_substr.go | 41 ++++++++----- 7 files changed, 148 insertions(+), 64 deletions(-) diff --git a/enginetest/engine_only_test.go b/enginetest/engine_only_test.go index 8e8866dfd1..4ea075ca3d 100644 --- a/enginetest/engine_only_test.go +++ b/enginetest/engine_only_test.go @@ -781,6 +781,47 @@ func TestRegex(t *testing.T) { }, }, }, + { + Name: "REGEXP caching behavior", + SetUpScript: []string{ + "CREATE TABLE test (v1 TEXT, v2 INT, v3 INT);", + "INSERT INTO test VALUES ('abc', 1, 2), ('[d-i]+', 2, 3), ('ghi', 3, 4);", + }, + Assertions: []queries.ScriptTestAssertion{ + { + Query: "SELECT REGEXP_LIKE('abc def ghi', 'abc') FROM test;", + Expected: []sql.Row{{1}, {1}, {1}}, + }, + { + Query: "SELECT REGEXP_LIKE('abc def ghi', v1) FROM test;", + Expected: []sql.Row{{1}, {1}, {1}}, + }, + { + Query: "SELECT REGEXP_INSTR('abc def ghi', '[a-z]+', 1, 2) FROM test;", + Expected: []sql.Row{{5}, {5}, {5}}, + }, + { + Query: "SELECT REGEXP_INSTR('abc def ghi', v1, 1, 1) FROM test;", + Expected: []sql.Row{{1}, {5}, {9}}, + }, + { + Query: "SELECT REGEXP_INSTR('abc def ghi', '[a-z]+', v2, v3) FROM test;", + Expected: []sql.Row{{5}, {9}, {0}}, + }, + { + Query: "SELECT REGEXP_SUBSTR('abc def ghi', '[a-z]+', 1, 2) FROM test;", + Expected: []sql.Row{{"def"}, {"def"}, {"def"}}, + }, + { + Query: "SELECT REGEXP_SUBSTR('abc def ghi', v1, 1, 1) FROM test;", + Expected: []sql.Row{{"abc"}, {"def"}, {"ghi"}}, + }, + { + Query: "SELECT REGEXP_SUBSTR('abc def ghi', '[a-z]+', v2, v3) FROM test;", + Expected: []sql.Row{{"def"}, {"ghi"}, {nil}}, + }, + }, + }, } { enginetest.TestScript(t, harness, test) } diff --git a/go.mod b/go.mod index 7d244bd1c2..ea2d40f0a0 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/dolthub/go-mysql-server require ( github.com/cespare/xxhash/v2 v2.2.0 github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2 - github.com/dolthub/go-icu-regex v0.0.0-20250228125923-c1fa04750a0f + github.com/dolthub/go-icu-regex v0.0.0-20250303123116-549b8d7cad00 github.com/dolthub/jsonpath v0.0.2-0.20240227200619-19675ab05c71 github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81 github.com/dolthub/vitess v0.0.0-20250228011932-c4f6bba87730 diff --git a/go.sum b/go.sum index 898abff7f0..e73a8a2d03 100644 --- a/go.sum +++ b/go.sum @@ -52,8 +52,8 @@ github.com/denisenkom/go-mssqldb v0.10.0/go.mod h1:xbL0rPBG9cCiLr28tMa8zpbdarY27 github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ= github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2 h1:u3PMzfF8RkKd3lB9pZ2bfn0qEG+1Gms9599cr0REMww= github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2/go.mod h1:mIEZOHnFx4ZMQeawhw9rhsj+0zwQj7adVsnBX7t+eKY= -github.com/dolthub/go-icu-regex v0.0.0-20250228125923-c1fa04750a0f h1:nCfSUnIviI4c7NY1qcs/XUu7SMy/OWwaEg2H4jp/H5Q= -github.com/dolthub/go-icu-regex v0.0.0-20250228125923-c1fa04750a0f/go.mod h1:ylU4XjUpsMcvl/BKeRRMXSH7e7WBrPXdSLvnRJYrxEA= +github.com/dolthub/go-icu-regex v0.0.0-20250303123116-549b8d7cad00 h1:rh2ij2yTYKJWlX+c8XRg4H5OzqPewbU1lPK8pcfVmx8= +github.com/dolthub/go-icu-regex v0.0.0-20250303123116-549b8d7cad00/go.mod h1:ylU4XjUpsMcvl/BKeRRMXSH7e7WBrPXdSLvnRJYrxEA= github.com/dolthub/jsonpath v0.0.2-0.20240227200619-19675ab05c71 h1:bMGS25NWAGTEtT5tOBsCuCrlYnLRKpbJVJkDbrTRhwQ= github.com/dolthub/jsonpath v0.0.2-0.20240227200619-19675ab05c71/go.mod h1:2/2zjLQ/JOOSbbSboojeg+cAwcRV0fDLzIiWch/lhqI= github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81 h1:7/v8q9XGFa6q5Ap4Z/OhNkAMBaK5YeuEzwJt+NZdhiE= diff --git a/sql/expression/function/regexp_instr.go b/sql/expression/function/regexp_instr.go index 653b8dd5cd..e56593d791 100644 --- a/sql/expression/function/regexp_instr.go +++ b/sql/expression/function/regexp_instr.go @@ -18,7 +18,6 @@ import ( "fmt" "strings" "sync" - "sync/atomic" regex "github.com/dolthub/go-icu-regex" @@ -37,7 +36,9 @@ type RegexpInstr struct { ReturnOption sql.Expression Flags sql.Expression - cachedVal atomic.Value + cachedVal any + cacheRegex bool + cacheVal bool re regex.Regex compileOnce sync.Once compileErr error @@ -45,7 +46,7 @@ type RegexpInstr struct { var _ sql.FunctionExpression = (*RegexpInstr)(nil) var _ sql.CollationCoercible = (*RegexpInstr)(nil) -var _ sql.Closer = (*RegexpInstr)(nil) +var _ sql.Disposable = (*RegexpInstr)(nil) // NewRegexpInstr creates a new RegexpInstr expression. func NewRegexpInstr(args ...sql.Expression) (sql.Expression, error) { @@ -158,10 +159,22 @@ func (r *RegexpInstr) String() string { } // compile handles compilation of the regex. -func (r *RegexpInstr) compile(ctx *sql.Context) { +func (r *RegexpInstr) compile(ctx *sql.Context, row sql.Row) { r.compileOnce.Do(func() { - r.re, r.compileErr = compileRegex(ctx, r.Pattern, r.Text, r.Flags, r.FunctionName(), nil) + r.cacheRegex = canBeCached(r.Text, r.Pattern, r.Flags) + r.cacheVal = canBeCached(r.Text, r.Pattern, r.Position, r.Occurrence, r.ReturnOption, r.Flags) + if r.cacheRegex { + r.re, r.compileErr = compileRegex(ctx, r.Pattern, r.Text, r.Flags, r.FunctionName(), row) + } }) + if !r.cacheRegex { + if r.re != nil { + if r.compileErr = r.re.Close(); r.compileErr != nil { + return + } + } + r.re, r.compileErr = compileRegex(ctx, r.Pattern, r.Text, r.Flags, r.FunctionName(), row) + } } // Eval implements the sql.Expression interface. @@ -169,12 +182,11 @@ func (r *RegexpInstr) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) { span, ctx := ctx.Span("function.RegexpInstr") defer span.End() - cached := r.cachedVal.Load() - if cached != nil { - return cached, nil + if r.cachedVal != nil { + return r.cachedVal, nil } - r.compile(ctx) + r.compile(ctx, row) if r.compileErr != nil { return nil, r.compileErr } @@ -239,17 +251,16 @@ func (r *RegexpInstr) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) { return nil, err } - outVal := int32(index + 1) - if canBeCached(r.Text) { - r.cachedVal.Store(outVal) + outVal := int32(index) + if r.cacheVal { + r.cachedVal = outVal } return outVal, nil } -// Close implements the sql.Closer interface. -func (r *RegexpInstr) Close(ctx *sql.Context) error { +// Dispose implements the sql.Disposable interface. +func (r *RegexpInstr) Dispose() { if r.re != nil { - return r.re.Close() + _ = r.re.Close() } - return nil } diff --git a/sql/expression/function/regexp_like.go b/sql/expression/function/regexp_like.go index 85eb525aa6..3939a15643 100644 --- a/sql/expression/function/regexp_like.go +++ b/sql/expression/function/regexp_like.go @@ -18,7 +18,6 @@ import ( "fmt" "strings" "sync" - "sync/atomic" regex "github.com/dolthub/go-icu-regex" "gopkg.in/src-d/go-errors.v1" @@ -35,7 +34,8 @@ type RegexpLike struct { Pattern sql.Expression Flags sql.Expression - cachedVal atomic.Value + cachedVal any + cacheable bool re regex.Regex compileOnce sync.Once compileErr error @@ -43,7 +43,7 @@ type RegexpLike struct { var _ sql.FunctionExpression = (*RegexpLike)(nil) var _ sql.CollationCoercible = (*RegexpLike)(nil) -var _ sql.Closer = (*RegexpLike)(nil) +var _ sql.Disposable = (*RegexpLike)(nil) // NewRegexpLike creates a new RegexpLike expression. func NewRegexpLike(args ...sql.Expression) (sql.Expression, error) { @@ -115,6 +115,7 @@ func (r *RegexpLike) WithChildren(children ...sql.Expression) (sql.Expression, e return NewRegexpLike(children...) } +// String implements the sql.Expression interface. func (r *RegexpLike) String() string { var args []string for _, e := range r.Children() { @@ -123,10 +124,22 @@ func (r *RegexpLike) String() string { return fmt.Sprintf("%s(%s)", r.FunctionName(), strings.Join(args, ",")) } -func (r *RegexpLike) compile(ctx *sql.Context) { +// compile handles compilation of the regex. +func (r *RegexpLike) compile(ctx *sql.Context, row sql.Row) { r.compileOnce.Do(func() { - r.re, r.compileErr = compileRegex(ctx, r.Pattern, r.Text, r.Flags, r.FunctionName(), nil) + r.cacheable = canBeCached(r.Text, r.Pattern, r.Flags) + if r.cacheable { + r.re, r.compileErr = compileRegex(ctx, r.Pattern, r.Text, r.Flags, r.FunctionName(), row) + } }) + if !r.cacheable { + if r.re != nil { + if r.compileErr = r.re.Close(); r.compileErr != nil { + return + } + } + r.re, r.compileErr = compileRegex(ctx, r.Pattern, r.Text, r.Flags, r.FunctionName(), row) + } } // Eval implements the sql.Expression interface. @@ -134,12 +147,11 @@ func (r *RegexpLike) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) { span, ctx := ctx.Span("function.RegexpLike") defer span.End() - cached := r.cachedVal.Load() - if cached != nil { - return cached, nil + if r.cachedVal != nil { + return r.cachedVal, nil } - r.compile(ctx) + r.compile(ctx, row) if r.compileErr != nil { return nil, r.compileErr } @@ -174,18 +186,17 @@ func (r *RegexpLike) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) { outVal = int8(0) } - if canBeCached(r.Text) { - r.cachedVal.Store(outVal) + if r.cacheable { + r.cachedVal = outVal } return outVal, nil } -// Close implements the sql.Closer interface. -func (r *RegexpLike) Close(ctx *sql.Context) error { +// Dispose implements the sql.Disposable interface. +func (r *RegexpLike) Dispose() { if r.re != nil { - return r.re.Close() + _ = r.re.Close() } - return nil } func compileRegex(ctx *sql.Context, pattern, text, flags sql.Expression, funcName string, row sql.Row) (regex.Regex, error) { @@ -293,14 +304,24 @@ func consolidateRegexpFlags(flags, funcName string) (string, error) { return flags, nil } -func canBeCached(e sql.Expression) bool { +// canBeCached returns whether the expression(s) can be cached +func canBeCached(exprs ...sql.Expression) bool { hasCols := false - sql.Inspect(e, func(e sql.Expression) bool { - switch e.(type) { - case *expression.GetField, *expression.UserVar, *expression.SystemVar, *expression.ProcedureParam: - hasCols = true + for _, expr := range exprs { + if expr == nil { + continue } - return true - }) + sql.Inspect(expr, func(e sql.Expression) bool { + switch e.(type) { + case *expression.GetField, *expression.UserVar, *expression.SystemVar, *expression.ProcedureParam: + hasCols = true + default: + if nonDet, ok := expr.(sql.NonDeterministicExpression); ok { + hasCols = hasCols || nonDet.IsNonDeterministic() + } + } + return true + }) + } return !hasCols } diff --git a/sql/expression/function/regexp_like_test.go b/sql/expression/function/regexp_like_test.go index d80984d8ea..2a23ee641e 100644 --- a/sql/expression/function/regexp_like_test.go +++ b/sql/expression/function/regexp_like_test.go @@ -222,7 +222,7 @@ func TestRegexpLikeWithoutFlags(t *testing.T) { expression.NewLiteral(test.pattern, types.LongText), ) require.NoError(t, err) - defer f.(*RegexpLike).Close(ctx) + defer f.(*RegexpLike).Dispose() res, err := f.Eval(ctx, nil) require.Equal(t, test.expected, res) }) @@ -277,7 +277,7 @@ func TestRegexpLikeWithFlags(t *testing.T) { expression.NewLiteral(test.flags, types.LongText), ) require.NoError(t, err) - defer f.(*RegexpLike).Close(ctx) + defer f.(*RegexpLike).Dispose() res, err := f.Eval(ctx, nil) require.Equal(t, test.expected, res) }) @@ -308,7 +308,7 @@ func TestRegexpLikeNilAndErrors(t *testing.T) { require.NoError(t, err) _, err = f.Eval(ctx, nil) require.True(t, sql.ErrInvalidArgument.Is(err)) - require.NoError(t, f.(*RegexpLike).Close(ctx)) + f.(*RegexpLike).Dispose() f, err = NewRegexpLike( expression.NewLiteral(nil, types.Null), @@ -319,7 +319,7 @@ func TestRegexpLikeNilAndErrors(t *testing.T) { res, err := f.Eval(ctx, nil) require.NoError(t, err) require.Equal(t, nil, res) - require.NoError(t, f.(*RegexpLike).Close(ctx)) + f.(*RegexpLike).Dispose() f, err = NewRegexpLike( expression.NewLiteral("foo", types.LongText), @@ -330,7 +330,7 @@ func TestRegexpLikeNilAndErrors(t *testing.T) { res, err = f.Eval(ctx, nil) require.NoError(t, err) require.Equal(t, nil, res) - require.NoError(t, f.(*RegexpLike).Close(ctx)) + f.(*RegexpLike).Dispose() f, err = NewRegexpLike( expression.NewLiteral("foo", types.LongText), @@ -341,7 +341,7 @@ func TestRegexpLikeNilAndErrors(t *testing.T) { res, err = f.Eval(ctx, nil) require.NoError(t, err) require.Equal(t, nil, res) - require.NoError(t, f.(*RegexpLike).Close(ctx)) + f.(*RegexpLike).Dispose() f, err = NewRegexpLike( expression.NewLiteral(nil, types.Null), @@ -351,7 +351,7 @@ func TestRegexpLikeNilAndErrors(t *testing.T) { res, err = f.Eval(ctx, nil) require.NoError(t, err) require.Equal(t, nil, res) - require.NoError(t, f.(*RegexpLike).Close(ctx)) + f.(*RegexpLike).Dispose() f, err = NewRegexpLike( expression.NewLiteral("foo", types.LongText), @@ -361,5 +361,5 @@ func TestRegexpLikeNilAndErrors(t *testing.T) { res, err = f.Eval(ctx, nil) require.NoError(t, err) require.Equal(t, nil, res) - require.NoError(t, f.(*RegexpLike).Close(ctx)) + f.(*RegexpLike).Dispose() } diff --git a/sql/expression/function/regexp_substr.go b/sql/expression/function/regexp_substr.go index 055a8a3b06..4493cbc301 100644 --- a/sql/expression/function/regexp_substr.go +++ b/sql/expression/function/regexp_substr.go @@ -18,7 +18,6 @@ import ( "fmt" "strings" "sync" - "sync/atomic" regex "github.com/dolthub/go-icu-regex" @@ -36,7 +35,9 @@ type RegexpSubstr struct { Occurrence sql.Expression Flags sql.Expression - cachedVal atomic.Value + cachedVal any + cacheRegex bool + cacheVal bool re regex.Regex compileOnce sync.Once compileErr error @@ -44,7 +45,7 @@ type RegexpSubstr struct { var _ sql.FunctionExpression = (*RegexpSubstr)(nil) var _ sql.CollationCoercible = (*RegexpSubstr)(nil) -var _ sql.Closer = (*RegexpSubstr)(nil) +var _ sql.Disposable = (*RegexpSubstr)(nil) // NewRegexpSubstr creates a new RegexpSubstr expression. func NewRegexpSubstr(args ...sql.Expression) (sql.Expression, error) { @@ -145,10 +146,22 @@ func (r *RegexpSubstr) String() string { } // compile handles compilation of the regex. -func (r *RegexpSubstr) compile(ctx *sql.Context) { +func (r *RegexpSubstr) compile(ctx *sql.Context, row sql.Row) { r.compileOnce.Do(func() { - r.re, r.compileErr = compileRegex(ctx, r.Pattern, r.Text, r.Flags, r.FunctionName(), nil) + r.cacheRegex = canBeCached(r.Text, r.Pattern, r.Flags) + r.cacheVal = canBeCached(r.Text, r.Pattern, r.Position, r.Occurrence, r.Flags) + if r.cacheRegex { + r.re, r.compileErr = compileRegex(ctx, r.Pattern, r.Text, r.Flags, r.FunctionName(), row) + } }) + if !r.cacheRegex { + if r.re != nil { + if r.compileErr = r.re.Close(); r.compileErr != nil { + return + } + } + r.re, r.compileErr = compileRegex(ctx, r.Pattern, r.Text, r.Flags, r.FunctionName(), row) + } } // Eval implements the sql.Expression interface. @@ -156,12 +169,11 @@ func (r *RegexpSubstr) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) span, ctx := ctx.Span("function.RegexpSubstr") defer span.End() - cached := r.cachedVal.Load() - if cached != nil { - return cached, nil + if r.cachedVal != nil { + return r.cachedVal, nil } - r.compile(ctx) + r.compile(ctx, row) if r.compileErr != nil { return nil, r.compileErr } @@ -217,16 +229,15 @@ func (r *RegexpSubstr) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) return nil, nil } - if canBeCached(r.Text) { - r.cachedVal.Store(substring) + if r.cacheVal { + r.cachedVal = substring } return substring, nil } -// Close implements the sql.Closer interface. -func (r *RegexpSubstr) Close(ctx *sql.Context) error { +// Dispose implements the sql.Disposable interface. +func (r *RegexpSubstr) Dispose() { if r.re != nil { - return r.re.Close() + _ = r.re.Close() } - return nil }