Skip to content

Commit c79a923

Browse files
jycorjycor
authored
fix null and empty paths for json functions (#2346)
1 parent a028ca9 commit c79a923

14 files changed

+209
-132
lines changed

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ require (
44
github.com/cespare/xxhash/v2 v2.2.0
55
github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2
66
github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e
7-
github.com/dolthub/jsonpath v0.0.2-0.20240201003050-392940944c15
7+
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-20240209125211-6c93b0341608
1010
github.com/go-kit/kit v0.10.0

‎go.sum‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,8 @@ github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2 h1:u3PMzfF8RkKd3lB9pZ2bfn0qEG+1G
5454
github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2/go.mod h1:mIEZOHnFx4ZMQeawhw9rhsj+0zwQj7adVsnBX7t+eKY=
5555
github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e h1:kPsT4a47cw1+y/N5SSCkma7FhAPw7KeGmD6c9PBZW9Y=
5656
github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e/go.mod h1:KPUcpx070QOfJK1gNe0zx4pA5sicIK1GMikIGLKC168=
57-
github.com/dolthub/jsonpath v0.0.2-0.20240201003050-392940944c15 h1:sfTETOpsrNJPDn2KydiCtDgVu6Xopq8k3JP8PjFT22s=
58-
github.com/dolthub/jsonpath v0.0.2-0.20240201003050-392940944c15/go.mod h1:2/2zjLQ/JOOSbbSboojeg+cAwcRV0fDLzIiWch/lhqI=
57+
github.com/dolthub/jsonpath v0.0.2-0.20240227200619-19675ab05c71 h1:bMGS25NWAGTEtT5tOBsCuCrlYnLRKpbJVJkDbrTRhwQ=
58+
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=
6060
github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81/go.mod h1:siLfyv2c92W1eN/R4QqG/+RjjX5W2+gCTRjZxBjI3TY=
6161
github.com/dolthub/vitess v0.0.0-20240209125211-6c93b0341608 h1:jnInva1KcJJf/QQsxbN9tTJckOZf73EzUen8rrik0Yw=

‎sql/expression/function/json/json_array_append_test.go‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
package json
1616

1717
import (
18-
"fmt"
1918
"strings"
2019
"testing"
2120

@@ -54,10 +53,10 @@ func TestArrayAppend(t *testing.T) {
5453
{f1, sql.Row{json, "$.a[0]", 4.1}, `{"a": [1, 4.1], "b": [2, 3], "c": {"d": "foo"}}`, nil},
5554
{f1, sql.Row{json, "$.a[last]", 4.1}, `{"a": [1, 4.1], "b": [2, 3], "c": {"d": "foo"}}`, nil},
5655
{f1, sql.Row{json, "$[0]", 4.1}, `[{"a": 1, "b": [2, 3], "c": {"d": "foo"}}, 4.1]`, nil},
57-
{f1, sql.Row{json, "$.[0]", 4.1}, nil, fmt.Errorf("Invalid JSON path expression")},
58-
{f1, sql.Row{json, "foo", "test"}, nil, fmt.Errorf("Invalid JSON path expression")},
59-
{f1, sql.Row{json, "$.c.*", "test"}, nil, fmt.Errorf("Path expressions may not contain the * and ** tokens")},
60-
{f1, sql.Row{json, "$.c.**", "test"}, nil, fmt.Errorf("Path expressions may not contain the * and ** tokens")},
56+
{f1, sql.Row{json, "$.[0]", 4.1}, nil, ErrInvalidPath},
57+
{f1, sql.Row{json, "foo", "test"}, nil, ErrInvalidPath},
58+
{f1, sql.Row{json, "$.c.*", "test"}, nil, ErrPathWildcard},
59+
{f1, sql.Row{json, "$.c.**", "test"}, nil, ErrPathWildcard},
6160
{f1, sql.Row{json, "$", 10.1}, `[{"a": 1, "b": [2, 3], "c": {"d": "foo"}}, 10.1]`, nil},
6261
{f1, sql.Row{nil, "$", 42.7}, nil, nil},
6362
{f1, sql.Row{json, nil, 10}, nil, nil},

‎sql/expression/function/json/json_array_insert_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,10 @@ func TestArrayInsert(t *testing.T) {
5555
{f1, sql.Row{json, "$.b.c", 4}, nil, fmt.Errorf("A path expression is not a path to a cell in an array at character 5 of $.b.c")},
5656
{f1, sql.Row{json, "$.a[0]", 4.1}, json, nil},
5757
{f1, sql.Row{json, "$[0]", 4.1}, json, nil},
58-
{f1, sql.Row{json, "$.[0]", 4.1}, nil, fmt.Errorf("Invalid JSON path expression")},
59-
{f1, sql.Row{json, "foo", "test"}, nil, fmt.Errorf("Invalid JSON path expression")},
60-
{f1, sql.Row{json, "$.c.*", "test"}, nil, fmt.Errorf("Path expressions may not contain the * and ** tokens")},
61-
{f1, sql.Row{json, "$.c.**", "test"}, nil, fmt.Errorf("Path expressions may not contain the * and ** tokens")},
58+
{f1, sql.Row{json, "$.[0]", 4.1}, nil, ErrInvalidPath},
59+
{f1, sql.Row{json, "foo", "test"}, nil, ErrInvalidPath},
60+
{f1, sql.Row{json, "$.c.*", "test"}, nil, ErrPathWildcard},
61+
{f1, sql.Row{json, "$.c.**", "test"}, nil, ErrPathWildcard},
6262
{f1, sql.Row{json, "$", 10.1}, nil, fmt.Errorf("Path expression is not a path to a cell in an array: $")},
6363
{f1, sql.Row{nil, "$", 42.7}, nil, nil},
6464
{f1, sql.Row{json, nil, 10}, nil, nil},

‎sql/expression/function/json/json_common.go‎

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ import (
2121
"github.com/dolthub/go-mysql-server/sql/types"
2222
)
2323

24+
var ErrInvalidPath = fmt.Errorf("Invalid JSON path expression")
25+
var ErrPathWildcard = fmt.Errorf("Path expressions may not contain the * and ** tokens")
26+
2427
// getMutableJSONVal returns a JSONValue from the given row and expression. The underling value is deeply copied so that
2528
// you are free to use the mutation functions on the returned value.
2629
// nil will be returned only if the inputs are nil. This will not return an error, so callers must check.
@@ -80,22 +83,30 @@ type pathValPair struct {
8083
val sql.JSONWrapper
8184
}
8285

83-
// buildPathValue builds a pathValPair from the given row and expressions. This is a common pattern in json methods to have
84-
// pairs of arguments, and this ensures they are of the right type, non-nil, and they wrapped in a struct as a unit.
85-
func buildPathValue(ctx *sql.Context, pathExp sql.Expression, valExp sql.Expression, row sql.Row) (*pathValPair, error) {
86+
// buildPath builds a path from the given row and expression
87+
func buildPath(ctx *sql.Context, pathExp sql.Expression, row sql.Row) (interface{}, error) {
8688
path, err := pathExp.Eval(ctx, row)
8789
if err != nil {
8890
return nil, err
8991
}
90-
9192
if path == nil {
92-
// MySQL documented behavior is to return null, not error, if any path is null.
9393
return nil, nil
9494
}
95-
96-
// make sure path is string
9795
if _, ok := path.(string); !ok {
98-
return nil, fmt.Errorf("Invalid JSON path expression")
96+
return "", ErrInvalidPath
97+
}
98+
return path.(string), nil
99+
}
100+
101+
// buildPathValue builds a pathValPair from the given row and expressions. This is a common pattern in json methods to have
102+
// pairs of arguments, and this ensures they are of the right type, non-nil, and they wrapped in a struct as a unit.
103+
func buildPathValue(ctx *sql.Context, pathExp sql.Expression, valExp sql.Expression, row sql.Row) (*pathValPair, error) {
104+
path, err := buildPath(ctx, pathExp, row)
105+
if err != nil {
106+
return nil, err
107+
}
108+
if path == nil {
109+
return nil, nil
99110
}
100111

101112
val, err := valExp.Eval(ctx, row)
@@ -104,7 +115,7 @@ func buildPathValue(ctx *sql.Context, pathExp sql.Expression, valExp sql.Express
104115
}
105116
jsonVal, ok := val.(sql.JSONWrapper)
106117
if !ok {
107-
jsonVal = types.JSONDocument{val}
118+
jsonVal = types.JSONDocument{Val: val}
108119
}
109120

110121
return &pathValPair{path.(string), jsonVal}, nil

‎sql/expression/function/json/json_insert_test.go‎

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
package json
1616

1717
import (
18-
"fmt"
1918
"strings"
2019
"testing"
2120

@@ -41,25 +40,25 @@ func TestInsert(t *testing.T) {
4140
expected interface{}
4241
err error
4342
}{
44-
{f1, sql.Row{json, "$.a", 10.1}, json, nil}, // insert existing does nothing
45-
{f1, sql.Row{json, "$.e", "new"}, `{"a": 1, "b": [2, 3], "c": {"d": "foo"},"e":"new"}`, nil}, // insert new
46-
{f1, sql.Row{json, "$.c.d", "test"}, json, nil}, // insert existing nested does nothing
47-
{f2, sql.Row{json, "$.a", 10.1, "$.e", "new"}, `{"a": 1, "b": [2, 3], "c": {"d": "foo"},"e":"new"}`, nil}, // insert multiple, one change.
48-
{f1, sql.Row{json, "$.a.e", "test"}, json, nil}, // insert nested does nothing
49-
{f1, sql.Row{json, "$.c.e", "test"}, `{"a": 1, "b": [2, 3], "c": {"d": "foo","e":"test"}}`, nil}, // insert nested in existing struct
50-
{f1, sql.Row{json, "$.c[5]", 4.1}, `{"a": 1, "b": [2, 3], "c": [{"d": "foo"}, 4.1]}`, nil}, // insert struct with indexing out of range
51-
{f1, sql.Row{json, "$.b[0]", 4.1}, json, nil}, // insert element in array does nothing
52-
{f1, sql.Row{json, "$.b[5]", 4.1}, `{"a": 1, "b": [2, 3, 4.1], "c": {"d": "foo"}}`, nil}, // insert element in array out of range
53-
{f1, sql.Row{json, "$.b.c", 4}, json, nil}, // insert nested in array does nothing
54-
{f1, sql.Row{json, "$.a[0]", 4.1}, json, nil}, // struct as array does nothing
55-
{f1, sql.Row{json, "$[0]", 4.1}, json, nil}, // struct does nothing.
56-
{f1, sql.Row{json, "$.[0]", 4.1}, nil, fmt.Errorf("Invalid JSON path expression")}, // improper struct indexing
57-
{f1, sql.Row{json, "foo", "test"}, nil, fmt.Errorf("Invalid JSON path expression")}, // invalid path
58-
{f1, sql.Row{json, "$.c.*", "test"}, nil, fmt.Errorf("Path expressions may not contain the * and ** tokens")}, // path contains * wildcard
59-
{f1, sql.Row{json, "$.c.**", "test"}, nil, fmt.Errorf("Path expressions may not contain the * and ** tokens")}, // path contains ** wildcard
60-
{f1, sql.Row{json, "$", 10.1}, json, nil}, // whole document no opt
61-
{f1, sql.Row{nil, "$", 42.7}, nil, nil}, // null document returns null
62-
{f1, sql.Row{json, nil, 10}, nil, nil}, // if any path is null, return null
43+
{f1, sql.Row{json, "$.a", 10.1}, json, nil}, // insert existing does nothing
44+
{f1, sql.Row{json, "$.e", "new"}, `{"a": 1, "b": [2, 3], "c": {"d": "foo"},"e":"new"}`, nil}, // insert new
45+
{f1, sql.Row{json, "$.c.d", "test"}, json, nil}, // insert existing nested does nothing
46+
{f2, sql.Row{json, "$.a", 10.1, "$.e", "new"}, `{"a": 1, "b": [2, 3], "c": {"d": "foo"},"e":"new"}`, nil}, // insert multiple, one change.
47+
{f1, sql.Row{json, "$.a.e", "test"}, json, nil}, // insert nested does nothing
48+
{f1, sql.Row{json, "$.c.e", "test"}, `{"a": 1, "b": [2, 3], "c": {"d": "foo","e":"test"}}`, nil}, // insert nested in existing struct
49+
{f1, sql.Row{json, "$.c[5]", 4.1}, `{"a": 1, "b": [2, 3], "c": [{"d": "foo"}, 4.1]}`, nil}, // insert struct with indexing out of range
50+
{f1, sql.Row{json, "$.b[0]", 4.1}, json, nil}, // insert element in array does nothing
51+
{f1, sql.Row{json, "$.b[5]", 4.1}, `{"a": 1, "b": [2, 3, 4.1], "c": {"d": "foo"}}`, nil}, // insert element in array out of range
52+
{f1, sql.Row{json, "$.b.c", 4}, json, nil}, // insert nested in array does nothing
53+
{f1, sql.Row{json, "$.a[0]", 4.1}, json, nil}, // struct as array does nothing
54+
{f1, sql.Row{json, "$[0]", 4.1}, json, nil}, // struct does nothing.
55+
{f1, sql.Row{json, "$.[0]", 4.1}, nil, ErrInvalidPath}, // improper struct indexing
56+
{f1, sql.Row{json, "foo", "test"}, nil, ErrInvalidPath}, // invalid path
57+
{f1, sql.Row{json, "$.c.*", "test"}, nil, ErrPathWildcard}, // path contains * wildcard
58+
{f1, sql.Row{json, "$.c.**", "test"}, nil, ErrPathWildcard}, // path contains ** wildcard
59+
{f1, sql.Row{json, "$", 10.1}, json, nil}, // whole document no opt
60+
{f1, sql.Row{nil, "$", 42.7}, nil, nil}, // null document returns null
61+
{f1, sql.Row{json, nil, 10}, nil, nil}, // if any path is null, return null
6362

6463
// mysql> select JSON_INSERT(JSON_ARRAY(), "$[2]", 1 , "$[2]", 2 ,"$[2]", 3 ,"$[2]", 4);
6564
// +------------------------------------------------------------------------+

‎sql/expression/function/json/json_keys.go‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,9 @@ func (j *JSONKeys) IsNullable() bool {
8686

8787
// Eval implements the sql.Expression interface.
8888
func (j *JSONKeys) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) {
89+
span, ctx := ctx.Span(fmt.Sprintf("function.%s", j.FunctionName()))
90+
defer span.End()
91+
8992
doc, err := getJSONDocumentFromRow(ctx, row, j.JSON)
9093
if err != nil {
9194
return nil, err
@@ -94,21 +97,15 @@ func (j *JSONKeys) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) {
9497
return nil, nil
9598
}
9699

97-
pathVal, err := j.Path.Eval(ctx, row)
100+
path, err := buildPath(ctx, j.Path, row)
98101
if err != nil {
99-
return false, nil
102+
return nil, err
100103
}
101-
if pathVal == nil {
104+
if path == nil {
102105
return nil, nil
103106
}
104-
var path string
105-
if p, _, strErr := types.LongText.Convert(pathVal); strErr == nil {
106-
path = p.(string)
107-
} else {
108-
return nil, strErr
109-
}
110107

111-
js, err := jsonpath.JsonPathLookup(doc.Val, path)
108+
js, err := jsonpath.JsonPathLookup(doc.Val, path.(string))
112109
if err != nil {
113110
if errors.Is(err, jsonpath.ErrKeyError) {
114111
return nil, nil

‎sql/expression/function/json/json_keys_test.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,16 @@ func TestJSONKeys(t *testing.T) {
9090
exp: types.MustJSON(`["c", "aa", "bb"]`),
9191
},
9292

93+
{
94+
f: f2,
95+
row: sql.Row{`{"a": [1, false]}`, nil},
96+
exp: nil,
97+
},
98+
{
99+
f: f2,
100+
row: sql.Row{`{"a": [1, false]}`, 123},
101+
err: true,
102+
},
93103
{
94104
f: f2,
95105
row: sql.Row{`{"a": [1, false]}`, "$"},

‎sql/expression/function/json/json_length.go‎

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,10 @@
1515
package json
1616

1717
import (
18-
"encoding/json"
1918
"fmt"
2019

2120
"github.com/dolthub/jsonpath"
21+
"gopkg.in/src-d/go-errors.v1"
2222

2323
"github.com/dolthub/go-mysql-server/sql"
2424
"github.com/dolthub/go-mysql-server/sql/expression"
@@ -76,34 +76,33 @@ func (j *JsonLength) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) {
7676
span, ctx := ctx.Span("function.JsonLength")
7777
defer span.End()
7878

79-
js, err := j.JSON.Eval(ctx, row)
79+
doc, err := getJSONDocumentFromRow(ctx, row, j.JSON)
8080
if err != nil {
8181
return nil, err
8282
}
83-
if js == nil {
84-
return nil, err
83+
if doc == nil {
84+
return nil, nil
8585
}
8686

87-
strData, _, err := types.LongBlob.Convert(js)
87+
pathVal, err := j.Path.Eval(ctx, row)
8888
if err != nil {
89-
return nil, fmt.Errorf("invalid data type for JSON data in argument 1 to function json_length; a JSON string or JSON type is required")
89+
return nil, err
9090
}
91-
if strData == nil {
91+
if pathVal == nil {
9292
return nil, nil
9393
}
94-
95-
var jsonData interface{}
96-
if err = json.Unmarshal(strData.([]byte), &jsonData); err != nil {
97-
return nil, err
98-
}
99-
100-
path, err := j.Path.Eval(ctx, row)
101-
if err != nil {
102-
return nil, err
94+
var path string
95+
if p, _, strErr := types.LongText.Convert(pathVal); strErr == nil {
96+
path = p.(string)
97+
} else {
98+
return nil, strErr
10399
}
104100

105-
res, err := jsonpath.JsonPathLookup(jsonData, path.(string))
101+
res, err := jsonpath.JsonPathLookup(doc.Val, path)
106102
if err != nil {
103+
if errors.Is(err, jsonpath.ErrKeyError) {
104+
return nil, nil
105+
}
107106
return nil, err
108107
}
109108

0 commit comments

Comments
 (0)