From e6ba8f34357005c0bff0312e2c859d79cf58afa5 Mon Sep 17 00:00:00 2001 From: Neil Macneale IV Date: Tue, 25 Jul 2023 14:47:21 -0700 Subject: [PATCH 1/5] Move over existing JSONContainsPath code to new file --- sql/expression/function/json_contains_path.go | 56 +++++++++++++++++++ sql/expression/function/json_unsupported.go | 39 ------------- 2 files changed, 56 insertions(+), 39 deletions(-) create mode 100644 sql/expression/function/json_contains_path.go diff --git a/sql/expression/function/json_contains_path.go b/sql/expression/function/json_contains_path.go new file mode 100644 index 0000000000..a8ea32b6e7 --- /dev/null +++ b/sql/expression/function/json_contains_path.go @@ -0,0 +1,56 @@ +// Copyright 2023 Dolthub, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package function + +import "github.com/dolthub/go-mysql-server/sql" + +// JSON_CONTAINS_PATH(json_doc, one_or_all, path[, path] ...) +// +// JSONContainsPath Returns 0 or 1 to indicate whether a JSON document contains data at a given path or paths. Returns +// NULL if any argument is NULL. An error occurs if the json_doc argument is not a valid JSON document, any path +// argument is not a valid path expression, or one_or_all is not 'one' or 'all'. To check for a specific value at a +// path, use JSON_CONTAINS() instead. +// +// The return value is 0 if no specified path exists within the document. Otherwise, the return value depends on the +// one_or_all argument: +// - 'one': 1 if at least one path exists within the document, 0 otherwise. +// - 'all': 1 if all paths exist within the document, 0 otherwise. +// +// https://dev.mysql.com/doc/refman/8.0/en/json-search-functions.html#function_json-contains-path +type JSONContainsPath struct { + sql.Expression +} + +var _ sql.FunctionExpression = JSONContainsPath{} + +// NewJSONContainsPath creates a new JSONContainsPath function. +func NewJSONContainsPath(args ...sql.Expression) (sql.Expression, error) { + return nil, ErrUnsupportedJSONFunction.New(JSONContainsPath{}.FunctionName()) +} + +// FunctionName implements sql.FunctionExpression +func (j JSONContainsPath) FunctionName() string { + return "json_contains_path" +} + +// Description implements sql.FunctionExpression +func (j JSONContainsPath) Description() string { + return "returns whether JSON document contains any data at path." +} + +// IsUnsupported implements sql.UnsupportedFunctionStub +func (j JSONContainsPath) IsUnsupported() bool { + return true +} diff --git a/sql/expression/function/json_unsupported.go b/sql/expression/function/json_unsupported.go index b399641d43..9e1e692fb5 100644 --- a/sql/expression/function/json_unsupported.go +++ b/sql/expression/function/json_unsupported.go @@ -27,45 +27,6 @@ var ErrUnsupportedJSONFunction = errors.NewKind("unsupported JSON function: %s") // JSON search functions // /////////////////////////// -// JSON_CONTAINS_PATH(json_doc, one_or_all, path[, path] ...) -// -// JSONContainsPath Returns 0 or 1 to indicate whether a JSON document contains data at a given path or paths. Returns -// NULL if any argument is NULL. An error occurs if the json_doc argument is not a valid JSON document, any path -// argument is not a valid path expression, or one_or_all is not 'one' or 'all'. To check for a specific value at a -// path, use JSON_CONTAINS() instead. -// -// The return value is 0 if no specified path exists within the document. Otherwise, the return value depends on the -// one_or_all argument: -// - 'one': 1 if at least one path exists within the document, 0 otherwise. -// - 'all': 1 if all paths exist within the document, 0 otherwise. -// -// https://dev.mysql.com/doc/refman/8.0/en/json-search-functions.html#function_json-contains-path -type JSONContainsPath struct { - sql.Expression -} - -var _ sql.FunctionExpression = JSONContainsPath{} - -// NewJSONContainsPath creates a new JSONContainsPath function. -func NewJSONContainsPath(args ...sql.Expression) (sql.Expression, error) { - return nil, ErrUnsupportedJSONFunction.New(JSONContainsPath{}.FunctionName()) -} - -// FunctionName implements sql.FunctionExpression -func (j JSONContainsPath) FunctionName() string { - return "json_contains_path" -} - -// Description implements sql.FunctionExpression -func (j JSONContainsPath) Description() string { - return "returns whether JSON document contains any data at path." -} - -// IsUnsupported implements sql.UnsupportedFunctionStub -func (j JSONContainsPath) IsUnsupported() bool { - return true -} - // JSON_KEYS(json_doc[, path]) // // JSONKeys Returns the keys from the top-level value of a JSON object as a JSON array, or, if a path argument is given, From 7e150322849c19a1699be403d3de589064cdc522 Mon Sep 17 00:00:00 2001 From: Neil Macneale IV Date: Tue, 25 Jul 2023 17:35:23 -0700 Subject: [PATCH 2/5] Implmentation of json_contains_path(json_doc, one_or_all, path[, path] ...) Verified matched MySQL behavior, which differs from documentation. --- sql/expression/function/json_contains.go | 1 - sql/expression/function/json_contains_path.go | 124 +++++++++++++++++- .../function/json_contains_path_test.go | 113 ++++++++++++++++ 3 files changed, 233 insertions(+), 5 deletions(-) create mode 100644 sql/expression/function/json_contains_path_test.go diff --git a/sql/expression/function/json_contains.go b/sql/expression/function/json_contains.go index 5d762d504e..84a8584613 100644 --- a/sql/expression/function/json_contains.go +++ b/sql/expression/function/json_contains.go @@ -90,7 +90,6 @@ func (j *JSONContains) Resolved() bool { return false } } - return true } diff --git a/sql/expression/function/json_contains_path.go b/sql/expression/function/json_contains_path.go index a8ea32b6e7..44a0b4f137 100644 --- a/sql/expression/function/json_contains_path.go +++ b/sql/expression/function/json_contains_path.go @@ -14,7 +14,13 @@ package function -import "github.com/dolthub/go-mysql-server/sql" +import ( + "fmt" + "strings" + + "github.com/dolthub/go-mysql-server/sql" + "github.com/dolthub/go-mysql-server/sql/types" +) // JSON_CONTAINS_PATH(json_doc, one_or_all, path[, path] ...) // @@ -29,15 +35,125 @@ import "github.com/dolthub/go-mysql-server/sql" // - 'all': 1 if all paths exist within the document, 0 otherwise. // // https://dev.mysql.com/doc/refman/8.0/en/json-search-functions.html#function_json-contains-path +// +// Above is the documentation from MySQL's documentation. Minor Nit - the observed behavior for NULL +// paths is that if a NULL path is found before the search can terminate, then NULL is returned. type JSONContainsPath struct { - sql.Expression + doc sql.Expression + all sql.Expression + paths []sql.Expression +} + +func (j JSONContainsPath) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) { + target, err := getSearchableJSONVal(ctx, row, j.doc) + if err != nil || target == nil { + return nil, err + } + + oneOrAll, err := j.all.Eval(ctx, row) + if err != nil || oneOrAll == nil { + return nil, err + } + oneOrAll, _, err = types.LongText.Convert(oneOrAll) + if err != nil { + return nil, err + } + if !strings.EqualFold(oneOrAll.(string), "one") && !strings.EqualFold(oneOrAll.(string), "all") { + return nil, fmt.Errorf("The oneOrAll argument to json_contains_path may take these values: 'one' or 'all'") + } + findAllPaths := strings.EqualFold(oneOrAll.(string), "all") + + // MySQL Behavior differs from their docs. The docs say that if any path is NULL, the result is NULL. However, + // they only return NULL when they search far enough to find one, so we match that behavior. + for _, path := range j.paths { + path, err := path.Eval(ctx, row) + if err != nil || path == nil { + return nil, err + } + + path, _, err = types.LongText.Convert(path) + if err != nil { + return nil, err + } + + result, err := target.Extract(ctx, path.(string)) + if err != nil { + return nil, err + } + + if result == nil && findAllPaths { + return false, nil + } + if result != nil && !findAllPaths { + return true, nil + } + } + + // If we got this far, then we had no reason to terminate the search. For all, that means they all matched. + // For one, that means none matched. The result is the value of findAllPaths. + return findAllPaths, nil +} + +func (j JSONContainsPath) Resolved() bool { + for _, child := range j.Children() { + if child != nil && !child.Resolved() { + return false + } + } + return true +} + +func (j JSONContainsPath) String() string { + children := j.Children() + var parts = make([]string, len(children)) + + for i, c := range children { + parts[i] = c.String() + } + return fmt.Sprintf("%s(%s)", j.FunctionName(), strings.Join(parts, ",")) +} + +func (j JSONContainsPath) Type() sql.Type { + return types.Boolean +} + +func (j JSONContainsPath) IsNullable() bool { + for _, path := range j.paths { + if path.IsNullable() { + return true + } + } + if j.all.IsNullable() { + return true + } + return j.doc.IsNullable() +} +func (j JSONContainsPath) Children() []sql.Expression { + answer := make([]sql.Expression, 0, len(j.paths)+2) + + answer = append(answer, j.doc) + answer = append(answer, j.all) + answer = append(answer, j.paths...) + + return answer +} + +func (j JSONContainsPath) WithChildren(children ...sql.Expression) (sql.Expression, error) { + if len(j.Children()) != len(children) { + return nil, fmt.Errorf("json_contains_path did not receive the correct amount of args") + } + return NewJSONContainsPath(children...) } var _ sql.FunctionExpression = JSONContainsPath{} // NewJSONContainsPath creates a new JSONContainsPath function. func NewJSONContainsPath(args ...sql.Expression) (sql.Expression, error) { - return nil, ErrUnsupportedJSONFunction.New(JSONContainsPath{}.FunctionName()) + if len(args) < 3 { + return nil, sql.ErrInvalidArgumentNumber.New("JSON_CONTAINS_PATH", "3 or more", len(args)) + } + + return &JSONContainsPath{args[0], args[1], args[2:]}, nil } // FunctionName implements sql.FunctionExpression @@ -52,5 +168,5 @@ func (j JSONContainsPath) Description() string { // IsUnsupported implements sql.UnsupportedFunctionStub func (j JSONContainsPath) IsUnsupported() bool { - return true + return false } diff --git a/sql/expression/function/json_contains_path_test.go b/sql/expression/function/json_contains_path_test.go new file mode 100644 index 0000000000..2b8dce9c5a --- /dev/null +++ b/sql/expression/function/json_contains_path_test.go @@ -0,0 +1,113 @@ +// Copyright 2021 Dolthub, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package function + +import ( + "testing" + + "github.com/dolthub/go-mysql-server/sql" + "github.com/dolthub/go-mysql-server/sql/expression" + "github.com/dolthub/go-mysql-server/sql/types" + "github.com/pkg/errors" + "github.com/stretchr/testify/require" +) + +func TestJSONContainsPath(t *testing.T) { + // Verify arg count 3 or more. + _, err := NewJSONContainsPath() + require.Error(t, err) + + _, err = NewJSONContainsPath( + expression.NewGetField(0, types.JSON, "arg1", false), + ) + require.Error(t, err) + + _, err = NewJSONContainsPath( + expression.NewGetField(0, types.JSON, "arg1", false), + expression.NewGetField(1, types.LongText, "arg2", false), + ) + require.Error(t, err) + + // setup call expressions for calling with 1, 2, and 3 paths. + onePath, err := NewJSONContainsPath( + expression.NewGetField(0, types.JSON, "arg1", false), + expression.NewGetField(1, types.LongText, "arg2", false), + expression.NewGetField(2, types.LongText, "arg3", false), + ) + require.NoError(t, err) + + twoPath, err := NewJSONContainsPath( + expression.NewGetField(0, types.JSON, "arg1", false), + expression.NewGetField(1, types.LongText, "arg2", false), + expression.NewGetField(2, types.LongText, "arg3", false), + expression.NewGetField(3, types.LongText, "arg4", false), + ) + require.NoError(t, err) + + threePath, err := NewJSONContainsPath( + expression.NewGetField(0, types.JSON, "arg1", false), + expression.NewGetField(1, types.LongText, "arg2", false), + expression.NewGetField(2, types.LongText, "arg3", false), + expression.NewGetField(3, types.LongText, "arg4", false), + expression.NewGetField(4, types.LongText, "arg5", false), + ) + require.NoError(t, err) + + testCases := []struct { + fCall sql.Expression + input sql.Row + expected interface{} + err error + }{ + {onePath, sql.Row{`{"a": 1, "b": 2, "c": {"d": 4}}`, `oNe`, `$.a`}, true, nil}, + {onePath, sql.Row{`{"a": 1, "b": 2, "c": {"d": 4}}`, `one`, `$.e`}, false, nil}, + {onePath, sql.Row{`{"a": 1, "b": 2, "c": {"d": 4}}`, `all`, `$.e`}, false, nil}, + {onePath, sql.Row{`{"a": 1, "b": 2, "c": {"d": 4}}`, `All`, `$.c.d`}, true, nil}, + + {twoPath, sql.Row{`{"a": 1, "b": 2, "c": {"d": 4}}`, `one`, `$.a`, `$.e`}, true, nil}, + {twoPath, sql.Row{`{"a": 1, "b": 2, "c": {"d": 4}}`, `ALL`, `$.a`, `$.e`}, false, nil}, + + {twoPath, sql.Row{`{"a": 1, "b": 2, "c": {"d": {"e" : 42}}}`, `all`, `$.a`, `$.c.d.e`}, true, nil}, + {threePath, sql.Row{`{"a": 1, "b": 2, "c": {"d": {"e" : 42}}}`, `all`, `$.a`, `$.c.d.e`, `$.x`}, false, nil}, + {threePath, sql.Row{`{"a": 1, "b": 2, "c": {"d": {"e" : 42}}}`, `one`, `$.a`, `$.c.d.e`, `$.x`}, true, nil}, + + // NULL inputs. Any NULL should result in NULL output. + {onePath, sql.Row{nil, `one`, `$.a`}, nil, nil}, + {onePath, sql.Row{`{"a": 1}`, nil, `$.a`}, nil, nil}, + {twoPath, sql.Row{`{"a": 1}`, `one`, `$.a`, nil}, true, nil}, // Match MySQL behavior, not docs. + {twoPath, sql.Row{`{"a": 1}`, `one`, nil, `$.a`}, nil, nil}, + {twoPath, sql.Row{`{"a": 1}`, "all", `$.x`, nil}, false, nil}, // Match MySQL behavior, not docs. + {twoPath, sql.Row{`{"a": 1}`, `all`, `$.a`, nil}, nil, nil}, + + // Error cases + {onePath, sql.Row{`{"a": 1}`, `None`, `$.a`}, nil, errors.New("The oneOrAll argument to json_contains_path may take these values: 'one' or 'all'")}, + {onePath, sql.Row{`{"a": 1`, `One`, `$.a`}, nil, errors.New(`Invalid JSON text: {"a": 1`)}, + {threePath, sql.Row{`{"a": 1, "b": 2, "c": {"d": {"e" : 42}}}`, `one`, 42, `$.c.d.e`, `$.x`}, nil, errors.New(`should start with '$'`)}, + } + + for _, testcase := range testCases { + t.Run(testcase.fCall.String(), func(t *testing.T) { + require := require.New(t) + result, err := testcase.fCall.Eval(sql.NewEmptyContext(), testcase.input) + if testcase.err == nil { + require.NoError(err) + } else { + require.Equal(err.Error(), testcase.err.Error()) + } + + require.Equal(testcase.expected, result) + }) + } +} From 310846eca08ea8093f1b2f4ecc16abc34e07936d Mon Sep 17 00:00:00 2001 From: macneale4 Date: Tue, 8 Aug 2023 17:20:49 +0000 Subject: [PATCH 3/5] [ga-format-pr] Run ./format_repo.sh to fix formatting --- sql/expression/function/json_contains_path_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sql/expression/function/json_contains_path_test.go b/sql/expression/function/json_contains_path_test.go index 2b8dce9c5a..b886a99e55 100644 --- a/sql/expression/function/json_contains_path_test.go +++ b/sql/expression/function/json_contains_path_test.go @@ -17,11 +17,12 @@ package function import ( "testing" + "github.com/pkg/errors" + "github.com/stretchr/testify/require" + "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/go-mysql-server/sql/expression" "github.com/dolthub/go-mysql-server/sql/types" - "github.com/pkg/errors" - "github.com/stretchr/testify/require" ) func TestJSONContainsPath(t *testing.T) { From 3558080fa3ddf1ac02e9d5e1bc1fc402e806f6c8 Mon Sep 17 00:00:00 2001 From: Neil Macneale IV Date: Tue, 8 Aug 2023 14:24:21 -0700 Subject: [PATCH 4/5] Add engine tests --- enginetest/queries/json_scripts.go | 54 ++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/enginetest/queries/json_scripts.go b/enginetest/queries/json_scripts.go index a93bb6957d..90762cb456 100644 --- a/enginetest/queries/json_scripts.go +++ b/enginetest/queries/json_scripts.go @@ -566,4 +566,58 @@ var JsonScripts = []ScriptTest{ }, }, }, + { + Name: "json_contains_path returns true if the path exists", + SetUpScript: []string{ + `create table t (pk int primary key, col1 json);`, + `insert into t values (1, '{"a": 1}');`, + `insert into t values (2, '{"a": 1, "b": 2, "c": {"d": 4}}');`, + `insert into t values (3, '{"w": 1, "x": 2, "c": {"d": 4}}');`, + `insert into t values (4, '{}');`, + `insert into t values (5, null);`, + }, + + Assertions: []ScriptTestAssertion{ + { + Query: "select pk, json_contains_path(col1, 'one', '$.a') from t order by pk;", + Expected: []sql.Row{ + {1, true}, + {2, true}, + {3, false}, + {4, false}, + {5, nil}, + }, + }, + { + Query: "select pk, json_contains_path(col1, 'one', '$.a', '$.x', '$.c.d') from t order by pk;", + Expected: []sql.Row{ + {1, true}, + {2, true}, + {3, true}, + {4, false}, + {5, nil}, + }, + }, + { + Query: "select pk, json_contains_path(col1, 'all', '$.a', '$.x') from t order by pk;", + Expected: []sql.Row{ + {1, false}, + {2, false}, + {3, false}, + {4, false}, + {5, nil}, + }, + }, + { + Query: "select pk, json_contains_path(col1, 'all', '$.c.d', '$.x') from t order by pk;", + Expected: []sql.Row{ + {1, false}, + {2, false}, + {3, true}, + {4, false}, + {5, nil}, + }, + }, + }, + }, } From 7d6e9b8f3d473a9aabdf0f1d981552809d376c26 Mon Sep 17 00:00:00 2001 From: Neil Macneale IV Date: Wed, 9 Aug 2023 09:56:37 -0700 Subject: [PATCH 5/5] Add single test for Error path in enginetest. Other errors covered in unit tests --- enginetest/queries/json_scripts.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/enginetest/queries/json_scripts.go b/enginetest/queries/json_scripts.go index 90762cb456..543e5f15ae 100644 --- a/enginetest/queries/json_scripts.go +++ b/enginetest/queries/json_scripts.go @@ -618,6 +618,10 @@ var JsonScripts = []ScriptTest{ {5, nil}, }, }, + { + Query: "select pk, json_contains_path(col1, 'other', '$.c.d', '$.x') from t order by pk;", + ExpectedErrStr: "The oneOrAll argument to json_contains_path may take these values: 'one' or 'all'", + }, }, }, }