Skip to content

Commit e787e8e

Browse files
elianddbelianddb
authored
Merge pull request #3197 from dolthub/elian/9794
dolthub/dolt#9794 - Fix string function panics with Dolt TextStorage
2 parents 85527db + c0a961f commit e787e8e

File tree

10 files changed

+304
-2
lines changed

10 files changed

+304
-2
lines changed

‎enginetest/queries/script_queries.go‎

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,175 @@ type ScriptTestAssertion struct {
120120
// Unlike other engine tests, ScriptTests must be self-contained. No other tables are created outside the definition of
121121
// the tests.
122122
var ScriptTests = []ScriptTest{
123+
{
124+
// https://github.com/dolthub/dolt/issues/9794
125+
Name: "UPDATE with TRIM function on TEXT column",
126+
SetUpScript: []string{
127+
"create table my_table (txt text);",
128+
"insert into my_table values('foobar');",
129+
},
130+
Assertions: []ScriptTestAssertion{
131+
{
132+
Query: "update my_table set txt = trim(txt);",
133+
SkipResultsCheck: true,
134+
},
135+
{
136+
Query: "select txt from my_table;",
137+
Expected: []sql.Row{{"foobar"}},
138+
},
139+
},
140+
},
141+
{
142+
// https://github.com/dolthub/dolt/issues/9794
143+
Name: "String functions with TextStorage (comprehensive test)",
144+
Dialect: "mysql",
145+
SetUpScript: []string{
146+
"create table test_strings (id int primary key, content text);",
147+
"insert into test_strings values (1, ' Hello World '), (2, 'Test String'), (3, 'LOWERCASE'), (4, 'abc123def');",
148+
},
149+
Assertions: []ScriptTestAssertion{
150+
{
151+
Query: "select id, trim(content) from test_strings order by id;",
152+
Expected: []sql.Row{{1, "Hello World"}, {2, "Test String"}, {3, "LOWERCASE"}, {4, "abc123def"}},
153+
},
154+
{
155+
Query: "select id, upper(content) from test_strings order by id;",
156+
Expected: []sql.Row{{1, " HELLO WORLD "}, {2, "TEST STRING"}, {3, "LOWERCASE"}, {4, "ABC123DEF"}},
157+
},
158+
{
159+
Query: "select id, lower(content) from test_strings order by id;",
160+
Expected: []sql.Row{{1, " hello world "}, {2, "test string"}, {3, "lowercase"}, {4, "abc123def"}},
161+
},
162+
{
163+
Query: "select id, reverse(content) from test_strings order by id;",
164+
Expected: []sql.Row{{1, " dlroW olleH "}, {2, "gnirtS tseT"}, {3, "ESACREWOL"}, {4, "fed321cba"}},
165+
},
166+
{
167+
Query: "select id, substring(content, 1, 5) from test_strings order by id;",
168+
Expected: []sql.Row{{1, " Hel"}, {2, "Test "}, {3, "LOWER"}, {4, "abc12"}},
169+
},
170+
{
171+
Query: "select id, length(content) from test_strings order by id;",
172+
Expected: []sql.Row{{1, 15}, {2, 11}, {3, 9}, {4, 9}},
173+
},
174+
{
175+
Query: "select id, left(content, 3) from test_strings order by id;",
176+
Expected: []sql.Row{{1, " H"}, {2, "Tes"}, {3, "LOW"}, {4, "abc"}},
177+
},
178+
{
179+
Query: "select id, right(content, 3) from test_strings order by id;",
180+
Expected: []sql.Row{{1, "d "}, {2, "ing"}, {3, "ASE"}, {4, "def"}},
181+
},
182+
{
183+
Query: "select id, ltrim(content) from test_strings order by id;",
184+
Expected: []sql.Row{{1, "Hello World "}, {2, "Test String"}, {3, "LOWERCASE"}, {4, "abc123def"}},
185+
},
186+
{
187+
Query: "select id, rtrim(content) from test_strings order by id;",
188+
Expected: []sql.Row{{1, " Hello World"}, {2, "Test String"}, {3, "LOWERCASE"}, {4, "abc123def"}},
189+
},
190+
{
191+
Query: "select id, replace(content, 'e', 'X') from test_strings order by id;",
192+
Expected: []sql.Row{{1, " HXllo World "}, {2, "TXst String"}, {3, "LOWERCASE"}, {4, "abc123dXf"}},
193+
},
194+
{
195+
Query: "select id, repeat(substring(content, 1, 2), 2) from test_strings order by id;",
196+
Expected: []sql.Row{{1, " "}, {2, "TeTe"}, {3, "LOLO"}, {4, "abab"}},
197+
},
198+
{
199+
Query: "select id, lpad(content, 12, '*') from test_strings where id = 4;",
200+
Expected: []sql.Row{{4, "***abc123def"}},
201+
},
202+
{
203+
Query: "select id, rpad(content, 12, '*') from test_strings where id = 4;",
204+
Expected: []sql.Row{{4, "abc123def***"}},
205+
},
206+
{
207+
Query: "select id, locate('o', content) from test_strings order by id;",
208+
Expected: []sql.Row{{1, 7}, {2, 0}, {3, 2}, {4, 0}},
209+
},
210+
{
211+
Query: "select id, position('o' in content) from test_strings order by id;",
212+
Expected: []sql.Row{{1, 7}, {2, 0}, {3, 2}, {4, 0}},
213+
},
214+
{
215+
Query: "select id, substr(content, 2, 4) from test_strings order by id;",
216+
Expected: []sql.Row{{1, " Hel"}, {2, "est "}, {3, "OWER"}, {4, "bc12"}},
217+
},
218+
{
219+
Query: "select id, mid(content, 3, 3) from test_strings order by id;",
220+
Expected: []sql.Row{{1, "Hel"}, {2, "st "}, {3, "WER"}, {4, "c12"}},
221+
},
222+
{
223+
Query: "select id, char_length(content) from test_strings order by id;",
224+
Expected: []sql.Row{{1, 15}, {2, 11}, {3, 9}, {4, 9}},
225+
},
226+
{
227+
Query: "select id, character_length(content) from test_strings order by id;",
228+
Expected: []sql.Row{{1, 15}, {2, 11}, {3, 9}, {4, 9}},
229+
},
230+
{
231+
Query: "select id, octet_length(content) from test_strings order by id;",
232+
Expected: []sql.Row{{1, 15}, {2, 11}, {3, 9}, {4, 9}},
233+
},
234+
{
235+
Query: "select id, lcase(content) from test_strings order by id;",
236+
Expected: []sql.Row{{1, " hello world "}, {2, "test string"}, {3, "lowercase"}, {4, "abc123def"}},
237+
},
238+
{
239+
Query: "select id, ucase(content) from test_strings order by id;",
240+
Expected: []sql.Row{{1, " HELLO WORLD "}, {2, "TEST STRING"}, {3, "LOWERCASE"}, {4, "ABC123DEF"}},
241+
},
242+
{
243+
Query: "select id, ascii(content) from test_strings order by id;",
244+
Expected: []sql.Row{{1, uint64(32)}, {2, uint64(84)}, {3, uint64(76)}, {4, uint64(97)}},
245+
},
246+
{
247+
Query: "select id, hex(content) from test_strings where id = 4;",
248+
Expected: []sql.Row{{4, "616263313233646566"}},
249+
},
250+
{
251+
Query: "select id, unhex(hex(content)) = content from test_strings where id = 4;",
252+
Expected: []sql.Row{{4, true}},
253+
},
254+
{
255+
Query: "select id, substring_index(content, 'e', 1) from test_strings order by id;",
256+
Expected: []sql.Row{{1, " H"}, {2, "T"}, {3, "LOWERCASE"}, {4, "abc123d"}},
257+
},
258+
{
259+
Query: "select id, insert(content, 2, 3, 'XYZ') from test_strings where id = 4;",
260+
Expected: []sql.Row{{4, "aXYZ23def"}},
261+
},
262+
{
263+
Query: "update test_strings set content = concat(trim(content), '!');",
264+
SkipResultsCheck: true,
265+
},
266+
{
267+
Query: "select id, content from test_strings order by id;",
268+
Expected: []sql.Row{{1, "Hello World!"}, {2, "Test String!"}, {3, "LOWERCASE!"}, {4, "abc123def!"}},
269+
},
270+
{
271+
Query: "SELECT CONCAT_WS(',', content, 'suffix') FROM test_strings WHERE id = 2;",
272+
Expected: []sql.Row{{"Test String!,suffix"}},
273+
},
274+
{
275+
Query: "SELECT EXPORT_SET(5, content, 'off') FROM test_strings WHERE id = 2;",
276+
Expected: []sql.Row{{"Test String!,off,Test String!,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off,off"}},
277+
},
278+
{
279+
Query: "SELECT FIND_IN_SET('String', content) FROM test_strings WHERE id = 2;",
280+
Expected: []sql.Row{{int32(0)}},
281+
},
282+
{
283+
Query: "SELECT MAKE_SET(3, content, 'second', 'third') FROM test_strings WHERE id = 2;",
284+
Expected: []sql.Row{{"Test String!,second"}},
285+
},
286+
{
287+
Query: "SELECT SOUNDEX(content) FROM test_strings WHERE id = 2;",
288+
Expected: []sql.Row{{"T2323652"}},
289+
},
290+
},
291+
},
123292
{
124293
// Regression test for https://github.com/dolthub/dolt/issues/9641
125294
Name: "bit union max1err dolt#9641",

‎sql/expression/function/concat_ws.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,12 @@ func (f *ConcatWithSeparator) Eval(ctx *sql.Context, row sql.Row) (interface{},
127127
return nil, err
128128
}
129129

130+
// Handle Dolt's TextStorage wrapper that doesn't convert to plain string
131+
val, err = sql.UnwrapAny(ctx, val)
132+
if err != nil {
133+
return nil, err
134+
}
135+
130136
parts = append(parts, val.(string))
131137
}
132138

‎sql/expression/function/export_set.go‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,13 @@ func (e *ExportSet) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) {
171171
if err != nil {
172172
return nil, err
173173
}
174+
175+
// Handle Dolt's TextStorage wrapper that doesn't convert to plain string
176+
sepStr, err = sql.UnwrapAny(ctx, sepStr)
177+
if err != nil {
178+
return nil, err
179+
}
180+
174181
separatorVal = sepStr.(string)
175182
}
176183

@@ -206,11 +213,23 @@ func (e *ExportSet) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) {
206213
return nil, err
207214
}
208215

216+
// Handle Dolt's TextStorage wrapper that doesn't convert to plain string
217+
onStr, err = sql.UnwrapAny(ctx, onStr)
218+
if err != nil {
219+
return nil, err
220+
}
221+
209222
offStr, _, err := types.LongText.Convert(ctx, offVal)
210223
if err != nil {
211224
return nil, err
212225
}
213226

227+
// Handle Dolt's TextStorage wrapper that doesn't convert to plain string
228+
offStr, err = sql.UnwrapAny(ctx, offStr)
229+
if err != nil {
230+
return nil, err
231+
}
232+
214233
bits := bitsInt.(uint64)
215234
on := onStr.(string)
216235
off := offStr.(string)

‎sql/expression/function/insert.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,12 @@ func (i *Insert) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) {
127127
return nil, err
128128
}
129129

130+
// Handle Dolt's TextStorage wrapper that doesn't convert to plain string
131+
strVal, err = sql.UnwrapAny(ctx, strVal)
132+
if err != nil {
133+
return nil, err
134+
}
135+
130136
posVal, _, err := types.Int64.Convert(ctx, pos)
131137
if err != nil {
132138
return nil, err
@@ -142,6 +148,12 @@ func (i *Insert) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) {
142148
return nil, err
143149
}
144150

151+
// Handle Dolt's TextStorage wrapper that doesn't convert to plain string
152+
newStrVal, err = sql.UnwrapAny(ctx, newStrVal)
153+
if err != nil {
154+
return nil, err
155+
}
156+
145157
s := strVal.(string)
146158
p := posVal.(int64)
147159
l := lengthVal.(int64)

‎sql/expression/function/locate.go‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,17 @@ func (l *Locate) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) {
104104
return nil, nil
105105
}
106106

107+
substrVal, _, err = types.LongText.Convert(ctx, substrVal)
108+
if err != nil {
109+
return nil, err
110+
}
111+
112+
// Handle Dolt's TextStorage wrapper that doesn't convert to plain string
113+
substrVal, err = sql.UnwrapAny(ctx, substrVal)
114+
if err != nil {
115+
return nil, err
116+
}
117+
107118
substr, ok := substrVal.(string)
108119
if !ok {
109120
return nil, sql.ErrInvalidArgumentDetails.New("locate", "substring must be a string")
@@ -118,6 +129,17 @@ func (l *Locate) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) {
118129
return nil, nil
119130
}
120131

132+
strVal, _, err = types.LongText.Convert(ctx, strVal)
133+
if err != nil {
134+
return nil, err
135+
}
136+
137+
// Handle Dolt's TextStorage wrapper that doesn't convert to plain string
138+
strVal, err = sql.UnwrapAny(ctx, strVal)
139+
if err != nil {
140+
return nil, err
141+
}
142+
121143
str, ok := strVal.(string)
122144
if !ok {
123145
return nil, sql.ErrInvalidArgumentDetails.New("locate", "string must be a string")

‎sql/expression/function/make_set.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,13 @@ func (m *MakeSet) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) {
143143
if err != nil {
144144
return nil, err
145145
}
146+
147+
// Handle Dolt's TextStorage wrapper that doesn't convert to plain string
148+
valStr, err = sql.UnwrapAny(ctx, valStr)
149+
if err != nil {
150+
return nil, err
151+
}
152+
146153
result = append(result, valStr.(string))
147154
}
148155
}

‎sql/expression/function/reverse_repeat_replace.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,12 @@ func (r *Reverse) Eval(
6464
return nil, err
6565
}
6666

67+
// Handle Dolt's TextStorage wrapper that doesn't convert to plain string
68+
v, err = sql.UnwrapAny(ctx, v)
69+
if err != nil {
70+
return nil, err
71+
}
72+
6773
return reverseString(v.(string)), nil
6874
}
6975

@@ -162,6 +168,12 @@ func (r *Repeat) Eval(
162168
return nil, err
163169
}
164170

171+
// Handle Dolt's TextStorage wrapper that doesn't convert to plain string
172+
str, err = sql.UnwrapAny(ctx, str)
173+
if err != nil {
174+
return nil, err
175+
}
176+
165177
count, err := r.RightChild.Eval(ctx, row)
166178
if count == nil || err != nil {
167179
return nil, err

‎sql/expression/function/soundex.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,12 @@ func (s *Soundex) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) {
6666
return nil, err
6767
}
6868

69+
// Handle Dolt's TextStorage wrapper that doesn't convert to plain string
70+
v, err = sql.UnwrapAny(ctx, v)
71+
if err != nil {
72+
return nil, err
73+
}
74+
6975
var b strings.Builder
7076
var last rune
7177
for _, c := range strings.ToUpper(v.(string)) {

‎sql/expression/function/substring.go‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,17 @@ func (s *Substring) Eval(
8484
return nil, err
8585
}
8686

87+
str, _, err = types.LongText.Convert(ctx, str)
88+
if err != nil {
89+
return nil, err
90+
}
91+
92+
// Handle Dolt's TextStorage wrapper that doesn't convert to plain string
93+
str, err = sql.UnwrapAny(ctx, str)
94+
if err != nil {
95+
return nil, err
96+
}
97+
8798
var text []rune
8899
switch str := str.(type) {
89100
case string:
@@ -223,6 +234,13 @@ func (s *SubstringIndex) Eval(ctx *sql.Context, row sql.Row) (interface{}, error
223234
if err != nil {
224235
return nil, err
225236
}
237+
238+
// Handle Dolt's TextStorage wrapper that doesn't convert to plain string
239+
ex, err = sql.UnwrapAny(ctx, ex)
240+
if err != nil {
241+
return nil, err
242+
}
243+
226244
str, ok := ex.(string)
227245
if !ok {
228246
return nil, sql.ErrInvalidType.New(reflect.TypeOf(ex).String())
@@ -236,6 +254,13 @@ func (s *SubstringIndex) Eval(ctx *sql.Context, row sql.Row) (interface{}, error
236254
if err != nil {
237255
return nil, err
238256
}
257+
258+
// Handle Dolt's TextStorage wrapper that doesn't convert to plain string
259+
ex, err = sql.UnwrapAny(ctx, ex)
260+
if err != nil {
261+
return nil, err
262+
}
263+
239264
delim, ok := ex.(string)
240265
if !ok {
241266
return nil, sql.ErrInvalidType.New(reflect.TypeOf(ex).String())

0 commit comments

Comments
 (0)