Skip to content

Commit ff2d313

Browse files
HydrochargedHydrocharged
committed
PR feedback
1 parent 13f6f04 commit ff2d313

File tree

7 files changed

+148
-64
lines changed

7 files changed

+148
-64
lines changed

‎enginetest/engine_only_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -781,6 +781,47 @@ func TestRegex(t *testing.T) {
781781
},
782782
},
783783
},
784+
{
785+
Name: "REGEXP caching behavior",
786+
SetUpScript: []string{
787+
"CREATE TABLE test (v1 TEXT, v2 INT, v3 INT);",
788+
"INSERT INTO test VALUES ('abc', 1, 2), ('[d-i]+', 2, 3), ('ghi', 3, 4);",
789+
},
790+
Assertions: []queries.ScriptTestAssertion{
791+
{
792+
Query: "SELECT REGEXP_LIKE('abc def ghi', 'abc') FROM test;",
793+
Expected: []sql.Row{{1}, {1}, {1}},
794+
},
795+
{
796+
Query: "SELECT REGEXP_LIKE('abc def ghi', v1) FROM test;",
797+
Expected: []sql.Row{{1}, {1}, {1}},
798+
},
799+
{
800+
Query: "SELECT REGEXP_INSTR('abc def ghi', '[a-z]+', 1, 2) FROM test;",
801+
Expected: []sql.Row{{5}, {5}, {5}},
802+
},
803+
{
804+
Query: "SELECT REGEXP_INSTR('abc def ghi', v1, 1, 1) FROM test;",
805+
Expected: []sql.Row{{1}, {5}, {9}},
806+
},
807+
{
808+
Query: "SELECT REGEXP_INSTR('abc def ghi', '[a-z]+', v2, v3) FROM test;",
809+
Expected: []sql.Row{{5}, {9}, {0}},
810+
},
811+
{
812+
Query: "SELECT REGEXP_SUBSTR('abc def ghi', '[a-z]+', 1, 2) FROM test;",
813+
Expected: []sql.Row{{"def"}, {"def"}, {"def"}},
814+
},
815+
{
816+
Query: "SELECT REGEXP_SUBSTR('abc def ghi', v1, 1, 1) FROM test;",
817+
Expected: []sql.Row{{"abc"}, {"def"}, {"ghi"}},
818+
},
819+
{
820+
Query: "SELECT REGEXP_SUBSTR('abc def ghi', '[a-z]+', v2, v3) FROM test;",
821+
Expected: []sql.Row{{"def"}, {"ghi"}, {nil}},
822+
},
823+
},
824+
},
784825
} {
785826
enginetest.TestScript(t, harness, test)
786827
}

‎go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ module github.com/dolthub/go-mysql-server
33
require (
44
github.com/cespare/xxhash/v2 v2.2.0
55
github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2
6-
github.com/dolthub/go-icu-regex v0.0.0-20250228125923-c1fa04750a0f
6+
github.com/dolthub/go-icu-regex v0.0.0-20250303123116-549b8d7cad00
77
github.com/dolthub/jsonpath v0.0.2-0.20240227200619-19675ab05c71
88
github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81
99
github.com/dolthub/vitess v0.0.0-20250228011932-c4f6bba87730

‎go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,8 @@ github.com/denisenkom/go-mssqldb v0.10.0/go.mod h1:xbL0rPBG9cCiLr28tMa8zpbdarY27
5252
github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ=
5353
github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2 h1:u3PMzfF8RkKd3lB9pZ2bfn0qEG+1Gms9599cr0REMww=
5454
github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2/go.mod h1:mIEZOHnFx4ZMQeawhw9rhsj+0zwQj7adVsnBX7t+eKY=
55-
github.com/dolthub/go-icu-regex v0.0.0-20250228125923-c1fa04750a0f h1:nCfSUnIviI4c7NY1qcs/XUu7SMy/OWwaEg2H4jp/H5Q=
56-
github.com/dolthub/go-icu-regex v0.0.0-20250228125923-c1fa04750a0f/go.mod h1:ylU4XjUpsMcvl/BKeRRMXSH7e7WBrPXdSLvnRJYrxEA=
55+
github.com/dolthub/go-icu-regex v0.0.0-20250303123116-549b8d7cad00 h1:rh2ij2yTYKJWlX+c8XRg4H5OzqPewbU1lPK8pcfVmx8=
56+
github.com/dolthub/go-icu-regex v0.0.0-20250303123116-549b8d7cad00/go.mod h1:ylU4XjUpsMcvl/BKeRRMXSH7e7WBrPXdSLvnRJYrxEA=
5757
github.com/dolthub/jsonpath v0.0.2-0.20240227200619-19675ab05c71 h1:bMGS25NWAGTEtT5tOBsCuCrlYnLRKpbJVJkDbrTRhwQ=
5858
github.com/dolthub/jsonpath v0.0.2-0.20240227200619-19675ab05c71/go.mod h1:2/2zjLQ/JOOSbbSboojeg+cAwcRV0fDLzIiWch/lhqI=
5959
github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81 h1:7/v8q9XGFa6q5Ap4Z/OhNkAMBaK5YeuEzwJt+NZdhiE=

‎sql/expression/function/regexp_instr.go

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import (
1818
"fmt"
1919
"strings"
2020
"sync"
21-
"sync/atomic"
2221

2322
regex "github.com/dolthub/go-icu-regex"
2423

@@ -37,15 +36,17 @@ type RegexpInstr struct {
3736
ReturnOption sql.Expression
3837
Flags sql.Expression
3938

40-
cachedVal atomic.Value
39+
cachedVal any
40+
cacheRegex bool
41+
cacheVal bool
4142
re regex.Regex
4243
compileOnce sync.Once
4344
compileErr error
4445
}
4546

4647
var _ sql.FunctionExpression = (*RegexpInstr)(nil)
4748
var _ sql.CollationCoercible = (*RegexpInstr)(nil)
48-
var _ sql.Closer = (*RegexpInstr)(nil)
49+
var _ sql.Disposable = (*RegexpInstr)(nil)
4950

5051
// NewRegexpInstr creates a new RegexpInstr expression.
5152
func NewRegexpInstr(args ...sql.Expression) (sql.Expression, error) {
@@ -158,23 +159,34 @@ func (r *RegexpInstr) String() string {
158159
}
159160

160161
// compile handles compilation of the regex.
161-
func (r *RegexpInstr) compile(ctx *sql.Context) {
162+
func (r *RegexpInstr) compile(ctx *sql.Context, row sql.Row) {
162163
r.compileOnce.Do(func() {
163-
r.re, r.compileErr = compileRegex(ctx, r.Pattern, r.Text, r.Flags, r.FunctionName(), nil)
164+
r.cacheRegex = canBeCached(r.Text, r.Pattern, r.Flags)
165+
r.cacheVal = canBeCached(r.Text, r.Pattern, r.Position, r.Occurrence, r.ReturnOption, r.Flags)
166+
if r.cacheRegex {
167+
r.re, r.compileErr = compileRegex(ctx, r.Pattern, r.Text, r.Flags, r.FunctionName(), row)
168+
}
164169
})
170+
if !r.cacheRegex {
171+
if r.re != nil {
172+
if r.compileErr = r.re.Close(); r.compileErr != nil {
173+
return
174+
}
175+
}
176+
r.re, r.compileErr = compileRegex(ctx, r.Pattern, r.Text, r.Flags, r.FunctionName(), row)
177+
}
165178
}
166179

167180
// Eval implements the sql.Expression interface.
168181
func (r *RegexpInstr) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) {
169182
span, ctx := ctx.Span("function.RegexpInstr")
170183
defer span.End()
171184

172-
cached := r.cachedVal.Load()
173-
if cached != nil {
174-
return cached, nil
185+
if r.cachedVal != nil {
186+
return r.cachedVal, nil
175187
}
176188

177-
r.compile(ctx)
189+
r.compile(ctx, row)
178190
if r.compileErr != nil {
179191
return nil, r.compileErr
180192
}
@@ -239,17 +251,16 @@ func (r *RegexpInstr) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) {
239251
return nil, err
240252
}
241253

242-
outVal := int32(index + 1)
243-
if canBeCached(r.Text) {
244-
r.cachedVal.Store(outVal)
254+
outVal := int32(index)
255+
if r.cacheVal {
256+
r.cachedVal = outVal
245257
}
246258
return outVal, nil
247259
}
248260

249-
// Close implements the sql.Closer interface.
250-
func (r *RegexpInstr) Close(ctx *sql.Context) error {
261+
// Dispose implements the sql.Disposable interface.
262+
func (r *RegexpInstr) Dispose() {
251263
if r.re != nil {
252-
return r.re.Close()
264+
_ = r.re.Close()
253265
}
254-
return nil
255266
}

‎sql/expression/function/regexp_like.go

Lines changed: 43 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import (
1818
"fmt"
1919
"strings"
2020
"sync"
21-
"sync/atomic"
2221

2322
regex "github.com/dolthub/go-icu-regex"
2423
"gopkg.in/src-d/go-errors.v1"
@@ -35,15 +34,16 @@ type RegexpLike struct {
3534
Pattern sql.Expression
3635
Flags sql.Expression
3736

38-
cachedVal atomic.Value
37+
cachedVal any
38+
cacheable bool
3939
re regex.Regex
4040
compileOnce sync.Once
4141
compileErr error
4242
}
4343

4444
var _ sql.FunctionExpression = (*RegexpLike)(nil)
4545
var _ sql.CollationCoercible = (*RegexpLike)(nil)
46-
var _ sql.Closer = (*RegexpLike)(nil)
46+
var _ sql.Disposable = (*RegexpLike)(nil)
4747

4848
// NewRegexpLike creates a new RegexpLike expression.
4949
func NewRegexpLike(args ...sql.Expression) (sql.Expression, error) {
@@ -115,6 +115,7 @@ func (r *RegexpLike) WithChildren(children ...sql.Expression) (sql.Expression, e
115115
return NewRegexpLike(children...)
116116
}
117117

118+
// String implements the sql.Expression interface.
118119
func (r *RegexpLike) String() string {
119120
var args []string
120121
for _, e := range r.Children() {
@@ -123,23 +124,34 @@ func (r *RegexpLike) String() string {
123124
return fmt.Sprintf("%s(%s)", r.FunctionName(), strings.Join(args, ","))
124125
}
125126

126-
func (r *RegexpLike) compile(ctx *sql.Context) {
127+
// compile handles compilation of the regex.
128+
func (r *RegexpLike) compile(ctx *sql.Context, row sql.Row) {
127129
r.compileOnce.Do(func() {
128-
r.re, r.compileErr = compileRegex(ctx, r.Pattern, r.Text, r.Flags, r.FunctionName(), nil)
130+
r.cacheable = canBeCached(r.Text, r.Pattern, r.Flags)
131+
if r.cacheable {
132+
r.re, r.compileErr = compileRegex(ctx, r.Pattern, r.Text, r.Flags, r.FunctionName(), row)
133+
}
129134
})
135+
if !r.cacheable {
136+
if r.re != nil {
137+
if r.compileErr = r.re.Close(); r.compileErr != nil {
138+
return
139+
}
140+
}
141+
r.re, r.compileErr = compileRegex(ctx, r.Pattern, r.Text, r.Flags, r.FunctionName(), row)
142+
}
130143
}
131144

132145
// Eval implements the sql.Expression interface.
133146
func (r *RegexpLike) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) {
134147
span, ctx := ctx.Span("function.RegexpLike")
135148
defer span.End()
136149

137-
cached := r.cachedVal.Load()
138-
if cached != nil {
139-
return cached, nil
150+
if r.cachedVal != nil {
151+
return r.cachedVal, nil
140152
}
141153

142-
r.compile(ctx)
154+
r.compile(ctx, row)
143155
if r.compileErr != nil {
144156
return nil, r.compileErr
145157
}
@@ -174,18 +186,17 @@ func (r *RegexpLike) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) {
174186
outVal = int8(0)
175187
}
176188

177-
if canBeCached(r.Text) {
178-
r.cachedVal.Store(outVal)
189+
if r.cacheable {
190+
r.cachedVal = outVal
179191
}
180192
return outVal, nil
181193
}
182194

183-
// Close implements the sql.Closer interface.
184-
func (r *RegexpLike) Close(ctx *sql.Context) error {
195+
// Dispose implements the sql.Disposable interface.
196+
func (r *RegexpLike) Dispose() {
185197
if r.re != nil {
186-
return r.re.Close()
198+
_ = r.re.Close()
187199
}
188-
return nil
189200
}
190201

191202
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) {
293304
return flags, nil
294305
}
295306

296-
func canBeCached(e sql.Expression) bool {
307+
// canBeCached returns whether the expression(s) can be cached
308+
func canBeCached(exprs ...sql.Expression) bool {
297309
hasCols := false
298-
sql.Inspect(e, func(e sql.Expression) bool {
299-
switch e.(type) {
300-
case *expression.GetField, *expression.UserVar, *expression.SystemVar, *expression.ProcedureParam:
301-
hasCols = true
310+
for _, expr := range exprs {
311+
if expr == nil {
312+
continue
302313
}
303-
return true
304-
})
314+
sql.Inspect(expr, func(e sql.Expression) bool {
315+
switch e.(type) {
316+
case *expression.GetField, *expression.UserVar, *expression.SystemVar, *expression.ProcedureParam:
317+
hasCols = true
318+
default:
319+
if nonDet, ok := expr.(sql.NonDeterministicExpression); ok {
320+
hasCols = hasCols || nonDet.IsNonDeterministic()
321+
}
322+
}
323+
return true
324+
})
325+
}
305326
return !hasCols
306327
}

‎sql/expression/function/regexp_like_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ func TestRegexpLikeWithoutFlags(t *testing.T) {
222222
expression.NewLiteral(test.pattern, types.LongText),
223223
)
224224
require.NoError(t, err)
225-
defer f.(*RegexpLike).Close(ctx)
225+
defer f.(*RegexpLike).Dispose()
226226
res, err := f.Eval(ctx, nil)
227227
require.Equal(t, test.expected, res)
228228
})
@@ -277,7 +277,7 @@ func TestRegexpLikeWithFlags(t *testing.T) {
277277
expression.NewLiteral(test.flags, types.LongText),
278278
)
279279
require.NoError(t, err)
280-
defer f.(*RegexpLike).Close(ctx)
280+
defer f.(*RegexpLike).Dispose()
281281
res, err := f.Eval(ctx, nil)
282282
require.Equal(t, test.expected, res)
283283
})
@@ -308,7 +308,7 @@ func TestRegexpLikeNilAndErrors(t *testing.T) {
308308
require.NoError(t, err)
309309
_, err = f.Eval(ctx, nil)
310310
require.True(t, sql.ErrInvalidArgument.Is(err))
311-
require.NoError(t, f.(*RegexpLike).Close(ctx))
311+
f.(*RegexpLike).Dispose()
312312

313313
f, err = NewRegexpLike(
314314
expression.NewLiteral(nil, types.Null),
@@ -319,7 +319,7 @@ func TestRegexpLikeNilAndErrors(t *testing.T) {
319319
res, err := f.Eval(ctx, nil)
320320
require.NoError(t, err)
321321
require.Equal(t, nil, res)
322-
require.NoError(t, f.(*RegexpLike).Close(ctx))
322+
f.(*RegexpLike).Dispose()
323323

324324
f, err = NewRegexpLike(
325325
expression.NewLiteral("foo", types.LongText),
@@ -330,7 +330,7 @@ func TestRegexpLikeNilAndErrors(t *testing.T) {
330330
res, err = f.Eval(ctx, nil)
331331
require.NoError(t, err)
332332
require.Equal(t, nil, res)
333-
require.NoError(t, f.(*RegexpLike).Close(ctx))
333+
f.(*RegexpLike).Dispose()
334334

335335
f, err = NewRegexpLike(
336336
expression.NewLiteral("foo", types.LongText),
@@ -341,7 +341,7 @@ func TestRegexpLikeNilAndErrors(t *testing.T) {
341341
res, err = f.Eval(ctx, nil)
342342
require.NoError(t, err)
343343
require.Equal(t, nil, res)
344-
require.NoError(t, f.(*RegexpLike).Close(ctx))
344+
f.(*RegexpLike).Dispose()
345345

346346
f, err = NewRegexpLike(
347347
expression.NewLiteral(nil, types.Null),
@@ -351,7 +351,7 @@ func TestRegexpLikeNilAndErrors(t *testing.T) {
351351
res, err = f.Eval(ctx, nil)
352352
require.NoError(t, err)
353353
require.Equal(t, nil, res)
354-
require.NoError(t, f.(*RegexpLike).Close(ctx))
354+
f.(*RegexpLike).Dispose()
355355

356356
f, err = NewRegexpLike(
357357
expression.NewLiteral("foo", types.LongText),
@@ -361,5 +361,5 @@ func TestRegexpLikeNilAndErrors(t *testing.T) {
361361
res, err = f.Eval(ctx, nil)
362362
require.NoError(t, err)
363363
require.Equal(t, nil, res)
364-
require.NoError(t, f.(*RegexpLike).Close(ctx))
364+
f.(*RegexpLike).Dispose()
365365
}

0 commit comments

Comments
 (0)