raminqaf commented on code in PR #29483:
URL: https://github.com/apache/flink/pull/29483#discussion_r4292920410
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/codegen/JsonCodeGenUtils.java:
##########
@@ -22,16 +22,220 @@
import org.apache.flink.table.planner.codegen.calls.JsonParseReuse;
import org.apache.flink.table.runtime.functions.SqlJsonUtils;
import org.apache.flink.table.types.logical.LogicalType;
+import org.apache.flink.types.variant.Variant;
import java.lang.reflect.Method;
+import scala.Option;
import scala.collection.Seq;
/** Utilities for the code generation of JSON functions. */
public final class JsonCodeGenUtils {
+ private static final int PARSED_VALUE_INDEX = Integer.MIN_VALUE;
+ private static final int PARSED_ERROR_INDEX = Integer.MIN_VALUE + 1;
+
private JsonCodeGenUtils() {}
+ /**
+ * Generates {@code PARSE_JSON(str[, allowDuplicateKeys])} ({@code isTry
== false}) or {@code
+ * TRY_PARSE_JSON(...)} ({@code isTry == true}).
+ *
+ * <p>The input is parsed at most once per record for a given (input,
allowDuplicateKeys) pair
+ * and shared across all such calls; {@code PARSE_JSON} rethrows a parse
error while {@code
+ * TRY_PARSE_JSON} returns NULL.
+ *
+ * <p>For {@code PARSE_JSON(str)} the generated code looks like:
+ *
+ * <pre>{@code
+ * // reusable members shared by every call on the same (input, allowDup):
+ * Variant variantParsed$0;
+ * Throwable variantParseError$0;
+ * StringData variantParseInput$0;
+ * java.lang.Boolean variantParseDup$0;
+ *
+ * private Variant parseVariant$0(StringData in, java.lang.Boolean
allowDup) {
+ * if (in != variantParseInput$0 || !java.util.Objects.equals(allowDup,
variantParseDup$0)) {
Review Comment:
Should we invert the if statement and return earlier?
##########
flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/SqlJsonUtils.java:
##########
@@ -611,6 +614,26 @@ public static JsonValueContext jsonParse(String input) {
return jsonValueExpression(input);
}
+ /** Parses a JSON string into a {@link Variant}, throwing on malformed
input. */
+ public static Variant parseJsonToVariant(StringData jsonStr, boolean
allowDuplicateKeys) {
+ final byte[] bytes = jsonStr.toBytes();
Review Comment:
Just out of curiosity, what are the trade-offs doing something like this:
```suggestion
public static Variant parseJsonToVariant(byte[] jsonBytes, boolean
allowDuplicateKeys) {
```
##########
flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/codegen/calls/BuiltInMethods.scala:
##########
@@ -514,6 +514,12 @@ object BuiltInMethods {
val JSON_PARSE =
Types.lookupMethod(classOf[SqlJsonUtils], "jsonParse", classOf[String])
+ val VARIANT_PARSE_JSON = Types.lookupMethod(
Review Comment:
nit: Even though I like the `VARIANT_` prefix, but maybe it is better to
keep the naming aligned with the Builtin function every where
```suggestion
val PARSE_JSON = Types.lookupMethod(
```
--
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]
