gustavodemorais commented on code in PR #28850:
URL: https://github.com/apache/flink/pull/28850#discussion_r3728693658
##########
docs/data/sql_functions.yml:
##########
@@ -1267,6 +1267,50 @@ json:
-- JSON_EXISTS separates an absent path from a present one
SELECT JSON_EXISTS(json_doc, '$.items[*]'), JSON_LENGTH(json_doc,
'$.items[*]');
+ - sql: JSON_TYPE(jsonValue[, path])
+ table: jsonType(jsonValue[, path])
+ description: |
+ Returns a string value indicating the type of jsonValue, optionally at
the given JSON
+ `path`. Returns `NULL` if jsonValue is `NULL`, is not valid JSON, or the
path does not
+ resolve to exactly one value.
+
+ The possible results are `OBJECT`, `ARRAY`, `STRING`, `NUMBER` and
`BOOLEAN`, plus the string
+ `'NULL'` for the JSON null literal. These are the types of JSON's own
grammar.
Review Comment:
AI over documentation. Not something a user is searching when reading docs
for how the function works.
```suggestion
`'NULL'` for the JSON null literal.
```
##########
docs/data/sql_functions.yml:
##########
@@ -1267,6 +1267,50 @@ json:
-- JSON_EXISTS separates an absent path from a present one
SELECT JSON_EXISTS(json_doc, '$.items[*]'), JSON_LENGTH(json_doc,
'$.items[*]');
+ - sql: JSON_TYPE(jsonValue[, path])
+ table: jsonType(jsonValue[, path])
+ description: |
+ Returns a string value indicating the type of jsonValue, optionally at
the given JSON
+ `path`. Returns `NULL` if jsonValue is `NULL`, is not valid JSON, or the
path does not
+ resolve to exactly one value.
+
+ The possible results are `OBJECT`, `ARRAY`, `STRING`, `NUMBER` and
`BOOLEAN`, plus the string
+ `'NULL'` for the JSON null literal. These are the types of JSON's own
grammar.
+
+ JSON has a single number rule with no width, so every number is reported
as `NUMBER`.
Review Comment:
```suggestion
All number format is reported as `NUMBER`.
```
##########
docs/data/sql_functions.yml:
##########
@@ -1267,6 +1267,50 @@ json:
-- JSON_EXISTS separates an absent path from a present one
SELECT JSON_EXISTS(json_doc, '$.items[*]'), JSON_LENGTH(json_doc,
'$.items[*]');
+ - sql: JSON_TYPE(jsonValue[, path])
+ table: jsonType(jsonValue[, path])
+ description: |
+ Returns a string value indicating the type of jsonValue, optionally at
the given JSON
+ `path`. Returns `NULL` if jsonValue is `NULL`, is not valid JSON, or the
path does not
+ resolve to exactly one value.
+
+ The possible results are `OBJECT`, `ARRAY`, `STRING`, `NUMBER` and
`BOOLEAN`, plus the string
+ `'NULL'` for the JSON null literal. These are the types of JSON's own
grammar.
+
+ JSON has a single number rule with no width, so every number is reported
as `NUMBER`.
+ Anything quoted is a `STRING`, including dates and date-times.
+
+ ```sql
+ -- OBJECT
+ SELECT JSON_TYPE('{"a": true}')
+
+ -- ARRAY
+ SELECT JSON_TYPE('[1, 2]')
+
+ -- the JSON null literal, returned as the string 'NULL'
+ SELECT JSON_TYPE('null')
+
+ -- BOOLEAN
+ SELECT JSON_TYPE('true')
+
+ -- STRING
+ SELECT JSON_TYPE('"Hello, World!"')
+
+ -- STRING, since a date in JSON is just a quoted string
+ SELECT JSON_TYPE('"2015-01-01"')
+
+ -- NUMBER
+ SELECT JSON_TYPE('66')
+
+ -- NUMBER, whatever the magnitude or precision
+ SELECT JSON_TYPE('11.1')
+
+ -- NULL, since 68s is not valid JSON
+ SELECT JSON_TYPE('68s')
+
+ -- ARRAY, read at the given path
+ SELECT JSON_TYPE('{"a": [1, 2]}', '$.a')
Review Comment:
A lot of examples, but simple to read, I like it 👍
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/JsonFunctionsITCase.java:
##########
@@ -1413,6 +1414,221 @@ private static List<TestSetSpec> jsonObjectSpec() {
STRING().notNull()));
}
+ private static List<TestSetSpec> jsonTypeSpec() {
+ return List.of(
+ // One flag per JSON type.
+ TestSetSpec.forFunction(BuiltInFunctionDefinitions.JSON_TYPE)
+ .onFieldsWithData(
+ "{\"a\": true}", "[1, 2]", "null", "true",
"66", "\"hello\"")
+ .andDataTypes(STRING(), STRING(), STRING(), STRING(),
STRING(), STRING())
+ .testResult(
+ $("f0").jsonType(), "JSON_TYPE(f0)", "OBJECT",
STRING().nullable())
+ .testResult(
+ $("f1").jsonType(), "JSON_TYPE(f1)", "ARRAY",
STRING().nullable())
+ .testResult(
+ $("f2").jsonType(), "JSON_TYPE(f2)", "NULL",
STRING().nullable())
+ .testResult(
+ $("f3").jsonType(), "JSON_TYPE(f3)",
"BOOLEAN", STRING().nullable())
+ .testResult(
+ $("f4").jsonType(), "JSON_TYPE(f4)", "NUMBER",
STRING().nullable())
+ .testResult(
+ $("f5").jsonType(), "JSON_TYPE(f5)", "STRING",
STRING().nullable()),
+
+ // Every number is NUMBER, whatever its magnitude or precision.
+ TestSetSpec.forFunction(BuiltInFunctionDefinitions.JSON_TYPE)
+ .onFieldsWithData(
+ "1.5", "1e2", "11.1", "1e40", "4294967296",
"99999999999999999999")
+ .andDataTypes(STRING(), STRING(), STRING(), STRING(),
STRING(), STRING())
+ .testResult(
+ $("f0").jsonType(), "JSON_TYPE(f0)", "NUMBER",
STRING().nullable())
+ .testResult(
+ $("f1").jsonType(), "JSON_TYPE(f1)", "NUMBER",
STRING().nullable())
+ .testResult(
+ $("f2").jsonType(), "JSON_TYPE(f2)", "NUMBER",
STRING().nullable())
+ .testResult(
+ $("f3").jsonType(), "JSON_TYPE(f3)", "NUMBER",
STRING().nullable())
+ .testResult(
+ $("f4").jsonType(), "JSON_TYPE(f4)", "NUMBER",
STRING().nullable())
+ .testResult(
+ $("f5").jsonType(), "JSON_TYPE(f5)", "NUMBER",
STRING().nullable()),
+
+ // Anything quoted is a STRING, with no inference from its
contents.
+ TestSetSpec.forFunction(BuiltInFunctionDefinitions.JSON_TYPE)
+ .onFieldsWithData(
+ "\"2015-01-01\"",
+ "\"2015-01-01T10:00:00\"",
+ "\"2015-01-01 09:30:00\"",
+ "\"2015\"",
+ "\"66\"",
+ "\"\"")
Review Comment:
Reduce the amount of test cases in jsonTypeSpec - without reducing coverage.
For example, here you want to test that a string maps to a string and you have
4 types of date. You can have only one test case for that. Always make sure you
have as close to 100% test coverage but with fewer, high quality tests.
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java:
##########
@@ -3104,6 +3104,24 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL)
.runtimeProvided()
.build();
+ public static final BuiltInFunctionDefinition JSON_TYPE =
+ BuiltInFunctionDefinition.newBuilder()
+ .name("JSON_TYPE")
+ .kind(SCALAR)
+ .inputTypeStrategy(
+ or(
+
sequence(logical(LogicalTypeFamily.CHARACTER_STRING)),
+ sequence(
+
logical(LogicalTypeFamily.CHARACTER_STRING),
+ and(
+
logical(LogicalTypeFamily.CHARACTER_STRING),
+ LITERAL))))
+ // Nullable rather than nullableIfArgs: a non-null but
unparseable input also
+ // yields NULL.
Review Comment:
AI overexplaning again? I don't even see a nullable
```suggestion
```
##########
docs/data/sql_functions_zh.yml:
##########
@@ -1353,6 +1353,50 @@ json:
-- JSON_EXISTS separates an absent path from a present one
SELECT JSON_EXISTS(json_doc, '$.items[*]'), JSON_LENGTH(json_doc,
'$.items[*]');
+ - sql: JSON_TYPE(jsonValue[, path])
+ table: jsonType(jsonValue[, path])
+ description: |
+ Returns a string value indicating the type of jsonValue, optionally at
the given JSON
+ `path`. Returns `NULL` if jsonValue is `NULL`, is not valid JSON, or the
path does not
+ resolve to exactly one value.
+
+ The possible results are `OBJECT`, `ARRAY`, `STRING`, `NUMBER` and
`BOOLEAN`, plus the string
+ `'NULL'` for the JSON null literal. These are the types of JSON's own
grammar.
+
+ JSON has a single number rule with no width, so every number is reported
as `NUMBER`.
+ Anything quoted is a `STRING`, including dates and date-times.
+
+ ```sql
+ -- OBJECT
+ SELECT JSON_TYPE('{"a": true}')
+
+ -- ARRAY
+ SELECT JSON_TYPE('[1, 2]')
+
+ -- the JSON null literal, returned as the string 'NULL'
+ SELECT JSON_TYPE('null')
+
+ -- BOOLEAN
+ SELECT JSON_TYPE('true')
+
+ -- STRING
+ SELECT JSON_TYPE('"Hello, World!"')
+
+ -- STRING, since a date in JSON is just a quoted string
Review Comment:
```suggestion
-- STRING
```
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/JsonFunctionsITCase.java:
##########
@@ -1413,6 +1414,221 @@ private static List<TestSetSpec> jsonObjectSpec() {
STRING().notNull()));
}
+ private static List<TestSetSpec> jsonTypeSpec() {
+ return List.of(
+ // One flag per JSON type.
+ TestSetSpec.forFunction(BuiltInFunctionDefinitions.JSON_TYPE)
+ .onFieldsWithData(
+ "{\"a\": true}", "[1, 2]", "null", "true",
"66", "\"hello\"")
+ .andDataTypes(STRING(), STRING(), STRING(), STRING(),
STRING(), STRING())
+ .testResult(
+ $("f0").jsonType(), "JSON_TYPE(f0)", "OBJECT",
STRING().nullable())
+ .testResult(
+ $("f1").jsonType(), "JSON_TYPE(f1)", "ARRAY",
STRING().nullable())
+ .testResult(
+ $("f2").jsonType(), "JSON_TYPE(f2)", "NULL",
STRING().nullable())
+ .testResult(
+ $("f3").jsonType(), "JSON_TYPE(f3)",
"BOOLEAN", STRING().nullable())
+ .testResult(
+ $("f4").jsonType(), "JSON_TYPE(f4)", "NUMBER",
STRING().nullable())
+ .testResult(
+ $("f5").jsonType(), "JSON_TYPE(f5)", "STRING",
STRING().nullable()),
+
+ // Every number is NUMBER, whatever its magnitude or precision.
+ TestSetSpec.forFunction(BuiltInFunctionDefinitions.JSON_TYPE)
+ .onFieldsWithData(
+ "1.5", "1e2", "11.1", "1e40", "4294967296",
"99999999999999999999")
+ .andDataTypes(STRING(), STRING(), STRING(), STRING(),
STRING(), STRING())
+ .testResult(
+ $("f0").jsonType(), "JSON_TYPE(f0)", "NUMBER",
STRING().nullable())
+ .testResult(
+ $("f1").jsonType(), "JSON_TYPE(f1)", "NUMBER",
STRING().nullable())
+ .testResult(
+ $("f2").jsonType(), "JSON_TYPE(f2)", "NUMBER",
STRING().nullable())
+ .testResult(
+ $("f3").jsonType(), "JSON_TYPE(f3)", "NUMBER",
STRING().nullable())
+ .testResult(
+ $("f4").jsonType(), "JSON_TYPE(f4)", "NUMBER",
STRING().nullable())
+ .testResult(
+ $("f5").jsonType(), "JSON_TYPE(f5)", "NUMBER",
STRING().nullable()),
+
+ // Anything quoted is a STRING, with no inference from its
contents.
+ TestSetSpec.forFunction(BuiltInFunctionDefinitions.JSON_TYPE)
+ .onFieldsWithData(
+ "\"2015-01-01\"",
+ "\"2015-01-01T10:00:00\"",
+ "\"2015-01-01 09:30:00\"",
+ "\"2015\"",
+ "\"66\"",
+ "\"\"")
+ .andDataTypes(STRING(), STRING(), STRING(), STRING(),
STRING(), STRING())
+ .testResult(
+ $("f0").jsonType(), "JSON_TYPE(f0)", "STRING",
STRING().nullable())
+ .testResult(
+ $("f1").jsonType(), "JSON_TYPE(f1)", "STRING",
STRING().nullable())
+ .testResult(
+ $("f2").jsonType(), "JSON_TYPE(f2)", "STRING",
STRING().nullable())
+ .testResult(
+ $("f3").jsonType(), "JSON_TYPE(f3)", "STRING",
STRING().nullable())
+ .testResult(
+ $("f4").jsonType(), "JSON_TYPE(f4)", "STRING",
STRING().nullable())
+ .testResult(
+ $("f5").jsonType(), "JSON_TYPE(f5)", "STRING",
STRING().nullable()),
+
+ // A SQL NULL input yields a SQL NULL, not the 'NULL' flag; so
does invalid JSON.
+ TestSetSpec.forFunction(BuiltInFunctionDefinitions.JSON_TYPE)
+ .onFieldsWithData("{", "")
+ .andDataTypes(STRING(), STRING())
+ .testResult(
+ nullOf(STRING()).jsonType(),
+ "JSON_TYPE(CAST(NULL AS STRING))",
+ null,
+ STRING().nullable())
+ .testResult($("f0").jsonType(), "JSON_TYPE(f0)", null,
STRING().nullable())
+ .testResult($("f1").jsonType(), "JSON_TYPE(f1)", null,
STRING().nullable()),
+
+ // A path reads the type at that location instead of the root.
+ TestSetSpec.forFunction(BuiltInFunctionDefinitions.JSON_TYPE)
+ .onFieldsWithData("{\"a\": {\"b\": [1, 2]}, \"c\":
\"hi\", \"d\": null}")
+ .andDataTypes(STRING())
+ .testResult(
+ $("f0").jsonType("$"),
+ "JSON_TYPE(f0, '$')",
+ "OBJECT",
+ STRING().nullable())
+ .testResult(
+ $("f0").jsonType("$.c"),
+ "JSON_TYPE(f0, '$.c')",
+ "STRING",
+ STRING().nullable())
+ .testResult(
+ $("f0").jsonType("$.a"),
+ "JSON_TYPE(f0, '$.a')",
+ "OBJECT",
+ STRING().nullable())
+ .testResult(
+ $("f0").jsonType("$.a.b"),
+ "JSON_TYPE(f0, '$.a.b')",
+ "ARRAY",
+ STRING().nullable())
+ .testResult(
+ $("f0").jsonType("$.a.b[0]"),
+ "JSON_TYPE(f0, '$.a.b[0]')",
+ "NUMBER",
+ STRING().nullable())
+ .testResult(
+ $("f0").jsonType("$.d"),
+ "JSON_TYPE(f0, '$.d')",
+ "NULL",
+ STRING().nullable()),
+
+ // A wildcard path is indefinite: it reads back as a list, so
it has a type only
+ // if it resolves to exactly one value.
+ TestSetSpec.forFunction(BuiltInFunctionDefinitions.JSON_TYPE)
+ .onFieldsWithData(
+ "{\"a\": [1, 2]}",
+ "{\"a\": [1]}",
+ "{\"x\": 1, \"y\": 2}",
+ "{\"x\": 1}")
+ .andDataTypes(STRING(), STRING(), STRING(), STRING())
+ .testResult(
+ $("f0").jsonType("$.a[*]"),
+ "JSON_TYPE(f0, '$.a[*]')",
+ null,
+ STRING().nullable())
+ .testResult(
+ $("f1").jsonType("$.a[*]"),
+ "JSON_TYPE(f1, '$.a[*]')",
+ "NUMBER",
+ STRING().nullable())
+ .testResult(
+ $("f2").jsonType("$.*"),
+ "JSON_TYPE(f2, '$.*')",
+ null,
+ STRING().nullable())
+ .testResult(
+ $("f3").jsonType("$.*"),
+ "JSON_TYPE(f3, '$.*')",
+ "NUMBER",
+ STRING().nullable()),
+
+ // A path that does not resolve to anything yields NULL, same
as invalid JSON.
+ TestSetSpec.forFunction(BuiltInFunctionDefinitions.JSON_TYPE)
+ .onFieldsWithData("{\"a\": 1}")
+ .andDataTypes(STRING())
+ .testResult(
+ $("f0").jsonType("$.b"),
+ "JSON_TYPE(f0, '$.b')",
+ null,
+ STRING().nullable())
+ .testResult(
+ $("f0").jsonType("$.a.b"),
+ "JSON_TYPE(f0, '$.a.b')",
+ null,
+ STRING().nullable())
+ .testResult(
+ $("f0").jsonType(""),
+ "JSON_TYPE(f0, '')",
+ null,
+ STRING().nullable()),
+
+ // A JSON null has no children: only the root path resolves.
+ TestSetSpec.forFunction(BuiltInFunctionDefinitions.JSON_TYPE)
+ .onFieldsWithData("null")
+ .andDataTypes(STRING())
+ .testResult(
+ $("f0").jsonType("$"),
+ "JSON_TYPE(f0, '$')",
+ "NULL",
+ STRING().nullable())
+ .testResult(
+ $("f0").jsonType("$.a"),
+ "JSON_TYPE(f0, '$.a')",
+ null,
+ STRING().nullable()),
+
+ // The 'lax'/'strict' path mode prefix is rejected: there's no
ON ERROR clause
+ // here for it to matter.
+ TestSetSpec.forFunction(BuiltInFunctionDefinitions.JSON_TYPE)
+ .onFieldsWithData("{\"a\": 1}")
+ .andDataTypes(STRING())
+ .testSqlRuntimeError(
+ "JSON_TYPE(f0, 'lax $.a')",
+ TableRuntimeException.class,
Review Comment:
This should be a validation error, not runtime error. It can be thrown
during planning and doesn't depend on any records or runtime information
--
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]
