twalthr commented on a change in pull request #17450:
URL: https://github.com/apache/flink/pull/17450#discussion_r727097560
##########
File path:
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/logical/utils/LogicalTypeCasts.java
##########
@@ -319,7 +322,8 @@ private static boolean supportsCasting(
} else if (sourceRoot == STRUCTURED_TYPE || targetRoot ==
STRUCTURED_TYPE) {
return supportsStructuredCasting(
sourceType, targetType, (s, t) -> supportsCasting(s, t,
allowExplicit));
- } else if (sourceRoot == RAW || targetRoot == RAW) {
+ } else if ((sourceRoot == RAW &&
!targetRoot.getFamilies().contains(BINARY_STRING))
Review comment:
very nit: We use `hasFamily` for better code readability. Can we rewrite
this to `(sourceRoot == RAW && !hasFamily(targetType, BINARY_STRING)`?
##########
File path:
flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/LogicalTypeCastsTest.java
##########
@@ -253,7 +256,15 @@
.build(),
false,
true
- }
+ },
+
+ // raw to binary
+ {
+ new RawType(Integer.class, new
ValueSerializer(Integer.class)),
Review comment:
nit: use `IntSeriaizer.INSTANCE`? should be more stable once we remove
ValueSerializer
##########
File path:
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/logical/utils/LogicalTypeCasts.java
##########
@@ -401,6 +405,8 @@ private static boolean supportsConstructedCasting(
}
}
return true;
+ } else if (sourceRoot == ARRAY && (targetRoot == CHAR || targetRoot ==
VARCHAR)) {
Review comment:
why this change?
##########
File path:
flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/codegen/calls/ScalarOperatorGens.scala
##########
@@ -40,15 +40,14 @@ import
org.apache.flink.table.types.logical.utils.LogicalTypeCasts.supportsExpli
import
org.apache.flink.table.types.logical.utils.LogicalTypeChecks.getFieldTypes
import
org.apache.flink.table.types.logical.utils.LogicalTypeMerging.findCommonType
import org.apache.flink.util.Preconditions.checkArgument
-
import org.apache.calcite.avatica.util.DateTimeUtils.MILLIS_PER_DAY
import org.apache.calcite.avatica.util.{DateTimeUtils, TimeUnitRange}
import org.apache.calcite.util.BuiltInMethod
+import org.apache.flink.table.api.DataTypes.BYTES
import java.lang.{StringBuilder => JStringBuilder}
import java.nio.charset.StandardCharsets
import java.util.Arrays.asList
-
Review comment:
make sure that you have configured your imports correctly, Scala needs
special settings. but it should be the same order as in our code style guides.
##########
File path:
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/CastFunctionMiscITCase.java
##########
@@ -26,9 +26,20 @@
import org.junit.runners.Parameterized;
+import java.nio.ByteBuffer;
import java.util.Arrays;
import java.util.List;
+import static org.apache.flink.table.api.DataTypes.BIGINT;
+import static org.apache.flink.table.api.DataTypes.BINARY;
+import static org.apache.flink.table.api.DataTypes.BOOLEAN;
+import static org.apache.flink.table.api.DataTypes.BYTES;
+import static org.apache.flink.table.api.DataTypes.FIELD;
+import static org.apache.flink.table.api.DataTypes.INT;
+import static org.apache.flink.table.api.DataTypes.ROW;
+import static org.apache.flink.table.api.DataTypes.STRING;
+import static org.apache.flink.table.api.DataTypes.VARBINARY;
+import static org.apache.flink.table.api.DataTypes.of;
Review comment:
nit: static imports of functions that are called `of` don't contribute
to code readability, I would not import it, other types are fine
--
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]
