raminqaf commented on code in PR #28688:
URL: https://github.com/apache/flink/pull/28688#discussion_r3577860817
##########
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/internal/BaseExpressions.java:
##########
@@ -2524,6 +2525,26 @@ public OutType jsonQuery(String path, JsonQueryWrapper
wrappingBehavior) {
path, wrappingBehavior, JsonQueryOnEmptyOrError.NULL,
JsonQueryOnEmptyOrError.NULL);
}
+ /**
+ * Returns the number of elements contained in a JSON value, optionally at
a given path.
+ *
+ * <p>Return the length of a JSON value by given rules: - Scalars have
length 1. - Arrays have
+ * length equal to their number of elements. - Objects have length equal
to their number of
+ * keys. - Nested arrays/objects are not counted recursively. - Returns
None if the value is
+ * None.
Review Comment:
use `<li>` for each point please
##########
docs/data/sql_functions_zh.yml:
##########
@@ -1772,6 +1772,32 @@ bitmapagg:
`bitmap BITMAP`
返回一个 `BIGINT`。
+ - sql: JSON_LENGTH(json_doc[, path])
+ table: jsonLength(jsonObject[, path])
+ description: |
+ 返回一个 JSON 文档中的元素个数；如果提供了 path，则返回该路径下值的长度。
+ 如果参数为 NULL 或 path 未定位到任何值，则返回 NULL。
+ 例如：
+ -- 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')
+
+
+ 长度的确定方式如下：
+
+ - 标量值（数字、字符串、布尔值）：长度为 1。
+ - 数组：长度等于其元素个数。
+ - 对象：长度等于其键值对个数。
+
+ 嵌套的数组和对象各自算作一个元素，其内部内容不计入长度。
Review Comment:
No need to translate to Chinese unless you can read/write Chinese. You can
mirror the english docs as a place holder and leave the translation to the
Chinese native speakers.
##########
docs/data/sql_functions.yml:
##########
@@ -1685,6 +1685,32 @@ bitmapagg:
`bitmap BITMAP`
Returns a `BIGINT`.
+ - 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 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')
Review Comment:
A bit nicer to read like this
```suggestion
JSON_LENGTH('{1: "hello", 2: "bye bye"}')
-- 2
JSON_LENGTH('[1,2,3,4,5]')
-- 5
JSON_LENGTH('hello')
-- 1
JSON_LENGTH('{1: "hello", 2: "bye bye"}', '$.2')
-- 1
```
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java:
##########
@@ -3081,6 +3081,21 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL)
.runtimeProvided()
.build();
+ public static final BuiltInFunctionDefinition JSON_LENGTH =
+ BuiltInFunctionDefinition.newBuilder()
+ .name("JSON_LENGTH")
+ .kind(SCALAR)
+ .inputTypeStrategy(
+ or(
+
sequence(logical(LogicalTypeFamily.CHARACTER_STRING)),
Review Comment:
Just out of curiosity @snuyanzin can we and should we pass a `VARIANT` type
into JSON_* functions?
##########
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/internal/BaseExpressions.java:
##########
@@ -2524,6 +2525,26 @@ public OutType jsonQuery(String path, JsonQueryWrapper
wrappingBehavior) {
path, wrappingBehavior, JsonQueryOnEmptyOrError.NULL,
JsonQueryOnEmptyOrError.NULL);
}
+ /**
+ * Returns the number of elements contained in a JSON value, optionally at
a given path.
+ *
+ * <p>Return the length of a JSON value by given rules: - Scalars have
length 1. - Arrays have
+ * length equal to their number of elements. - Objects have length equal
to their number of
+ * keys. - Nested arrays/objects are not counted recursively. - Returns
None if the value is
+ * None.
Review Comment:
```suggestion
* <p>Return the length of a JSON value by given rules: - Scalars have
length 1. - Arrays have
* length equal to their number of elements. - Objects have length equal
to their number of
* keys. - Nested arrays/objects are not counted recursively. - Returns
Null if the value is
* Null.
```
Make the explanation a bit more verbose please. Which "value" are we talking
about here?
> Returns Null if the value is Null.
##########
flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/SqlJsonUtils.java:
##########
@@ -374,6 +375,77 @@ private static Object errorResultForJsonQuery(
}
}
+ /** Accepts a pre-parsed context from {@link #jsonParse}. */
+ public static Integer jsonLength(final JsonValueContext parsedInput) {
+ if (parsedInput.hasException()) {
+ return null;
+ }
+ // Whole document: a top-level JSON null literal counts as a scalar
(length 1).
+ return jsonLengthValue(parsedInput.obj);
+ }
+
+ /** Accepts a pre-parsed context from {@link #jsonParse}. */
+ public static Integer jsonLength(final JsonValueContext parsedInput,final
String pathSpec) {
+ if (parsedInput.hasException()) {
+ // invalid input JSON
+ return null;
+ }
+ final JsonPathContext context = jsonApiCommonSyntax(parsedInput,
pathSpec);
+ final Object value = context.hasException() ? null : context.obj;
+
+ if (value instanceof LinkedList) {
+ final LinkedList<?> matched = (LinkedList<?>) value;
+ if (matched.size() != 1) {
+ if (context.mode == PathMode.STRICT || matched.isEmpty()) {
+ return null;
+ }
+ return matched.size();
+ }
+ return jsonLengthValue(matched.get(0));
+ }
+ if (context.hasException() || value == null) {
+ return pathExists(parsedInput.obj, pathSpec) ? 1 : null;
+ }
+
+ return jsonLengthValue(value);
+ }
+
+ private static Integer jsonLengthValue(final Object value) {
Review Comment:
Here we never return `null` so we can use primitiv `int`
```suggestion
private static int jsonLengthValue(final Object value) {
```
##########
flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/SqlJsonUtils.java:
##########
@@ -374,6 +375,77 @@ private static Object errorResultForJsonQuery(
}
}
+ /** Accepts a pre-parsed context from {@link #jsonParse}. */
+ public static Integer jsonLength(final JsonValueContext parsedInput) {
+ if (parsedInput.hasException()) {
+ return null;
+ }
+ // Whole document: a top-level JSON null literal counts as a scalar
(length 1).
+ return jsonLengthValue(parsedInput.obj);
+ }
+
+ /** Accepts a pre-parsed context from {@link #jsonParse}. */
+ public static Integer jsonLength(final JsonValueContext parsedInput,final
String pathSpec) {
+ if (parsedInput.hasException()) {
+ // invalid input JSON
+ return null;
+ }
+ final JsonPathContext context = jsonApiCommonSyntax(parsedInput,
pathSpec);
+ final Object value = context.hasException() ? null : context.obj;
+
+ if (value instanceof LinkedList) {
+ final LinkedList<?> matched = (LinkedList<?>) value;
+ if (matched.size() != 1) {
+ if (context.mode == PathMode.STRICT || matched.isEmpty()) {
+ return null;
+ }
+ return matched.size();
+ }
+ return jsonLengthValue(matched.get(0));
+ }
+ if (context.hasException() || value == null) {
+ return pathExists(parsedInput.obj, pathSpec) ? 1 : null;
+ }
+
+ return jsonLengthValue(value);
+ }
+
+ private static Integer jsonLengthValue(final Object value) {
+ if (value instanceof Map) {
+ return ((Map<?, ?>) value).size();
+ }
+ if (value instanceof Collection) {
+ return ((Collection<?>) value).size();
+ }
+ // Scalars, including a JSON null literal, have length 1.
+ return 1;
+ }
+
+ /**
+ * Returns whether {@code pathSpec} matches at least one node in {@code
json}, independent of
+ * whether the matched value is a JSON null literal. Uses {@link
Option#AS_PATH_LIST} so the
+ * result is the list of matched canonical paths, which is empty iff the
path does not exist.
+ */
+ private static boolean pathExists(Object json, String pathSpec) {
Review Comment:
nit: Since you are at it
```suggestion
private static boolean pathExists(final Object json, final String
pathSpec) {
```
##########
flink-python/pyflink/table/expression.py:
##########
@@ -2257,6 +2257,18 @@ def json_unquote(self) -> 'Expression':
"""
return _unary_op("jsonUnquote")(self)
+ def json_length(self, path = None) -> 'Expression':
+ """
+ Return the length of a JSON value, or of the value at `path` if
given.
+
+ - Scalars have length 1.
+ - Arrays have length equal to their number of elements.
+ - Objects have length equal to their number of keys.
+ - Nested arrays/objects are not counted recursively.
Review Comment:
> Nested arrays/objects are not counted recursively.
Should be also added to the docs
##########
flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/SqlJsonUtils.java:
##########
@@ -374,6 +375,77 @@ private static Object errorResultForJsonQuery(
}
}
+ /** Accepts a pre-parsed context from {@link #jsonParse}. */
+ public static Integer jsonLength(final JsonValueContext parsedInput) {
+ if (parsedInput.hasException()) {
+ return null;
+ }
+ // Whole document: a top-level JSON null literal counts as a scalar
(length 1).
+ return jsonLengthValue(parsedInput.obj);
+ }
+
+ /** Accepts a pre-parsed context from {@link #jsonParse}. */
+ public static Integer jsonLength(final JsonValueContext parsedInput,final
String pathSpec) {
+ if (parsedInput.hasException()) {
+ // invalid input JSON
+ return null;
+ }
+ final JsonPathContext context = jsonApiCommonSyntax(parsedInput,
pathSpec);
+ final Object value = context.hasException() ? null : context.obj;
+
+ if (value instanceof LinkedList) {
+ final LinkedList<?> matched = (LinkedList<?>) value;
+ if (matched.size() != 1) {
+ if (context.mode == PathMode.STRICT || matched.isEmpty()) {
+ return null;
+ }
+ return matched.size();
+ }
+ return jsonLengthValue(matched.get(0));
+ }
+ if (context.hasException() || value == null) {
+ return pathExists(parsedInput.obj, pathSpec) ? 1 : null;
+ }
Review Comment:
See comment: https://github.com/apache/flink/pull/28688/changes#r3577941235
and move it higher up
##########
flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/SqlJsonUtils.java:
##########
@@ -374,6 +375,77 @@ private static Object errorResultForJsonQuery(
}
}
+ /** Accepts a pre-parsed context from {@link #jsonParse}. */
+ public static Integer jsonLength(final JsonValueContext parsedInput) {
+ if (parsedInput.hasException()) {
+ return null;
+ }
+ // Whole document: a top-level JSON null literal counts as a scalar
(length 1).
+ return jsonLengthValue(parsedInput.obj);
+ }
+
+ /** Accepts a pre-parsed context from {@link #jsonParse}. */
+ public static Integer jsonLength(final JsonValueContext parsedInput,final
String pathSpec) {
+ if (parsedInput.hasException()) {
+ // invalid input JSON
+ return null;
+ }
+ final JsonPathContext context = jsonApiCommonSyntax(parsedInput,
pathSpec);
+ final Object value = context.hasException() ? null : context.obj;
+
+ if (value instanceof LinkedList) {
+ final LinkedList<?> matched = (LinkedList<?>) value;
+ if (matched.size() != 1) {
+ if (context.mode == PathMode.STRICT || matched.isEmpty()) {
+ return null;
+ }
+ return matched.size();
+ }
Review Comment:
Could you please elaborate on this? In which conditions we get a `null` and
in which conditions we get the `size` of the `matched`?
##########
flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/SqlJsonUtils.java:
##########
@@ -374,6 +375,77 @@ private static Object errorResultForJsonQuery(
}
}
+ /** Accepts a pre-parsed context from {@link #jsonParse}. */
+ public static Integer jsonLength(final JsonValueContext parsedInput) {
+ if (parsedInput.hasException()) {
+ return null;
+ }
+ // Whole document: a top-level JSON null literal counts as a scalar
(length 1).
+ return jsonLengthValue(parsedInput.obj);
+ }
+
+ /** Accepts a pre-parsed context from {@link #jsonParse}. */
+ public static Integer jsonLength(final JsonValueContext parsedInput,final
String pathSpec) {
+ if (parsedInput.hasException()) {
+ // invalid input JSON
+ return null;
+ }
+ final JsonPathContext context = jsonApiCommonSyntax(parsedInput,
pathSpec);
+ final Object value = context.hasException() ? null : context.obj;
Review Comment:
if `context.hasException()` is true, then `value` would be `null`. We should
probably then short circuit and return `null` instead of continuing and risking
a NPE
##########
flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/SqlJsonUtils.java:
##########
@@ -374,6 +375,77 @@ private static Object errorResultForJsonQuery(
}
}
+ /** Accepts a pre-parsed context from {@link #jsonParse}. */
+ public static Integer jsonLength(final JsonValueContext parsedInput) {
+ if (parsedInput.hasException()) {
+ return null;
+ }
+ // Whole document: a top-level JSON null literal counts as a scalar
(length 1).
+ return jsonLengthValue(parsedInput.obj);
+ }
+
+ /** Accepts a pre-parsed context from {@link #jsonParse}. */
+ public static Integer jsonLength(final JsonValueContext parsedInput,final
String pathSpec) {
Review Comment:
you probably need to run `spotless`
```suggestion
public static Integer jsonLength(final JsonValueContext parsedInput,
final String pathSpec) {
```
--
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]
