twalthr commented on a change in pull request #17579:
URL: https://github.com/apache/flink/pull/17579#discussion_r738258450
##########
File path:
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/CastFunctionITCase.java
##########
@@ -191,149 +210,159 @@ protected Configuration configuration() {
.failTableApi(FLOAT(), -123.456)
.failTableApi(DOUBLE(), 0)
// Not supported - no fix
- // DATE
- // TIME
- // TIMESTAMP
+ .fail(DATE(), LocalDate.of(2021, 10, 26))
+ .fail(TIME(), LocalTime.parse("12:34:56.123"))
+ .fail(TIMESTAMP(),
LocalDateTime.parse("2021-09-24T12:34:56.123456"))
// TIMESTAMP_WITH_TIME_ZONE
- // TIMESTAMP_LTZ
- // INTERVAL_YEAR_MONTH
- // INTERVAL_DAY_TIME
- // ARRAY
+ .fail(TIMESTAMP_LTZ(),
fromLocalTZ("2021-09-24T18:34:56.123456"))
+ .fail(INTERVAL(YEAR(), MONTH()), Period.of(10, 4, 0))
+ .fail(INTERVAL(DAY(), SECOND()), Duration.ofHours(12))
+ .fail(ARRAY(INT()), new int[] {0, 1, 2})
// MULTISET
// MAP
// ROW
// RAW
.build(),
CastTestSpecBuilder.testCastTo(BINARY(2))
+ .fromCase(BINARY(5), null, null)
.fromCase(CHAR(3), "foo", new byte[] {102, 111, 111})
.fromCase(VARCHAR(5), "Flink", new byte[] {70, 108,
105, 110, 107})
// https://issues.apache.org/jira/browse/FLINK-24419 -
not trimmed to 2
// bytes
.fromCase(STRING(), "Apache", new byte[] {65, 112, 97,
99, 104, 101})
// Not supported - no fix
- // BOOLEAN
+ .fail(BOOLEAN(), randomBoolean())
+ //
.fromCase(BINARY(2), new byte[] {0, 1}, new byte[] {0,
1})
.fromCase(VARBINARY(3), new byte[] {0, 1, 2}, new
byte[] {0, 1, 2})
.fromCase(BYTES(), new byte[] {0, 1, 2, 3, 4}, new
byte[] {0, 1, 2, 3, 4})
// Not supported - no fix
- // DECIMAL
- // TINYINT
- // SMALLINT
- // INT
- // BIGINT
- // FLOAT
- // DOUBLE
- // DATE
- // TIME
- // TIMESTAMP
+ .fail(DECIMAL(5, 3), 12.345)
+ .fail(TINYINT(), (byte) 5)
+ .fail(SMALLINT(), (short) 12)
+ .fail(INT(), 122345)
+ .fail(BIGINT(), 12345678912L)
+ .fail(FLOAT(), 1.2345)
+ .fail(DOUBLE(), 1.23456789)
+ .fail(DATE(), LocalDate.of(2021, 10, 26))
+ .fail(TIME(), LocalTime.parse("12:34:56.123"))
+ .fail(TIMESTAMP(),
LocalDateTime.parse("2021-09-24T12:34:56.123456"))
// TIMESTAMP_WITH_TIME_ZONE
- // TIMESTAMP_LTZ
- // INTERVAL_YEAR_MONTH
- // INTERVAL_DAY_TIME
- // ARRAY
+ .fail(TIMESTAMP_LTZ(),
fromLocalTZ("2021-09-24T18:34:56.123456"))
+ .fail(INTERVAL(YEAR(), MONTH()), Period.of(10, 4, 0))
+ .fail(INTERVAL(DAY(), SECOND()), Duration.ofHours(12))
+ .fail(ARRAY(INT()), new int[] {0, 1, 2})
Review comment:
btw have you thought about introducing a default value for every type in
this test. I think reading `DEFAULT_INT` or `DEFAULT_ARRAY_INT` would improve
the readability. One can still add non default values for specific tests that
test e.g. boundaries.
##########
File path:
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/CastFunctionITCase.java
##########
@@ -930,16 +1066,44 @@ private TestSpec build() {
int idxOffset = 0;
for (int i = 0; i < columnData.size(); i++) {
String colName = "f" + i;
+ LogicalType colType = columnTypes.get(i).getLogicalType();
+ String errorMsg;
switch (testTypes.get(i)) {
case ERROR_TABLE_API:
- testSpec.testTableApiValidationError(
- $(colName).cast(targetType),
- String.format(
- "Invalid function call:%ncast("
- + columnTypes.get(i).toString()
- + ", "
- + targetType.toString()
- + ")"));
+ if (isTimestampLtzToNumeric(colType,
targetType.getLogicalType())) {
Review comment:
maybe move those in a dedicated method for `specific errors` or `hint
errors`?
##########
File path:
flink-table/flink-table-planner/src/test/scala/org/apache/flink/table/planner/expressions/ScalarOperatorsTest.scala
##########
@@ -144,14 +144,10 @@ class ScalarOperatorsTest extends ScalarOperatorsTestBase
{
testSqlApi("CASE WHEN 'a'='a' THEN 1 END", "1")
testSqlApi("CASE 2 WHEN 1 THEN 'a' WHEN 2 THEN 'bcd' END", "bcd")
testSqlApi("CASE 1 WHEN 1 THEN 'a' WHEN 2 THEN 'bcd' END", "a")
- testSqlApi("CASE 1 WHEN 1 THEN CAST ('a' as varchar(1)) WHEN 2 THEN " +
Review comment:
I would still keep this. As we had issue with
`org.apache.calcite.rel.type.RelDataTypeSystem#shouldConvertRaggedUnionTypesToVarying`
##########
File path:
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/CastFunctionITCase.java
##########
@@ -966,4 +1130,21 @@ private static Instant fromLocalToUTC(LocalDateTime
localDateTime) {
private static Instant fromLocalTZ(String str) {
return LocalDateTime.parse(str).toInstant(TEST_OFFSET);
}
+
+ private static boolean randomBoolean() {
Review comment:
randomness in tests is never a good idea
##########
File path:
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/logical/utils/LogicalTypeCasts.java
##########
@@ -184,17 +184,17 @@
castTo(DATE)
.implicitFrom(DATE, TIMESTAMP_WITHOUT_TIME_ZONE)
- .explicitFromFamily(TIMESTAMP, CHARACTER_STRING, BINARY_STRING)
Review comment:
can we put such change in `table-common` into separate commits. this
change should be visible in the log afterwards.
##########
File path:
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/MiscFunctionsITCase.java
##########
@@ -46,7 +46,9 @@
"INT NOT NULL",
DataTypes.STRING())
.testTableApiValidationError(
- call("TYPEOF", $("f0"), $("f2")), "Invalid
input arguments.")
+ call("TYPEOF", $("f0"), $("f2")),
Review comment:
hotfix commit? not really related to the JIRA issue
--
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: issues-unsubscr...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
