gustavodemorais commented on code in PR #28688:
URL: https://github.com/apache/flink/pull/28688#discussion_r3613060875
##########
docs/data/sql_functions.yml:
##########
@@ -1223,6 +1218,60 @@ json:
-- [{"nested_json":{"value":42}}]
JSON_ARRAY(JSON('{"nested_json": {"value": 42}}'))
```
+ - sql: JSON_LENGTH(json_doc[, path])
+ table: jsonLength(jsonObject[, path])
+ description: |
+ Returns the number of elements in a JSON document, or the length of the
value at the specified path if one is provided.
+ Returns NULL if the argument is NULL, the json is invalid, or the path
does not locate a value.
+ eg.
+ -- 2
+ JSON_LENGTH('{1: "hello", 2: "bye bye"}')
+
+ -- 5
+ JSON_LENGTH('[1,2,3,4,5]')
+
+ -- 1
+ JSON_LENGTH('hello')
+
+ -- 1
+ JSON_LENGTH('{1: "hello", 2: "bye bye"}', '$.2')
+
+
+ The length is determined as follows:
+
+ - Scalar values (number, string, boolean): has length 1.
+ - Array: has a length equal to the number of its elements.
+ - Object: has a length equal to the number of its key-value pairs.
+
+ Nested arrays and objects each count as a single element and their
contents are not included in the count.
+
+ Lax and strict mode produce the same output, so mode selection is
disabled for this function.
Review Comment:
I think you can just drop this message. If users try it, they will see a
proper error message
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/JsonFunctionsITCase.java:
##########
@@ -97,6 +98,152 @@ Stream<TestSetSpec> getTestSetSpecs() {
return testCases.stream();
}
+ private static TestSetSpec jsonLengthSpec() {
+ final String jsonValue = getJsonFromResource("/json/json-exists.json");
+
+ return TestSetSpec.forFunction(BuiltInFunctionDefinitions.JSON_LENGTH)
+ .onFieldsWithData(
+ jsonValue,
+ "{\"a\":1,\"b\":2}",
+ "[1,2,3]",
+ "\"abc\"",
+ "null",
+ "{",
+ ((String) null), // f6: SQL NULL
+ "$",
+ "{\"a\":[true, false, null]}",
+ "{}",
+ "[]")
+ .andDataTypes(
+ STRING(), STRING(), STRING(), STRING(), STRING(),
STRING(), STRING(),
+ STRING(), STRING(), STRING(), STRING())
+ // path exists but resolves to a JSON null literal -> scalar,
length 1
+ // (distinct from a missing path, which yields NULL)
+ .testSqlResult("JSON_LENGTH(f8, '$.a[2]')", 1,
INT().nullable())
+ // contrast: missing paths on the same document -> NULL
+ .testSqlResult("JSON_LENGTH(f8, '$.a[9]')", null,
INT().nullable())
+ .testSqlResult("JSON_LENGTH(f8, '$.b')", null,
INT().nullable())
+
+ // SQL NULL input
+ .testSqlResult("JSON_LENGTH(f6)", null, INT().nullable())
+
+ // whole-document length from the existing resource:
+ .testSqlResult("JSON_LENGTH(f0)", 3, INT().nullable())
+
+ // basic shapes
+ .testSqlResult("JSON_LENGTH(f1)", 2, INT().nullable())
+ .testSqlResult("JSON_LENGTH(f2)", 3, INT().nullable())
+ .testSqlResult("JSON_LENGTH(f3)", 1, INT().nullable())
+
+ // empty containers -> 0
+ .testSqlResult("JSON_LENGTH(f9)", 0, INT().nullable())
+ .testSqlResult("JSON_LENGTH(f10)", 0, INT().nullable())
+ .testSqlResult("JSON_LENGTH(f9, '$')", 0, INT().nullable())
+ .testSqlResult("JSON_LENGTH(f10, '$')", 0, INT().nullable())
+
+ // keep this line aligned with your intended semantics
+ // current implementation returns NULL for JSON literal null
+ .testSqlResult("JSON_LENGTH(f4)", 1, INT().nullable())
+
+ // (valid) paths
+ .testSqlResult("JSON_LENGTH(f0, '$')", 3, INT().nullable())
+ .testSqlResult("JSON_LENGTH(f0, '$.type')", 1,
INT().nullable())
+ .testSqlResult("JSON_LENGTH(f0, '$.author')", 2,
INT().nullable())
+ .testSqlResult("JSON_LENGTH(f0, '$.author.address')", 2,
INT().nullable())
+ .testSqlResult("JSON_LENGTH(f0, '$.metadata.tags')", 3,
INT().nullable())
+ .testSqlResult("JSON_LENGTH(f0, '$.metadata.references')", 1,
INT().nullable())
+ .testSqlResult("JSON_LENGTH(f0, '$.metadata.references[0]')",
2, INT().nullable())
+ .testSqlResult(
+ "JSON_LENGTH(f0, '$.metadata.references[0].url')", 1,
INT().nullable())
+ // (invalid) path
+ .testSqlResult("JSON_LENGTH(f0, '$.missing')", null,
INT().nullable())
+ .testSqlResult("JSON_LENGTH(f7)", null, INT().nullable())
+
+ // invalid JSON -> NULL
+ .testSqlResult("JSON_LENGTH(f5)", null, INT().nullable())
+
+ // literal (NOT NULL) arguments must still yield a nullable
result,
+ // otherwise constant folding / output writing NPEs when the
path is missing
+ .testSqlResult("JSON_LENGTH('{\"a\":[1,2,3]}', '$.b')", null,
INT().nullable())
+ .testSqlResult("JSON_LENGTH('{\"a\":[1,2,3]}', '$.a')", 3,
INT().nullable())
+
+ // vs strict
+ .testSqlResult("JSON_LENGTH(f0, '$.type')", 1,
INT().nullable())
+ .testSqlResult("JSON_LENGTH(f0, '$.type')", 1,
INT().nullable())
+ .testSqlResult("JSON_LENGTH(f0, '$.author')", 2,
INT().nullable())
+ .testSqlResult("JSON_LENGTH(f0, '$.author')", 2,
INT().nullable())
+ .testSqlResult("JSON_LENGTH(f0, '$.metadata.tags')", 3,
INT().nullable())
+ .testSqlResult("JSON_LENGTH(f0, '$.metadata.tags')", 3,
INT().nullable())
+
+ // missing path: neither mode throws -> both yield NULL
+ // (this is the "doesn't throw" behaviour)
Review Comment:
```suggestion
```
We want to avoid comments where AI is over explaining like this. Delete
##########
docs/data/sql_functions.yml:
##########
@@ -1223,6 +1218,60 @@ json:
-- [{"nested_json":{"value":42}}]
JSON_ARRAY(JSON('{"nested_json": {"value": 42}}'))
```
+ - sql: JSON_LENGTH(json_doc[, path])
+ table: jsonLength(jsonObject[, path])
+ description: |
+ Returns the number of elements in a JSON document, or the length of the
value at the specified path if one is provided.
+ Returns NULL if the argument is NULL, the json is invalid, or the path
does not locate a value.
+ eg.
+ -- 2
+ JSON_LENGTH('{1: "hello", 2: "bye bye"}')
+
+ -- 5
+ JSON_LENGTH('[1,2,3,4,5]')
+
+ -- 1
+ JSON_LENGTH('hello')
+
+ -- 1
+ JSON_LENGTH('{1: "hello", 2: "bye bye"}', '$.2')
+
+
+ The length is determined as follows:
+
+ - Scalar values (number, string, boolean): has length 1.
+ - Array: has a length equal to the number of its elements.
+ - Object: has a length equal to the number of its key-value pairs.
+
+ Nested arrays and objects each count as a single element and their
contents are not included in the count.
+
+ Lax and strict mode produce the same output, so mode selection is
disabled for this function.
+
+ The path argument uses the form:
+
+ path ::= '$' ( '.' `field` | '[' `index` ']' )*
+ field ::= a key in a JSON object
+ index ::= a zero-based position in a JSON array
+
+ For example: `$.author.address` or `$.metadata.tags[0]`.
+
+ When provided with a path that uses a wildcard and resolves in 2 or more
paths, JSON_LENGTH will resolve as NULL.
+
+ Because a NULL result can mean several different things (the input is
not valid JSON, the path
+ does not match anything, or a wildcard path matched 2 or more nodes), it
is recommended to pair
+ JSON_LENGTH with a helper function so invalid input is handled
explicitly rather than silently
+ returning NULL:
+
+ - Without a path, guard the call with IS JSON to separate malformed
input from a real result:
+
+ -- returns the length only for valid JSON, otherwise NULL means
"invalid input"
+ SELECT CASE WHEN json_doc IS JSON THEN JSON_LENGTH(json_doc) END;
+
+ - With a path, use JSON_EXISTS to tell "the path is absent" apart from
"the path matched but
+ was ambiguous / matched 2 or more nodes":
+
+ -- path_present is TRUE even when JSON_LENGTH is NULL because of a
multi-match wildcard
+ SELECT JSON_EXISTS(json_doc, '$.items[*]'), JSON_LENGTH(json_doc,
'$.items[*]');
Review Comment:
Also, could we add a test for the $.items[*] wildcard shape we show in the
docs? Right now the docs promise that behavior but there's no coverage for [*]
with JSON_EXISTS
##########
docs/data/sql_functions.yml:
##########
@@ -1223,6 +1218,60 @@ json:
-- [{"nested_json":{"value":42}}]
JSON_ARRAY(JSON('{"nested_json": {"value": 42}}'))
```
+ - sql: JSON_LENGTH(json_doc[, path])
+ table: jsonLength(jsonObject[, path])
+ description: |
+ Returns the number of elements in a JSON document, or the length of the
value at the specified path if one is provided.
+ Returns NULL if the argument is NULL, the json is invalid, or the path
does not locate a value.
+ eg.
+ -- 2
+ JSON_LENGTH('{1: "hello", 2: "bye bye"}')
+
+ -- 5
+ JSON_LENGTH('[1,2,3,4,5]')
+
+ -- 1
+ JSON_LENGTH('hello')
+
+ -- 1
+ JSON_LENGTH('{1: "hello", 2: "bye bye"}', '$.2')
+
+
+ The length is determined as follows:
+
+ - Scalar values (number, string, boolean): has length 1.
+ - Array: has a length equal to the number of its elements.
+ - Object: has a length equal to the number of its key-value pairs.
+
+ Nested arrays and objects each count as a single element and their
contents are not included in the count.
+
+ Lax and strict mode produce the same output, so mode selection is
disabled for this function.
+
+ The path argument uses the form:
+
+ path ::= '$' ( '.' `field` | '[' `index` ']' )*
+ field ::= a key in a JSON object
+ index ::= a zero-based position in a JSON array
+
+ For example: `$.author.address` or `$.metadata.tags[0]`.
+
+ When provided with a path that uses a wildcard and resolves in 2 or more
paths, JSON_LENGTH will resolve as NULL.
+
+ Because a NULL result can mean several different things (the input is
not valid JSON, the path
+ does not match anything, or a wildcard path matched 2 or more nodes), it
is recommended to pair
+ JSON_LENGTH with a helper function so invalid input is handled
explicitly rather than silently
+ returning NULL:
+
+ - Without a path, guard the call with IS JSON to separate malformed
input from a real result:
+
+ -- returns the length only for valid JSON, otherwise NULL means
"invalid input"
+ SELECT CASE WHEN json_doc IS JSON THEN JSON_LENGTH(json_doc) END;
+
+ - With a path, use JSON_EXISTS to tell "the path is absent" apart from
"the path matched but
+ was ambiguous / matched 2 or more nodes":
+
+ -- path_present is TRUE even when JSON_LENGTH is NULL because of a
multi-match wildcard
+ SELECT JSON_EXISTS(json_doc, '$.items[*]'), JSON_LENGTH(json_doc,
'$.items[*]');
Review Comment:
This part of a bit too wordy. We can convey the same information in a more
concise way. Suggestion:
```suggestion
```suggestion
A wildcard path that matches 2 or more nodes returns NULL.
A NULL result is ambiguous - it means invalid JSON, no match, or a
multi-match wildcard.
Pair JSON_LENGTH with a helper function to handle these cases
explicitly:
-- IS JSON separates invalid input from a real result
SELECT CASE WHEN json_doc IS JSON THEN JSON_LENGTH(json_doc) END;
-- JSON_EXISTS separates an absent path from a present one
SELECT JSON_EXISTS(json_doc, '$.items[*]'), JSON_LENGTH(json_doc,
'$.items[*]');
-- To count multiple wildcard matches, wrap them into an array first
SELECT JSON_LENGTH(JSON_QUERY(json_doc, '$.items[*]' WITH ARRAY
WRAPPER));
```
```
##########
flink-python/pyflink/table/expression.py:
##########
@@ -2257,6 +2257,70 @@ def json_unquote(self) -> 'Expression':
"""
return _unary_op("jsonUnquote")(self)
+ def json_length(self, path = None) -> 'Expression':
Review Comment:
Look at how other docs are written. We need to change a bit the formatting
so it's more pythonic (content should be the same)
The -Scalar / -Arrays bullets need a space after the dash (- Scalar) to
render as a list. For the examples, let's use the reST doctest style like
json_exists does so they render as a code block:
Examples:
::
>>> lit('[1,2,3,4,5]').json_length() # 5
>>> lit('[1,2,3,4,5]').json_length('$[3]') # 1
Also, I think you need a small syntax fix in one example: ` $.[3] should be
$[3]. `
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/JsonFunctionsITCase.java:
##########
@@ -97,6 +98,152 @@ Stream<TestSetSpec> getTestSetSpecs() {
return testCases.stream();
}
+ private static TestSetSpec jsonLengthSpec() {
+ final String jsonValue = getJsonFromResource("/json/json-exists.json");
+
+ return TestSetSpec.forFunction(BuiltInFunctionDefinitions.JSON_LENGTH)
+ .onFieldsWithData(
+ jsonValue,
+ "{\"a\":1,\"b\":2}",
+ "[1,2,3]",
+ "\"abc\"",
+ "null",
+ "{",
+ ((String) null), // f6: SQL NULL
+ "$",
+ "{\"a\":[true, false, null]}",
+ "{}",
+ "[]")
+ .andDataTypes(
+ STRING(), STRING(), STRING(), STRING(), STRING(),
STRING(), STRING(),
+ STRING(), STRING(), STRING(), STRING())
+ // path exists but resolves to a JSON null literal -> scalar,
length 1
+ // (distinct from a missing path, which yields NULL)
+ .testSqlResult("JSON_LENGTH(f8, '$.a[2]')", 1,
INT().nullable())
+ // contrast: missing paths on the same document -> NULL
+ .testSqlResult("JSON_LENGTH(f8, '$.a[9]')", null,
INT().nullable())
+ .testSqlResult("JSON_LENGTH(f8, '$.b')", null,
INT().nullable())
+
+ // SQL NULL input
+ .testSqlResult("JSON_LENGTH(f6)", null, INT().nullable())
+
+ // whole-document length from the existing resource:
+ .testSqlResult("JSON_LENGTH(f0)", 3, INT().nullable())
+
+ // basic shapes
+ .testSqlResult("JSON_LENGTH(f1)", 2, INT().nullable())
+ .testSqlResult("JSON_LENGTH(f2)", 3, INT().nullable())
+ .testSqlResult("JSON_LENGTH(f3)", 1, INT().nullable())
+
+ // empty containers -> 0
+ .testSqlResult("JSON_LENGTH(f9)", 0, INT().nullable())
+ .testSqlResult("JSON_LENGTH(f10)", 0, INT().nullable())
+ .testSqlResult("JSON_LENGTH(f9, '$')", 0, INT().nullable())
+ .testSqlResult("JSON_LENGTH(f10, '$')", 0, INT().nullable())
+
+ // keep this line aligned with your intended semantics
+ // current implementation returns NULL for JSON literal null
+ .testSqlResult("JSON_LENGTH(f4)", 1, INT().nullable())
+
+ // (valid) paths
+ .testSqlResult("JSON_LENGTH(f0, '$')", 3, INT().nullable())
+ .testSqlResult("JSON_LENGTH(f0, '$.type')", 1,
INT().nullable())
+ .testSqlResult("JSON_LENGTH(f0, '$.author')", 2,
INT().nullable())
+ .testSqlResult("JSON_LENGTH(f0, '$.author.address')", 2,
INT().nullable())
+ .testSqlResult("JSON_LENGTH(f0, '$.metadata.tags')", 3,
INT().nullable())
+ .testSqlResult("JSON_LENGTH(f0, '$.metadata.references')", 1,
INT().nullable())
+ .testSqlResult("JSON_LENGTH(f0, '$.metadata.references[0]')",
2, INT().nullable())
+ .testSqlResult(
+ "JSON_LENGTH(f0, '$.metadata.references[0].url')", 1,
INT().nullable())
+ // (invalid) path
+ .testSqlResult("JSON_LENGTH(f0, '$.missing')", null,
INT().nullable())
+ .testSqlResult("JSON_LENGTH(f7)", null, INT().nullable())
+
+ // invalid JSON -> NULL
+ .testSqlResult("JSON_LENGTH(f5)", null, INT().nullable())
+
+ // literal (NOT NULL) arguments must still yield a nullable
result,
+ // otherwise constant folding / output writing NPEs when the
path is missing
+ .testSqlResult("JSON_LENGTH('{\"a\":[1,2,3]}', '$.b')", null,
INT().nullable())
+ .testSqlResult("JSON_LENGTH('{\"a\":[1,2,3]}', '$.a')", 3,
INT().nullable())
+
+ // vs strict
Review Comment:
Looks like a leftover from lax vs strict?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
