Skip to content

Commit 83cb3b3

Browse files
arthurschreiberarthurschreiber
committed
evalengine: Fix NULL document handling in JSON functions (#19052)
Signed-off-by: Arthur Schreiber <arthur@planetscale.com>
1 parent 1132e9f commit 83cb3b3

5 files changed

Lines changed: 50 additions & 33 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
@@ -2319,6 +2319,15 @@ func (asm *assembler) Fn_JSON_KEYS(jp *json.Path) {
23192319
return 1
23202320
}, "FN JSON_KEYS (SP-1)")
23212321
} else {
2322+
if jp.ContainsWildcards() {
2323+
asm.emit(func(env *ExpressionEnv) int {
2324+
env.vm.err = errInvalidPathForTransform
2325+
return 1
2326+
}, "FN JSON_KEYS (SP-1), %q", jp.String())
2327+
2328+
return
2329+
}
2330+
23222331
asm.emit(func(env *ExpressionEnv) int {
23232332
doc := env.vm.stack[env.vm.sp-1]
23242333
if doc == nil {

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

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -129,16 +129,9 @@ func (call *builtinJSONExtract) compile(c *compiler) (ctype, error) {
129129
nullable := doct.nullable()
130130
skip := c.compileNullCheck1(doct)
131131

132-
// TODO: `*compiler.compileParseJSON` should handle `sqltypes.Null`` properly but
133-
// we'll handle it here until all call sites are fixed.
134-
var jt ctype
135-
if doct.Type != sqltypes.Null {
136-
jt, err = c.compileParseJSON("JSON_EXTRACT", doct, 1)
137-
if err != nil {
138-
return ctype{}, err
139-
}
140-
} else {
141-
jt = ctype{Type: sqltypes.Null, Flag: flagNull | flagNullable, Col: collationNull}
132+
jt, err := c.compileParseJSON("JSON_EXTRACT", doct, 1)
133+
if err != nil {
134+
return ctype{}, err
142135
}
143136

144137
staticPaths := make([]staticPath, 0, len(call.Arguments[1:]))
@@ -411,6 +404,13 @@ func (call *builtinJSONContainsPath) compile(c *compiler) (ctype, error) {
411404
return ctype{}, c.unsupported(call)
412405
}
413406

407+
skip := c.compileNullCheck1(doct)
408+
409+
_, err = c.compileParseJSON("JSON_CONTAINS_PATH", doct, 1)
410+
if err != nil {
411+
return ctype{}, err
412+
}
413+
414414
match, err := c.jsonExtractOneOrAll("JSON_CONTAINS_PATH", call.Arguments[1])
415415
if err != nil {
416416
return ctype{}, err
@@ -426,12 +426,10 @@ func (call *builtinJSONContainsPath) compile(c *compiler) (ctype, error) {
426426
paths = append(paths, jp)
427427
}
428428

429-
_, err = c.compileParseJSON("JSON_CONTAINS_PATH", doct, 1)
430-
if err != nil {
431-
return ctype{}, err
432-
}
433-
434429
c.asm.Fn_JSON_CONTAINS_PATH(match, paths)
430+
431+
c.asm.jumpDestination(skip)
432+
435433
return ctype{Type: sqltypes.Int64, Col: collationNumeric, Flag: flagIsBoolean | flagNullable}, nil
436434
}
437435

@@ -522,6 +520,8 @@ func (call *builtinJSONKeys) compile(c *compiler) (ctype, error) {
522520
return ctype{}, err
523521
}
524522

523+
skip := c.compileNullCheck1(doc)
524+
525525
_, err = c.compileParseJSON("JSON_KEYS", doc, 1)
526526
if err != nil {
527527
return ctype{}, err
@@ -533,11 +533,11 @@ func (call *builtinJSONKeys) compile(c *compiler) (ctype, error) {
533533
if err != nil {
534534
return ctype{}, err
535535
}
536-
if jp.ContainsWildcards() {
537-
return ctype{}, errInvalidPathForTransform
538-
}
539536
}
540537

541538
c.asm.Fn_JSON_KEYS(jp)
539+
540+
c.asm.jumpDestination(skip)
541+
542542
return ctype{Type: sqltypes.TypeJSON, Flag: flagNullable, Col: collationJSON}, nil
543543
}

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

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ var Cases = []TestCase{
3333
{Run: FnJSONKeys},
3434
{Run: FnJSONExtract},
3535
{Run: FnJSONContainsPath},
36+
{Run: FnJSONUnquote},
3637
{Run: JSONArray},
3738
{Run: JSONObject},
3839
{Run: CharsetConversionOperators},
@@ -180,21 +181,21 @@ var Cases = []TestCase{
180181

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

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

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

196197
for _, path2 := range inputJSONPaths {
197-
yield(fmt.Sprintf("JSON_EXTRACT('%s', '%s', '%s')", obj, path1, path2), nil, false)
198+
yield(fmt.Sprintf("JSON_EXTRACT(%s, '%s', '%s')", obj, path1, path2), nil, false)
198199
}
199200
}
200201
}
@@ -235,17 +236,21 @@ func FnJSONExtract(yield Query) {
235236
func FnJSONContainsPath(yield Query) {
236237
for _, obj := range inputJSONObjects {
237238
for _, path1 := range inputJSONPaths {
238-
yield(fmt.Sprintf("JSON_CONTAINS_PATH('%s', 'one', '%s')", obj, path1), nil, false)
239-
yield(fmt.Sprintf("JSON_CONTAINS_PATH('%s', 'all', '%s')", obj, path1), nil, false)
239+
yield(fmt.Sprintf("JSON_CONTAINS_PATH(%s, 'one', '%s')", obj, path1), nil, false)
240+
yield(fmt.Sprintf("JSON_CONTAINS_PATH(%s, 'all', '%s')", obj, path1), nil, false)
240241

241242
for _, path2 := range inputJSONPaths {
242-
yield(fmt.Sprintf("JSON_CONTAINS_PATH('%s', 'one', '%s', '%s')", obj, path1, path2), nil, false)
243-
yield(fmt.Sprintf("JSON_CONTAINS_PATH('%s', 'all', '%s', '%s')", obj, path1, path2), nil, false)
243+
yield(fmt.Sprintf("JSON_CONTAINS_PATH(%s, 'one', '%s', '%s')", obj, path1, path2), nil, false)
244+
yield(fmt.Sprintf("JSON_CONTAINS_PATH(%s, 'all', '%s', '%s')", obj, path1, path2), nil, false)
244245
}
245246
}
246247
}
247248
}
248249

250+
func FnJSONUnquote(yield Query) {
251+
yield("JSON_UNQUOTE(NULL)", nil, false)
252+
}
253+
249254
func JSONArray(yield Query) {
250255
for _, a := range inputJSONPrimitives {
251256
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)