Skip to content

Commit 7ff1682

Browse files
HydrochargedHydrocharged
committed
PR feedback
1 parent 13f6f04 commit 7ff1682

File tree

6 files changed

+136
-56
lines changed

6 files changed

+136
-56
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: 25 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,16 @@ type RegexpInstr struct {
3736
ReturnOption sql.Expression
3837
Flags sql.Expression
3938

40-
cachedVal atomic.Value
39+
cachedVal any
40+
cacheable bool
4141
re regex.Regex
4242
compileOnce sync.Once
4343
compileErr error
4444
}
4545

4646
var _ sql.FunctionExpression = (*RegexpInstr)(nil)
4747
var _ sql.CollationCoercible = (*RegexpInstr)(nil)
48-
var _ sql.Closer = (*RegexpInstr)(nil)
48+
var _ sql.Disposable = (*RegexpInstr)(nil)
4949

5050
// NewRegexpInstr creates a new RegexpInstr expression.
5151
func NewRegexpInstr(args ...sql.Expression) (sql.Expression, error) {
@@ -158,23 +158,33 @@ func (r *RegexpInstr) String() string {
158158
}
159159

160160
// compile handles compilation of the regex.
161-
func (r *RegexpInstr) compile(ctx *sql.Context) {
161+
func (r *RegexpInstr) compile(ctx *sql.Context, row sql.Row) {
162162
r.compileOnce.Do(func() {
163-
r.re, r.compileErr = compileRegex(ctx, r.Pattern, r.Text, r.Flags, r.FunctionName(), nil)
163+
r.cacheable = canBeCached(r.Text, r.Pattern, r.Position, r.Occurrence, r.ReturnOption, r.Flags)
164+
if r.cacheable {
165+
r.re, r.compileErr = compileRegex(ctx, r.Pattern, r.Text, r.Flags, r.FunctionName(), row)
166+
}
164167
})
168+
if !r.cacheable {
169+
if r.re != nil {
170+
if r.compileErr = r.re.Close(); r.compileErr != nil {
171+
return
172+
}
173+
}
174+
r.re, r.compileErr = compileRegex(ctx, r.Pattern, r.Text, r.Flags, r.FunctionName(), row)
175+
}
165176
}
166177

167178
// Eval implements the sql.Expression interface.
168179
func (r *RegexpInstr) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) {
169180
span, ctx := ctx.Span("function.RegexpInstr")
170181
defer span.End()
171182

172-
cached := r.cachedVal.Load()
173-
if cached != nil {
174-
return cached, nil
183+
if r.cachedVal != nil {
184+
return r.cachedVal, nil
175185
}
176186

177-
r.compile(ctx)
187+
r.compile(ctx, row)
178188
if r.compileErr != nil {
179189
return nil, r.compileErr
180190
}
@@ -239,17 +249,16 @@ func (r *RegexpInstr) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) {
239249
return nil, err
240250
}
241251

242-
outVal := int32(index + 1)
243-
if canBeCached(r.Text) {
244-
r.cachedVal.Store(outVal)
252+
outVal := int32(index)
253+
if r.cacheable {
254+
r.cachedVal = outVal
245255
}
246256
return outVal, nil
247257
}
248258

249-
// Close implements the sql.Closer interface.
250-
func (r *RegexpInstr) Close(ctx *sql.Context) error {
259+
// Dispose implements the sql.Disposable interface.
260+
func (r *RegexpInstr) Dispose() {
251261
if r.re != nil {
252-
return r.re.Close()
262+
_ = r.re.Close()
253263
}
254-
return nil
255264
}

‎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_substr.go‎

Lines changed: 24 additions & 15 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

@@ -36,15 +35,16 @@ type RegexpSubstr struct {
3635
Occurrence sql.Expression
3736
Flags sql.Expression
3837

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

4545
var _ sql.FunctionExpression = (*RegexpSubstr)(nil)
4646
var _ sql.CollationCoercible = (*RegexpSubstr)(nil)
47-
var _ sql.Closer = (*RegexpSubstr)(nil)
47+
var _ sql.Disposable = (*RegexpSubstr)(nil)
4848

4949
// NewRegexpSubstr creates a new RegexpSubstr expression.
5050
func NewRegexpSubstr(args ...sql.Expression) (sql.Expression, error) {
@@ -145,23 +145,33 @@ func (r *RegexpSubstr) String() string {
145145
}
146146

147147
// compile handles compilation of the regex.
148-
func (r *RegexpSubstr) compile(ctx *sql.Context) {
148+
func (r *RegexpSubstr) compile(ctx *sql.Context, row sql.Row) {
149149
r.compileOnce.Do(func() {
150-
r.re, r.compileErr = compileRegex(ctx, r.Pattern, r.Text, r.Flags, r.FunctionName(), nil)
150+
r.cacheable = canBeCached(r.Text, r.Pattern, r.Position, r.Occurrence, r.Flags)
151+
if r.cacheable {
152+
r.re, r.compileErr = compileRegex(ctx, r.Pattern, r.Text, r.Flags, r.FunctionName(), row)
153+
}
151154
})
155+
if !r.cacheable {
156+
if r.re != nil {
157+
if r.compileErr = r.re.Close(); r.compileErr != nil {
158+
return
159+
}
160+
}
161+
r.re, r.compileErr = compileRegex(ctx, r.Pattern, r.Text, r.Flags, r.FunctionName(), row)
162+
}
152163
}
153164

154165
// Eval implements the sql.Expression interface.
155166
func (r *RegexpSubstr) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) {
156167
span, ctx := ctx.Span("function.RegexpSubstr")
157168
defer span.End()
158169

159-
cached := r.cachedVal.Load()
160-
if cached != nil {
161-
return cached, nil
170+
if r.cachedVal != nil {
171+
return r.cachedVal, nil
162172
}
163173

164-
r.compile(ctx)
174+
r.compile(ctx, row)
165175
if r.compileErr != nil {
166176
return nil, r.compileErr
167177
}
@@ -217,16 +227,15 @@ func (r *RegexpSubstr) Eval(ctx *sql.Context, row sql.Row) (interface{}, error)
217227
return nil, nil
218228
}
219229

220-
if canBeCached(r.Text) {
221-
r.cachedVal.Store(substring)
230+
if r.cacheable {
231+
r.cachedVal = substring
222232
}
223233
return substring, nil
224234
}
225235

226-
// Close implements the sql.Closer interface.
227-
func (r *RegexpSubstr) Close(ctx *sql.Context) error {
236+
// Dispose implements the sql.Disposable interface.
237+
func (r *RegexpSubstr) Dispose() {
228238
if r.re != nil {
229-
return r.re.Close()
239+
_ = r.re.Close()
230240
}
231-
return nil
232241
}

0 commit comments

Comments
 (0)