twalthr commented on code in PR #29073:
URL: https://github.com/apache/flink/pull/29073#discussion_r3924446501


##########
docs/content.zh/docs/sql/reference/data-types.md:
##########
@@ -1588,6 +1588,33 @@ rendering. Use `JSON_STRING` for the JSON representation 
instead, where a string
 `"foo"` and an object or array is serialized. A variant that stores a JSON 
`null` casts to SQL
 `NULL`.
 
+A `VARIANT` can also be cast to a constructed target, which imposes a schema 
on it. A variant array
+casts to `ARRAY<T>`. The variant must be an array, otherwise the cast fails. 
Each element is itself a
+`VARIANT`, so it casts to the element type `T` by the same rules, recursively, 
bottoming out at the
+scalar cast above. A leaf is never parsed either, so a stored string does not 
reach an integer
+target. Cast the leaf to `STRING` first and convert with a regular cast.

Review Comment:
   ```suggestion
   `VARIANT`, so it casts to the element type `T` by the same rules, 
recursively.
   ```



##########
docs/content.zh/docs/sql/reference/data-types.md:
##########
@@ -1588,6 +1588,33 @@ rendering. Use `JSON_STRING` for the JSON representation 
instead, where a string
 `"foo"` and an object or array is serialized. A variant that stores a JSON 
`null` casts to SQL
 `NULL`.
 
+A `VARIANT` can also be cast to a constructed target, which imposes a schema 
on it. A variant array
+casts to `ARRAY<T>`. The variant must be an array, otherwise the cast fails. 
Each element is itself a
+`VARIANT`, so it casts to the element type `T` by the same rules, recursively, 
bottoming out at the
+scalar cast above. A leaf is never parsed either, so a stored string does not 
reach an integer
+target. Cast the leaf to `STRING` first and convert with a regular cast.
+
+- Each element casts to `T`. A JSON null element maps to SQL `NULL` when `T` 
is nullable and fails
+  the cast when `T` is `NOT NULL`. An empty array casts to an empty `ARRAY<T>`.
+- `ARRAY<VARIANT>` is the identity leaf: it shreds one level and keeps each 
element as a variant. A

Review Comment:
   ```suggestion
   - `ARRAY<VARIANT>` is the identity element: it shreds one level and keeps 
each element as a variant. A
   ```



##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/logical/utils/LogicalTypeCasts.java:
##########
@@ -666,6 +668,12 @@ private static boolean supportsCasting(
             return supportsStructuredCasting(
                     sourceType, targetType, (s, t) -> supportsCasting(s, t, 
allowExplicit));
 
+        } else if (sourceRoot == VARIANT && targetRoot == ARRAY) {
+            // A variant array casts to ARRAY<T> when VARIANT casts to the 
single element type T.
+            // Explicit only, so no accidental coercion. Each runtime element 
is cast to T by the
+            // array cast rule; a per-element mismatch fails there, not here.
+            return allowExplicit
+                    && supportsCasting(sourceType, ((ArrayType) 
targetType).getElementType(), true);

Review Comment:
   why this?
   ```suggestion
               return supportsCasting(sourceType, ((ArrayType) 
targetType).getElementType(), true);
   ```



##########
docs/content.zh/docs/sql/reference/data-types.md:
##########
@@ -1588,6 +1588,33 @@ rendering. Use `JSON_STRING` for the JSON representation 
instead, where a string
 `"foo"` and an object or array is serialized. A variant that stores a JSON 
`null` casts to SQL
 `NULL`.
 
+A `VARIANT` can also be cast to a constructed target, which imposes a schema 
on it. A variant array
+casts to `ARRAY<T>`. The variant must be an array, otherwise the cast fails. 
Each element is itself a
+`VARIANT`, so it casts to the element type `T` by the same rules, recursively, 
bottoming out at the
+scalar cast above. A leaf is never parsed either, so a stored string does not 
reach an integer
+target. Cast the leaf to `STRING` first and convert with a regular cast.
+
+- Each element casts to `T`. A JSON null element maps to SQL `NULL` when `T` 
is nullable and fails
+  the cast when `T` is `NOT NULL`. An empty array casts to an empty `ARRAY<T>`.
+- `ARRAY<VARIANT>` is the identity leaf: it shreds one level and keeps each 
element as a variant. A
+  JSON null element stays a variant null rather than becoming SQL `NULL`.
+
+If any element cast fails, the whole cast fails, and `TRY_CAST` returns `NULL` 
for the entire value
+rather than a partial result. An element type with no variant counterpart, 
such as
+`ARRAY<INTERVAL YEAR TO MONTH>`, is rejected at validation. A top-level JSON 
`null` casts to SQL `NULL` for a nullable target before any
+shape check runs.

Review Comment:
   ```suggestion
   `ARRAY<INTERVAL YEAR TO MONTH>`, is rejected at validation. 
   ```



##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/casting/AbstractVariantToConstructedCastRule.java:
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.table.planner.functions.casting;
+
+import org.apache.flink.table.types.logical.LogicalType;
+import org.apache.flink.types.variant.Variant;
+
+import static 
org.apache.flink.table.planner.functions.casting.CastRuleUtils.methodCall;
+
+/**
+ * Base class for the rules that cast a {@link
+ * org.apache.flink.table.types.logical.LogicalTypeRoot#VARIANT} to a 
constructed target, imposing a

Review Comment:
   nit: import for better readability
   ```suggestion
    * LogicalTypeRoot#VARIANT} to a constructed target, imposing a
   ```



##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/casting/AbstractVariantToConstructedCastRule.java:
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.table.planner.functions.casting;
+
+import org.apache.flink.table.types.logical.LogicalType;
+import org.apache.flink.types.variant.Variant;
+
+import static 
org.apache.flink.table.planner.functions.casting.CastRuleUtils.methodCall;
+
+/**
+ * Base class for the rules that cast a {@link
+ * org.apache.flink.table.types.logical.LogicalTypeRoot#VARIANT} to a 
constructed target, imposing a
+ * schema on a variant. A constructed cast is the scalar cast applied to every 
leaf plus a shape
+ * check at each level, so the recursion bottoms out at the same scalar cast 
the primitive and
+ * string rules perform and no new leaf semantics are introduced.
+ *
+ * <p>A constructed cast can always fail, on a shape mismatch, an unreadable 
leaf, or a missing
+ * {@code NOT NULL} field, so {@code TRY_CAST} wraps the whole value and 
returns {@code NULL} for
+ * any failure rather than a partial result.
+ */
+abstract class AbstractVariantToConstructedCastRule<OUT>
+        extends AbstractNullAwareCodeGeneratorCastRule<Variant, OUT> {
+
+    protected AbstractVariantToConstructedCastRule(CastRulePredicate 
predicate) {
+        super(predicate);
+    }
+
+    @Override
+    public boolean canFail(LogicalType inputLogicalType, LogicalType 
targetLogicalType) {
+        return true;
+    }
+
+    /**
+     * Treats a variant that stores a JSON {@code null} as a {@code NULL} 
input, so a top-level JSON
+     * null casts to SQL {@code NULL} before any shape check runs. Only 
applied for a nullable
+     * target: a {@code NOT NULL} result cannot carry {@code NULL}, so a 
null-valued variant then
+     * fails the shape check as a regular mismatch.

Review Comment:
   I would not mention JSON here. Variant is not JSON, it is a dedicated type.



##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/casting/VariantToArrayCastRule.java:
##########
@@ -0,0 +1,171 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.table.planner.functions.casting;
+
+import org.apache.flink.table.data.ArrayData;
+import org.apache.flink.table.data.GenericArrayData;
+import org.apache.flink.table.planner.codegen.CodeGenUtils;
+import org.apache.flink.table.runtime.functions.VariantCastUtils;
+import org.apache.flink.table.types.logical.ArrayType;
+import org.apache.flink.table.types.logical.LogicalType;
+import org.apache.flink.table.types.logical.LogicalTypeRoot;
+import org.apache.flink.types.variant.Variant;
+
+import static org.apache.flink.table.planner.codegen.CodeGenUtils.newName;
+import static 
org.apache.flink.table.planner.functions.casting.CastRuleUtils.constructorCall;
+import static 
org.apache.flink.table.planner.functions.casting.CastRuleUtils.methodCall;
+import static 
org.apache.flink.table.planner.functions.casting.CastRuleUtils.newArray;
+import static 
org.apache.flink.table.planner.functions.casting.CastRuleUtils.staticCall;
+import static 
org.apache.flink.table.planner.functions.casting.CastRuleUtils.strLiteral;
+
+/**
+ * {@link LogicalTypeRoot#VARIANT} to {@link LogicalTypeRoot#ARRAY} cast rule.
+ *
+ * <p>The variant must be an array, otherwise the cast fails. Each element is 
itself a variant and
+ * casts to the target element type by the full {@code VARIANT}-to-element 
rule, recursively. An
+ * element that stores a JSON {@code null} maps to SQL {@code NULL} when the 
element type is
+ * nullable and fails the cast when it is {@code NOT NULL}. The exception is a 
{@code VARIANT}
+ * element type: there the element cast is the identity, so a JSON {@code 
null} element is kept as a
+ * variant null rather than downgraded to SQL {@code NULL}.
+ */
+class VariantToArrayCastRule extends 
AbstractVariantToConstructedCastRule<ArrayData> {
+
+    static final VariantToArrayCastRule INSTANCE = new 
VariantToArrayCastRule();
+
+    private VariantToArrayCastRule() {
+        super(
+                CastRulePredicate.builder()
+                        .predicate(
+                                (input, target) ->
+                                        input.is(LogicalTypeRoot.VARIANT)
+                                                && 
target.is(LogicalTypeRoot.ARRAY)
+                                                && CastRuleProvider.resolve(
+                                                                input,
+                                                                ((ArrayType) 
target)
+                                                                        
.getElementType())
+                                                        != null)
+                        .build());
+    }
+
+    /* Example generated code for ARRAY<INT>:
+
+    int arraySize$2 =
+            
org.apache.flink.table.runtime.functions.VariantCastUtils.arraySize(
+                    variant$1, "ARRAY<INT>");
+    java.lang.Integer[] objArray$3 = new java.lang.Integer[arraySize$2];
+    for (int i$4 = 0; i$4 < arraySize$2; i$4++) {
+        org.apache.flink.types.variant.Variant element$5 = 
variant$1.getElement(i$4);
+        if (!element$5.isNull()) {
+            result$6 =
+                    ((int) 
org.apache.flink.table.runtime.functions.VariantCastUtils.toIntegral(
+                            element$5, -2147483648L, 2147483647L, "INTEGER"));
+            objArray$3[i$4] = result$6;
+        }
+    }
+    result$0 = new org.apache.flink.table.data.GenericArrayData(objArray$3);
+
+    A JSON null element leaves the slot null (SQL NULL); a NOT NULL element 
type emits a throw instead.
+
+    */
+    @Override
+    protected String generateCodeBlockInternal(
+            CodeGeneratorCastRule.Context context,
+            String inputTerm,
+            String returnVariable,
+            LogicalType inputLogicalType,
+            LogicalType targetLogicalType) {
+        final LogicalType elementType = ((ArrayType) 
targetLogicalType).getElementType();
+        final String elementTypeTerm = arrayElementType(elementType);
+        final String sizeTerm = newName(context.getCodeGeneratorContext(), 
"arraySize");
+        final String arrayTerm = newName(context.getCodeGeneratorContext(), 
"objArray");
+        final String elementTerm = newName(context.getCodeGeneratorContext(), 
"element");
+
+        // For a typed element the JSON null is handled in the loop below, so 
the inner cast is the

Review Comment:
   ```suggestion
           // For a typed element the VARIANT null is handled in the loop 
below, so the inner cast is the
   ```



##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/CastFunctionITCase.java:
##########
@@ -379,6 +384,116 @@ private static List<TestSetSpec> variantCasts() {
                                 TINYINT()));
     }
 
+    private static List<TestSetSpec> variantArrayCasts() {
+        return List.of(
+                TestSetSpec.forExpression("Cast a VARIANT produced by 
PARSE_JSON to an ARRAY")
+                        .onFieldsWithData("unused")
+                        .andDataTypes(STRING())
+                        // ARRAY: each element casts by the same 
VARIANT-to-element rule.
+                        .testResult(
+                                call("PARSE_JSON", "[1, 2, 
3]").cast(ARRAY(INT())),
+                                "CAST(PARSE_JSON('[1, 2, 3]') AS ARRAY<INT>)",
+                                new Integer[] {1, 2, 3},
+                                ARRAY(INT()).notNull())
+                        // an approximate leaf takes any numeric kind
+                        .testResult(
+                                call("PARSE_JSON", "[1, 2, 
3]").cast(ARRAY(DOUBLE())),
+                                "CAST(PARSE_JSON('[1, 2, 3]') AS 
ARRAY<DOUBLE>)",
+                                new Double[] {1.0, 2.0, 3.0},
+                                ARRAY(DOUBLE()).notNull())
+                        // each element renders to string like the scalar cast
+                        .testResult(
+                                call("PARSE_JSON", "[1, 2, 
3]").cast(ARRAY(STRING())),
+                                "CAST(PARSE_JSON('[1, 2, 3]') AS 
ARRAY<STRING>)",
+                                new String[] {"1", "2", "3"},
+                                ARRAY(STRING()).notNull())
+                        // a heterogeneous array renders every element to 
string
+                        .testResult(
+                                call("PARSE_JSON", "[1, \"a\", 2, 
\"b\"]").cast(ARRAY(STRING())),
+                                "CAST(PARSE_JSON('[1, \"a\", 2, \"b\"]') AS 
ARRAY<STRING>)",
+                                new String[] {"1", "a", "2", "b"},
+                                ARRAY(STRING()).notNull())
+                        .testResult(
+                                call("PARSE_JSON", "[]").cast(ARRAY(INT())),
+                                "CAST(PARSE_JSON('[]') AS ARRAY<INT>)",
+                                new Integer[] {},
+                                ARRAY(INT()).notNull())
+                        // a JSON null element maps to SQL NULL for a nullable 
element type
+                        .testResult(
+                                call("PARSE_JSON", "[1, null, 
3]").cast(ARRAY(INT())),
+                                "CAST(PARSE_JSON('[1, null, 3]') AS 
ARRAY<INT>)",
+                                new Integer[] {1, null, 3},
+                                ARRAY(INT()).notNull())
+                        // a JSON null element fails a NOT NULL element type
+                        .testTableApiRuntimeError(
+                                call("PARSE_JSON", "[1, null, 
3]").cast(ARRAY(INT().notNull())),

Review Comment:
   is there no "Table API" way of parsing JSON? Can we avoid `call()`?



##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/casting/CastRulesTest.java:
##########
@@ -167,27 +168,83 @@ class CastRulesTest {
     /** U+1D54F, one code point but two UTF-16 units and four UTF-8 bytes. */
     private static final String NON_BMP = "𝕏";
 
+    private static final VariantBuilder VARIANT_BUILDER = Variant.newBuilder();
     private static final Variant VARIANT_ARRAY =
-            Variant.newBuilder()
+            VARIANT_BUILDER
                     .array()
-                    .add(Variant.newBuilder().of(1))
-                    .add(Variant.newBuilder().of("two"))
-                    .add(Variant.newBuilder().of(false))
-                    .add(Variant.newBuilder().ofNull())
+                    .add(VARIANT_BUILDER.of(1))
+                    .add(VARIANT_BUILDER.of("two"))
+                    .add(VARIANT_BUILDER.of(false))
+                    .add(VARIANT_BUILDER.ofNull())
                     .build();
 
     private static final Variant VARIANT_OBJECT =
-            Variant.newBuilder()
+            VARIANT_BUILDER
                     .object()
                     .add(
                             "k",
-                            Variant.newBuilder()
+                            VARIANT_BUILDER
                                     .array()
-                                    .add(Variant.newBuilder().of(1))
-                                    .add(Variant.newBuilder().of(2))
+                                    .add(VARIANT_BUILDER.of(1))
+                                    .add(VARIANT_BUILDER.of(2))
                                     .build())
                     .build();
 
+    /** {@code [1, 2, 3]}, the design's running array value. */
+    private static final Variant VARIANT_INT_ARRAY =
+            VARIANT_BUILDER
+                    .array()
+                    .add(VARIANT_BUILDER.of(1))
+                    .add(VARIANT_BUILDER.of(2))
+                    .add(VARIANT_BUILDER.of(3))
+                    .build();
+
+    /** {@code [1, null, 3]}, an array carrying a JSON null element. */

Review Comment:
   ```suggestion
       /** {@code [1, null, 3]}, an array carrying a VARIANT null element. */
   ```



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