Skip to content

Commit 04dcac5

Browse files
arthurschreibermhamza15
arthurschreiber
authored and
mhamza15
committed
evalengine: Fix NULL document handling in JSON functions (#19052)
Signed-off-by: Arthur Schreiber <arthur@planetscale.com>
1 parent 292cb90 commit 04dcac5

5 files changed

Lines changed: 54 additions & 44 deletions

File tree

‎go/vt/vtgate/evalengine/compiler.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -585,6 +585,8 @@ func (c *compiler) compileParseJSON(fn string, doct ctype, offset int) (ctype, e
585585
case sqltypes.TypeJSON:
586586
case sqltypes.VarChar, sqltypes.VarBinary:
587587
c.asm.Parse_j(offset)
588+
case sqltypes.Null:
589+
return ctype{Type: sqltypes.Null, Flag: flagNull | flagNullable, Col: collationNull}, nil
588590
default:
589591
return ctype{}, errJSONType(fn)
590592
}

‎go/vt/vtgate/evalengine/compiler_asm.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2308,6 +2308,15 @@ func (asm *assembler) Fn_JSON_KEYS(jp *json.Path) {
23082308
return 1
23092309
}, "FN JSON_KEYS (SP-1)")
23102310
} else {
2311+
if jp.ContainsWildcards() {
2312+
asm.emit(func(env *ExpressionEnv) int {
2313+
env.vm.err = errInvalidPathForTransform
2314+
return 1
2315+
}, "FN JSON_KEYS (SP-1), %q", jp.String())
2316+
2317+
return
2318+
}
2319+
23112320
asm.emit(func(env *ExpressionEnv) int {
23122321
doc := env.vm.stack[env.vm.sp-1]
23132322
if doc == nil {

‎go/vt/vtgate/evalengine/fn_json.go‎

Lines changed: 21 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -136,16 +136,9 @@ func (call *builtinJSONExtract) compile(c *compiler) (ctype, error) {
136136
nullable := doct.nullable()
137137
skip := c.compileNullCheck1(doct)
138138

139-
// TODO: `*compiler.compileParseJSON` should handle `sqltypes.Null`` properly but
140-
// we'll handle it here until all call sites are fixed.
141-
var jt ctype
142-
if doct.Type != sqltypes.Null {
143-
jt, err = c.compileParseJSON("JSON_EXTRACT", doct, 1)
144-
if err != nil {
145-
return ctype{}, err
146-
}
147-
} else {
148-
jt = ctype{Type: sqltypes.Null, Flag: flagNull | flagNullable, Col: collationNull}
139+
jt, err := c.compileParseJSON("JSON_EXTRACT", doct, 1)
140+
if err != nil {
141+
return ctype{}, err
149142
}
150143

151144
staticPaths := make([]staticPath, 0, len(call.Arguments[1:]))
@@ -234,16 +227,9 @@ func (call *builtinJSONRemove) compile(c *compiler) (ctype, error) {
234227
nullable := doct.nullable()
235228
skip := c.compileNullCheck1(doct)
236229

237-
// TODO: `*compiler.compileParseJSON` should handle `sqltypes.Null`` properly but
238-
// we'll handle it here until all call sites are fixed.
239-
var jt ctype
240-
if doct.Type != sqltypes.Null {
241-
jt, err = c.compileParseJSON("JSON_REMOVE", doct, 1)
242-
if err != nil {
243-
return ctype{}, err
244-
}
245-
} else {
246-
jt = ctype{Type: sqltypes.Null, Flag: flagNull | flagNullable, Col: collationNull}
230+
jt, err := c.compileParseJSON("JSON_REMOVE", doct, 1)
231+
if err != nil {
232+
return ctype{}, err
247233
}
248234

249235
staticPaths := make([]staticPath, 0, len(call.Arguments[1:]))
@@ -515,6 +501,13 @@ func (call *builtinJSONContainsPath) compile(c *compiler) (ctype, error) {
515501
return ctype{}, c.unsupported(call)
516502
}
517503

504+
skip := c.compileNullCheck1(doct)
505+
506+
_, err = c.compileParseJSON("JSON_CONTAINS_PATH", doct, 1)
507+
if err != nil {
508+
return ctype{}, err
509+
}
510+
518511
match, err := c.jsonExtractOneOrAll("JSON_CONTAINS_PATH", call.Arguments[1])
519512
if err != nil {
520513
return ctype{}, err
@@ -530,12 +523,10 @@ func (call *builtinJSONContainsPath) compile(c *compiler) (ctype, error) {
530523
paths = append(paths, jp)
531524
}
532525

533-
_, err = c.compileParseJSON("JSON_CONTAINS_PATH", doct, 1)
534-
if err != nil {
535-
return ctype{}, err
536-
}
537-
538526
c.asm.Fn_JSON_CONTAINS_PATH(match, paths)
527+
528+
c.asm.jumpDestination(skip)
529+
539530
return ctype{Type: sqltypes.Int64, Col: collationNumeric, Flag: flagIsBoolean | flagNullable}, nil
540531
}
541532

@@ -626,6 +617,8 @@ func (call *builtinJSONKeys) compile(c *compiler) (ctype, error) {
626617
return ctype{}, err
627618
}
628619

620+
skip := c.compileNullCheck1(doc)
621+
629622
_, err = c.compileParseJSON("JSON_KEYS", doc, 1)
630623
if err != nil {
631624
return ctype{}, err
@@ -637,11 +630,11 @@ func (call *builtinJSONKeys) compile(c *compiler) (ctype, error) {
637630
if err != nil {
638631
return ctype{}, err
639632
}
640-
if jp.ContainsWildcards() {
641-
return ctype{}, errInvalidPathForTransform
642-
}
643633
}
644634

645635
c.asm.Fn_JSON_KEYS(jp)
636+
637+
c.asm.jumpDestination(skip)
638+
646639
return ctype{Type: sqltypes.TypeJSON, Flag: flagNullable, Col: collationJSON}, nil
647640
}

‎go/vt/vtgate/evalengine/testcases/cases.go‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ var Cases = []TestCase{
3434
{Run: FnJSONExtract},
3535
{Run: FnJSONRemove},
3636
{Run: FnJSONContainsPath},
37+
{Run: FnJSONUnquote},
3738
{Run: JSONArray},
3839
{Run: JSONObject},
3940
{Run: CharsetConversionOperators},
@@ -181,21 +182,21 @@ var Cases = []TestCase{
181182

182183
func FnJSONKeys(yield Query) {
183184
for _, obj := range inputJSONObjects {
184-
yield(fmt.Sprintf("JSON_KEYS('%s')", obj), nil, false)
185+
yield(fmt.Sprintf("JSON_KEYS(%s)", obj), nil, false)
185186

186187
for _, path1 := range inputJSONPaths {
187-
yield(fmt.Sprintf("JSON_KEYS('%s', '%s')", obj, path1), nil, false)
188+
yield(fmt.Sprintf("JSON_KEYS(%s, '%s')", obj, path1), nil, false)
188189
}
189190
}
190191
}
191192

192193
func FnJSONExtract(yield Query) {
193194
for _, obj := range inputJSONObjects {
194195
for _, path1 := range inputJSONPaths {
195-
yield(fmt.Sprintf("JSON_EXTRACT('%s', '%s')", obj, path1), nil, false)
196+
yield(fmt.Sprintf("JSON_EXTRACT(%s, '%s')", obj, path1), nil, false)
196197

197198
for _, path2 := range inputJSONPaths {
198-
yield(fmt.Sprintf("JSON_EXTRACT('%s', '%s', '%s')", obj, path1, path2), nil, false)
199+
yield(fmt.Sprintf("JSON_EXTRACT(%s, '%s', '%s')", obj, path1, path2), nil, false)
199200
}
200201
}
201202
}
@@ -236,25 +237,29 @@ func FnJSONExtract(yield Query) {
236237
func FnJSONRemove(yield Query) {
237238
for _, obj := range inputJSONObjects {
238239
for _, path1 := range inputJSONPaths {
239-
yield(fmt.Sprintf("JSON_REMOVE('%s', '%s')", obj, path1), nil, false)
240+
yield(fmt.Sprintf("JSON_REMOVE(%s, '%s')", obj, path1), nil, false)
240241
}
241242
}
242243
}
243244

244245
func FnJSONContainsPath(yield Query) {
245246
for _, obj := range inputJSONObjects {
246247
for _, path1 := range inputJSONPaths {
247-
yield(fmt.Sprintf("JSON_CONTAINS_PATH('%s', 'one', '%s')", obj, path1), nil, false)
248-
yield(fmt.Sprintf("JSON_CONTAINS_PATH('%s', 'all', '%s')", obj, path1), nil, false)
248+
yield(fmt.Sprintf("JSON_CONTAINS_PATH(%s, 'one', '%s')", obj, path1), nil, false)
249+
yield(fmt.Sprintf("JSON_CONTAINS_PATH(%s, 'all', '%s')", obj, path1), nil, false)
249250

250251
for _, path2 := range inputJSONPaths {
251-
yield(fmt.Sprintf("JSON_CONTAINS_PATH('%s', 'one', '%s', '%s')", obj, path1, path2), nil, false)
252-
yield(fmt.Sprintf("JSON_CONTAINS_PATH('%s', 'all', '%s', '%s')", obj, path1, path2), nil, false)
252+
yield(fmt.Sprintf("JSON_CONTAINS_PATH(%s, 'one', '%s', '%s')", obj, path1, path2), nil, false)
253+
yield(fmt.Sprintf("JSON_CONTAINS_PATH(%s, 'all', '%s', '%s')", obj, path1, path2), nil, false)
253254
}
254255
}
255256
}
256257
}
257258

259+
func FnJSONUnquote(yield Query) {
260+
yield("JSON_UNQUOTE(NULL)", nil, false)
261+
}
262+
258263
func JSONArray(yield Query) {
259264
for _, a := range inputJSONPrimitives {
260265
yield(fmt.Sprintf("JSON_ARRAY(%s)", a), nil, false)

‎go/vt/vtgate/evalengine/testcases/inputs.go‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,14 @@ import (
2424
)
2525

2626
var inputJSONObjects = []string{
27-
`[ { "a": 1 }, { "a": 2 } ]`,
28-
`{ "a" : "foo", "b" : [ true, { "c" : 123, "c" : 456 } ] }`,
29-
`{ "a" : "foo", "b" : [ true, { "c" : "123" } ] }`,
30-
`{ "a" : "foo", "b" : [ true, { "c" : 123 } ] }`,
31-
`{"a": 1, "b": 2, "c": {"d": 4}}`,
32-
`["a", {"b": [true, false]}, [10, 20]]`,
33-
`[10, 20, [30, 40]]`,
27+
`'[ { "a": 1 }, { "a": 2 } ]'`,
28+
`'{ "a" : "foo", "b" : [ true, { "c" : 123, "c" : 456 } ] }'`,
29+
`'{ "a" : "foo", "b" : [ true, { "c" : "123" } ] }'`,
30+
`'{ "a" : "foo", "b" : [ true, { "c" : 123 } ] }'`,
31+
`'{"a": 1, "b": 2, "c": {"d": 4}}'`,
32+
`'["a", {"b": [true, false]}, [10, 20]]'`,
33+
`'[10, 20, [30, 40]]'`,
34+
`NULL`,
3435
}
3536

3637
var inputJSONPaths = []string{

0 commit comments

Comments
 (0)