yunfengzhou-hub commented on code in PR #27330:
URL: https://github.com/apache/flink/pull/27330#discussion_r2719475541
##########
flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/codegen/CodeGenUtils.scala:
##########
@@ -43,7 +43,7 @@ import
org.apache.flink.table.types.logical.utils.LogicalTypeUtils.toInternalCon
import org.apache.flink.table.types.utils.DataTypeUtils.isInternal
import org.apache.flink.table.utils.EncodingUtils
import org.apache.flink.types.{ColumnList, Row, RowKind}
-import org.apache.flink.types.variant.Variant
+import org.apache.flink.types.variant.{BinaryVariant, Variant}
Review Comment:
The `BinaryVariant` seems unused and can be removed.
##########
docs/data/sql_functions.yml:
##########
@@ -1167,6 +1167,16 @@ variant:
parser will keep the last occurrence of all fields with the same key,
otherwise when
`allowDuplicateKeys` is false it will throw an error. The default value
of
`allowDuplicateKeys` is false.
+ - sql: variant '[' INT ']'
+ table: VARIANT.at(INT)
+ description: |
+ If the VARIANT is an ARRAY value, returns a VARIANT whose value is the
element at
+ the specified index. Otherwise, NULL is returned.
+ - sql: variant '[' STRING ']'
Review Comment:
Apart from `variant ‘[’ INT ‘]’` and `variant ‘[’ STRING ‘]’`, FLIP-521 also
mentioned `variant.field` as a built-in function to access field or element in
the Variant. I'm also OK if we wants to add support for this function later in
a separated PR.
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/inference/strategies/ItemAtIndexArgumentTypeStrategy.java:
##########
@@ -86,12 +89,36 @@ public Optional<DataType> inferArgumentType(
}
}
+ if (collectionType.is(LogicalTypeRoot.VARIANT)) {
+ if
(indexType.getLogicalType().is(LogicalTypeFamily.INTEGER_NUMERIC)) {
+
+ if (callContext.isArgumentLiteral(1)) {
+ Optional<Integer> literalVal =
callContext.getArgumentValue(1, Integer.class);
+ if (literalVal.isPresent() && literalVal.get() <= 0) {
+ return callContext.fail(
+ throwOnFailure,
+ "The provided index must be a valid SQL index
starting from 1, but was '%s'",
+ literalVal.get());
+ }
+ }
+
+ return Optional.of(indexType);
+ } else if
(indexType.getLogicalType().is(LogicalTypeFamily.CHARACTER_STRING)) {
Review Comment:
Would the following be better in aspect of extensibility?
```java
if (LogicalTypeCasts.supportsImplicitCast(indexType.getLogicalType(), new
VarCharType())) {
```
--
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]
