gustavodemorais commented on code in PR #28688:
URL: https://github.com/apache/flink/pull/28688#discussion_r3601984700


##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/JsonFunctionsITCase.java:
##########
@@ -97,6 +98,148 @@ 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, 'lax $.a[9]')", null, 
INT().nullable())
+                .testSqlResult("JSON_LENGTH(f8, 'lax $.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())
+
+                // lax vs strict

Review Comment:
   Since we've said both behaviors will be the same, I guess "// lax vs strict" 
tests are not bringing value and we can delete them?
   
   We should reject lax/strict mode with a nice error stating that we don't 
support and and point users to use the helper functions "IS JSON/JSON_EXISTS"



##########
docs/data/sql_functions.yml:
##########
@@ -1690,6 +1685,34 @@ 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.

Review Comment:
   ```suggestion
         Returns NULL if the argument is NULL, the json is invalid, or the path 
does not locate a value.
   ```



##########
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/internal/BaseExpressions.java:
##########
@@ -2525,6 +2525,32 @@ 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:
+     *
+     * <ul>
+     *   <li>Scalars have length 1.
+     *   <li>Arrays have length equal to their number of elements.
+     *   <li>Objects have length equal to their number of keys.
+     *   <li>Nested arrays/objects are not counted recursively.
+     *   <li>Returns Null if the json input is Null or if the given path does 
not resolve to
+     *       anything.
+     * </ul>
+     *
+     * <p>When a path is provided, the count applies to the value found at 
that path rather than the
+     * document as a whole. Returns {@code NULL} if any argument is {@code 
NULL} or the path does
+     * not locate a value, or if the path given describes more than 1 path ie. 
using the "*" wildcard.
+     */
+    public OutType jsonLength() {
+        return toApiSpecificExpression(unresolvedCall(JSON_LENGTH, toExpr()));
+    }
+
+    public OutType jsonLength(String path) {

Review Comment:
   Add documentation and examples here



##########
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':
+        """

Review Comment:
   Use the same javadoc and the one you added to the java interface in 
BaseExpressions.java. We use the Python API as a mirror of the java one and we 
don't want to have two versions



##########
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/internal/BaseExpressions.java:
##########
@@ -2525,6 +2525,32 @@ 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:
+     *
+     * <ul>
+     *   <li>Scalars have length 1.
+     *   <li>Arrays have length equal to their number of elements.
+     *   <li>Objects have length equal to their number of keys.
+     *   <li>Nested arrays/objects are not counted recursively.
+     *   <li>Returns Null if the json input is Null or if the given path does 
not resolve to
+     *       anything.
+     * </ul>
+     *

Review Comment:
   Examples are always very useful so add an example here as well



##########
docs/data/sql_functions.yml:
##########
@@ -1690,6 +1685,34 @@ 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. 
+        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')

Review Comment:
   Let's add an example using it with IS_JSON for the case where the user pass 
on param and JSON_EXISTS for the case with a path. We always want to point 
users to good patterns, which in this case is using  helper functions to make 
sure they explicitly deal with invalid json 



##########
docs/data/sql_functions.yml:
##########
@@ -1690,6 +1685,34 @@ 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. 
+        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
+      
+      
+      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.
+      
+      When provided with a path that uses a wildcard and resolves in 2 or more 
paths, JSON_LENGTH will resolve as NULL. 

Review Comment:
   Make sure to sync the chinese version



-- 
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]

Reply via email to