twalthr commented on code in PR #29092:
URL: https://github.com/apache/flink/pull/29092#discussion_r3934354114
##########
docs/content.zh/docs/sql/reference/data-types.md:
##########
@@ -1614,6 +1614,33 @@ CAST(m AS ARRAY<STRING>) -- ['1', 'a'], a
heterogeneous array still renders ea
CAST(m AS ARRAY<VARIANT>) -- [1, "a"] as variants, one level shredded
```
+A variant object casts to `ROW` or `STRUCTURED`, which likewise imposes a
schema on it. The variant
+must be an object, otherwise the cast fails. Each field is itself a `VARIANT`,
so it casts to its
+declared type by the same rules, recursively. Fields match by name rather than
by position, since a
+JSON object is unordered, and name matching is case sensitive. A `ROW`
declared without field names
Review Comment:
```suggestion
declared type by the same rules, recursively. Fields match by name and name
matching is case sensitive. A `ROW` declared without field names
```
##########
docs/content.zh/docs/sql/reference/data-types.md:
##########
@@ -1614,6 +1614,33 @@ CAST(m AS ARRAY<STRING>) -- ['1', 'a'], a
heterogeneous array still renders ea
CAST(m AS ARRAY<VARIANT>) -- [1, "a"] as variants, one level shredded
```
+A variant object casts to `ROW` or `STRUCTURED`, which likewise imposes a
schema on it. The variant
+must be an object, otherwise the cast fails. Each field is itself a `VARIANT`,
so it casts to its
+declared type by the same rules, recursively. Fields match by name rather than
by position, since a
+JSON object is unordered, and name matching is case sensitive. A `ROW`
declared without field names
+uses the default names `f0`, `f1`, and so on, which must then be present in
the object.
+
+- A field absent from the object fails the cast, whether the target field is
nullable or not.
+- A field present but set to a variant null maps to SQL `NULL` when the field
is nullable and fails
+ the cast when it is `NOT NULL`.
Review Comment:
```suggestion
the cast when the target is `NOT NULL`.
```
##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/LogicalTypeCastsTest.java:
##########
@@ -307,6 +308,23 @@ private static Stream<Arguments> testData() {
YearMonthIntervalType.YearMonthResolution.MONTH)),
false,
false),
+ // A variant object casts to ROW or STRUCTURED when every
field is castable; an
+ // empty
+ // row is vacuously castable and matching is by name
+ Arguments.of(new VariantType(), new RowType(List.of()), false,
true),
+ Arguments.of(
+ new VariantType(),
+ new RowType(
+ List.of(
+ new RowField("f0", new IntType()),
+ new RowField("f1",
VarCharType.STRING_TYPE))),
+ false,
+ true),
+ Arguments.of(
+ new VariantType(),
+ new RowType(List.of(new RowField("f0", new
TimeType()))),
+ false,
+ false),
Review Comment:
Isn't this about to fail soon? Rather avoid to risk a red master.
##########
docs/content.zh/docs/sql/reference/data-types.md:
##########
@@ -1614,6 +1614,33 @@ CAST(m AS ARRAY<STRING>) -- ['1', 'a'], a
heterogeneous array still renders ea
CAST(m AS ARRAY<VARIANT>) -- [1, "a"] as variants, one level shredded
```
+A variant object casts to `ROW` or `STRUCTURED`, which likewise imposes a
schema on it. The variant
+must be an object, otherwise the cast fails. Each field is itself a `VARIANT`,
so it casts to its
+declared type by the same rules, recursively. Fields match by name rather than
by position, since a
+JSON object is unordered, and name matching is case sensitive. A `ROW`
declared without field names
+uses the default names `f0`, `f1`, and so on, which must then be present in
the object.
+
+- A field absent from the object fails the cast, whether the target field is
nullable or not.
+- A field present but set to a variant null maps to SQL `NULL` when the field
is nullable and fails
+ the cast when it is `NOT NULL`.
+- Object fields the target does not name are dropped, so the row is a
projection.
+- A `ROW` or `STRUCTURED` whose fields are `VARIANT` is the identity on those
fields: it shreds one
+ level and keeps the rest semi-structured. A field set to a variant null
stays a variant null
+ rather than becoming SQL `NULL`.
+
+If any field cast fails, the whole cast fails, and `TRY_CAST` returns `NULL`
for the entire value
+rather than a partial result, exactly as for an array target.
Review Comment:
```suggestion
rather than a partial result.
```
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/casting/AbstractVariantToConstructedCastRule.java:
##########
@@ -30,9 +30,9 @@
* 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.
+ * <p>A constructed cast can always fail, on a shape mismatch, an unreadable
leaf, a missing field,
+ * or a JSON {@code null} in a {@code NOT NULL} position, so {@code TRY_CAST}
wraps the whole value
Review Comment:
```suggestion
* or a VARIANT {@code null} in a {@code NOT NULL} position, so {@code
TRY_CAST} wraps the whole value
```
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/casting/CastRulesTest.java:
##########
@@ -267,6 +333,16 @@ class CastRulesTest {
"d",
ARRAY(STRING()).getLogicalType())))
.build());
+ // The variant a VARIANT row field round-trips to. A ROW cast serializes
each VARIANT field via
+ // BinaryRowWriter.writeVariant, which stores the field's sliced value and
the object metadata
+ // but
+ // drops the position, so the read-back variant equals neither the
original field view (non-zero
+ // pos) nor a freshly built scalar (empty metadata). This rebuilds that
exact form at pos 0.
Review Comment:
remove this comment? You just create a deep copy right?
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/casting/VariantToRowCastRule.java:
##########
@@ -0,0 +1,234 @@
+/*
+ * 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.api.TableRuntimeException;
+import org.apache.flink.table.data.RowData;
+import org.apache.flink.table.data.binary.BinaryRowData;
+import org.apache.flink.table.data.writer.BinaryRowWriter;
+import org.apache.flink.table.planner.codegen.CodeGeneratorContext;
+import org.apache.flink.table.runtime.functions.VariantCastUtils;
+import org.apache.flink.table.types.logical.LogicalType;
+import org.apache.flink.table.types.logical.LogicalTypeRoot;
+import org.apache.flink.table.types.logical.utils.LogicalTypeChecks;
+import org.apache.flink.types.variant.Variant;
+
+import java.util.List;
+
+import static org.apache.flink.table.planner.codegen.CodeGenUtils.className;
+import static org.apache.flink.table.planner.codegen.CodeGenUtils.newName;
+import static
org.apache.flink.table.planner.functions.casting.CastRuleUtils.binaryWriterWriteField;
+import static
org.apache.flink.table.planner.functions.casting.CastRuleUtils.binaryWriterWriteNull;
+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.staticCall;
+import static
org.apache.flink.table.planner.functions.casting.CastRuleUtils.strLiteral;
+
+/**
+ * {@link LogicalTypeRoot#VARIANT} to {@link LogicalTypeRoot#ROW} and {@link
+ * LogicalTypeRoot#STRUCTURED_TYPE} cast rule.
+ *
+ * <p>The variant must be an object, otherwise the cast fails. Fields match
<b>by name</b> rather
+ * than by position, because a JSON object is unordered. Name matching is case
sensitive. A target
Review Comment:
```suggestion
* than by position, because a VARIANT object is unordered. Name matching is
case sensitive. A target
```
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/casting/CastRuleProviderTest.java:
##########
@@ -157,4 +157,17 @@ void testResolveVariantToArray() {
// MULTISET has no variant counterpart
assertThat(CastRuleProvider.exists(VARIANT,
MULTISET(STRING()).getLogicalType())).isFalse();
}
+
+ @Test
+ void testResolveVariantToRow() {
+ assertThat(CastRuleProvider.resolve(VARIANT, ROW(FIELD("f0",
INT())).getLogicalType()))
+ .isSameAs(VariantToRowCastRule.INSTANCE);
+ // a structured target shares the ROW rule
+ assertThat(CastRuleProvider.resolve(VARIANT, STRUCTURED))
+ .isSameAs(VariantToRowCastRule.INSTANCE);
+
+ // a field with no variant counterpart makes the whole cast
unresolvable
+ assertThat(CastRuleProvider.exists(VARIANT, ROW(FIELD("f0",
TIME())).getLogicalType()))
Review Comment:
same comment as above
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/casting/CastRulesTest.java:
##########
@@ -1991,7 +2067,99 @@ Stream<CastTestSpecBuilder> testCases() {
VARIANT_INT_ARRAY_WITH_NULL.getElement(0),
VARIANT_INT_ARRAY_WITH_NULL.getElement(1),
VARIANT_INT_ARRAY_WITH_NULL.getElement(2)
- })));
+ })),
+ CastTestSpecBuilder.testCastTo(ROW(FIELD("id", INT()),
FIELD("name", STRING())))
+ .fromCase(VARIANT(), null, null)
+ .fromCase(
+ VARIANT(), VARIANT_RECORD,
GenericRowData.of(7, fromString("ada")))
+ // an array or a scalar is not an object
+ .fail(VARIANT(), VARIANT_INT_ARRAY,
TableRuntimeException.class),
Review Comment:
add expected error message substring
--
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]
