gustavodemorais commented on code in PR #28850:
URL: https://github.com/apache/flink/pull/28850#discussion_r3729870535
##########
flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/SqlJsonUtils.java:
##########
@@ -526,6 +532,78 @@ private static Object dejsonize(String input) {
return JSON_PATH_JSON_PROVIDER.parse(input);
}
+ /**
+ * Returns the JSON type flag for the parsed value: {@code OBJECT}, {@code
ARRAY}, {@code
+ * STRING}, {@code NUMBER}, {@code BOOLEAN}, or {@code NULL} for a JSON
null. Returns SQL {@code
+ * NULL} for invalid JSON.
+ */
+ public static String jsonType(final JsonValueContext parsedInput) {
+ // Unparsed, or shared with a call that never assigned it: report NULL
either way.
+ if (parsedInput == null || parsedInput.hasException()) {
+ return null;
+ }
+ return getJsonType(parsedInput.obj);
+ }
+
+ private static String getJsonType(final Object val) {
+ if (val instanceof Number) {
+ return "NUMBER";
+ } else if (val instanceof String) {
+ return "STRING";
+ } else if (val instanceof Boolean) {
+ return "BOOLEAN";
+ } else if (val instanceof Map) {
+ return "OBJECT";
+ } else if (val instanceof Collection) {
+ return "ARRAY";
+ } else if (val == null) {
+ // JSON null, distinct from the SQL NULL returned for invalid
input.
+ return "NULL";
+ }
+ return null;
+ }
+
+ /**
+ * Returns the JSON type flag at {@code path}, or {@code null} if the path
doesn't resolve to
+ * exactly one value. The {@code lax}/{@code strict} prefix is not
supported: there's no {@code
+ * ON ERROR} clause here for it to matter.
+ */
+ public static String jsonType(final JsonValueContext parsedInput, final
String path) {
+ if (parsedInput == null || parsedInput.hasException() ||
path.isEmpty()) {
+ return null;
+ }
+
+ if (JSON_PATH_BASE.matcher(path).matches()) {
+ throw new TableRuntimeException(
+ String.format(
+ "JSON_TYPE does not support the 'lax'/'strict'
path mode prefix (got: '%s'). "
+ + "Use a plain path such as '$.a.b'. To
check path existence or handle "
+ + "invalid input, use JSON_EXISTS or IS
JSON.",
+ path));
+ }
+
+ if (parsedInput.obj == null) {
+ // Null has no children, so only the root path resolves.
+ return "$".equals(path) ? "NULL" : null;
+ }
+
+ final Object value;
+ try {
+ // PathNotFoundException extends InvalidPathException, so this
also covers a path
+ // that doesn't match anything.
+ value = JsonPath.parse(parsedInput.obj,
JSON_PATH_TYPE_CONFIG).read(path);
+ } catch (Exception e) {
+ return null;
+ }
+
+ if (!JsonPath.isPathDefinite(path)) {
Review Comment:
"JsonPath.isPathDefinite(pathSpec)" This always yields the same result, the
path is always the same. However, this is currently being called for again for
every record in the pipeline. Move this to planning
```
Codegen can read literal - GeneratedExpression have field for it:
case class GeneratedExpression(
resultTerm: String, nullTerm: String, var code: String,
resultType: LogicalType, literalValue: Option[Any] = None)
Neighbours already do this. LikeCallGen.scala pull LIKE pattern this way.
JsonQueryCallGen and JsonValueCallGen pull their behaviors this way.
So in JsonCodeGenUtils:
final boolean definite =
!hasPath ||
JsonPath.isPathDefinite(operands.apply(1).literalValue().get().toString());
Compile happen once, at plan time. Then pass the answer down as plain
boolean:
String call = hasPath
? qualifyMethod(BuiltInMethods.JSON_TYPE_PATH())
+ "(" + parsed.varName + ", " + argTerms.apply(1) +
".toString(), " + definite + ")"
: qualifyMethod(BuiltInMethods.JSON_TYPE()) + "(" + parsed.varName +
")";
```
I think we are doing the same for json_length and should fix it there as
well. In general, the rule is: we want to do the least amount of repeated
process during runtime.
##########
flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/SqlJsonUtils.java:
##########
@@ -526,6 +532,78 @@ private static Object dejsonize(String input) {
return JSON_PATH_JSON_PROVIDER.parse(input);
}
+ /**
+ * Returns the JSON type flag for the parsed value: {@code OBJECT}, {@code
ARRAY}, {@code
+ * STRING}, {@code NUMBER}, {@code BOOLEAN}, or {@code NULL} for a JSON
null. Returns SQL {@code
+ * NULL} for invalid JSON.
+ */
+ public static String jsonType(final JsonValueContext parsedInput) {
+ // Unparsed, or shared with a call that never assigned it: report NULL
either way.
+ if (parsedInput == null || parsedInput.hasException()) {
+ return null;
+ }
+ return getJsonType(parsedInput.obj);
+ }
+
+ private static String getJsonType(final Object val) {
+ if (val instanceof Number) {
+ return "NUMBER";
+ } else if (val instanceof String) {
+ return "STRING";
+ } else if (val instanceof Boolean) {
+ return "BOOLEAN";
+ } else if (val instanceof Map) {
+ return "OBJECT";
+ } else if (val instanceof Collection) {
+ return "ARRAY";
+ } else if (val == null) {
+ // JSON null, distinct from the SQL NULL returned for invalid
input.
+ return "NULL";
+ }
+ return null;
+ }
+
+ /**
+ * Returns the JSON type flag at {@code path}, or {@code null} if the path
doesn't resolve to
+ * exactly one value. The {@code lax}/{@code strict} prefix is not
supported: there's no {@code
+ * ON ERROR} clause here for it to matter.
+ */
+ public static String jsonType(final JsonValueContext parsedInput, final
String path) {
+ if (parsedInput == null || parsedInput.hasException() ||
path.isEmpty()) {
+ return null;
+ }
+
+ if (JSON_PATH_BASE.matcher(path).matches()) {
+ throw new TableRuntimeException(
+ String.format(
+ "JSON_TYPE does not support the 'lax'/'strict'
path mode prefix (got: '%s'). "
+ + "Use a plain path such as '$.a.b'. To
check path existence or handle "
+ + "invalid input, use JSON_EXISTS or IS
JSON.",
+ path));
+ }
+
+ if (parsedInput.obj == null) {
+ // Null has no children, so only the root path resolves.
+ return "$".equals(path) ? "NULL" : null;
+ }
+
+ final Object value;
+ try {
+ // PathNotFoundException extends InvalidPathException, so this
also covers a path
+ // that doesn't match anything.
+ value = JsonPath.parse(parsedInput.obj,
JSON_PATH_TYPE_CONFIG).read(path);
+ } catch (Exception e) {
Review Comment:
Don't use generic exception in production code or else it might swallow
other issues that you want to know about
```suggestion
} catch (InvalidPathException e) {
```
--
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]
