twalthr commented on code in PR #27603:
URL: https://github.com/apache/flink/pull/27603#discussion_r2815708422
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/logical/utils/LogicalTypeCasts.java:
##########
@@ -110,124 +110,186 @@ public final class LogicalTypeCasts {
private static final Map<LogicalTypeRoot, Set<LogicalTypeRoot>>
explicitCastingRules;
+ private static final Map<LogicalTypeRoot, Set<LogicalTypeRoot>>
injectiveCastingRules;
+
+ // Types with deterministic, unique string representations (for injective
casts to STRING)
+ private static final LogicalTypeRoot[] STRING_INJECTIVE_SOURCES = {
+ TINYINT,
+ SMALLINT,
+ INTEGER,
+ BIGINT,
+ FLOAT,
+ DOUBLE,
+ BOOLEAN,
+ DATE,
Review Comment:
TIME_WITHOUT_TIME_ZONE?
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/logical/utils/LogicalTypeCasts.java:
##########
@@ -110,124 +110,186 @@ public final class LogicalTypeCasts {
private static final Map<LogicalTypeRoot, Set<LogicalTypeRoot>>
explicitCastingRules;
+ private static final Map<LogicalTypeRoot, Set<LogicalTypeRoot>>
injectiveCastingRules;
+
+ // Types with deterministic, unique string representations (for injective
casts to STRING)
+ private static final LogicalTypeRoot[] STRING_INJECTIVE_SOURCES = {
+ TINYINT,
+ SMALLINT,
+ INTEGER,
+ BIGINT,
+ FLOAT,
+ DOUBLE,
+ BOOLEAN,
+ DATE,
+ TIMESTAMP_WITHOUT_TIME_ZONE,
+ TIMESTAMP_WITH_TIME_ZONE,
Review Comment:
```suggestion
```
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/logical/utils/LogicalTypeCasts.java:
##########
@@ -110,124 +110,186 @@ public final class LogicalTypeCasts {
private static final Map<LogicalTypeRoot, Set<LogicalTypeRoot>>
explicitCastingRules;
+ private static final Map<LogicalTypeRoot, Set<LogicalTypeRoot>>
injectiveCastingRules;
+
+ // Types with deterministic, unique string representations (for injective
casts to STRING)
+ private static final LogicalTypeRoot[] STRING_INJECTIVE_SOURCES = {
+ TINYINT,
+ SMALLINT,
+ INTEGER,
+ BIGINT,
+ FLOAT,
+ DOUBLE,
+ BOOLEAN,
+ DATE,
+ TIMESTAMP_WITHOUT_TIME_ZONE,
+ TIMESTAMP_WITH_TIME_ZONE,
+ TIMESTAMP_WITH_LOCAL_TIME_ZONE
+ };
+
static {
implicitCastingRules = new HashMap<>();
explicitCastingRules = new HashMap<>();
+ injectiveCastingRules = new HashMap<>();
- // identity casts
-
+ // Identity casts: all types can be cast to themselves, and identity
is always injective
for (LogicalTypeRoot typeRoot : allTypes()) {
- castTo(typeRoot).implicitFrom(typeRoot).build();
+
castTo(typeRoot).implicitFrom(typeRoot).injectiveFrom(typeRoot).build();
}
- // cast specification
+ //
-----------------------------------------------------------------------------------------
+ // Character string types
+ //
-----------------------------------------------------------------------------------------
castTo(CHAR)
.implicitFrom(CHAR)
.explicitFromFamily(PREDEFINED, CONSTRUCTED)
.explicitFrom(RAW, NULL, STRUCTURED_TYPE)
+ .injectiveFrom(CHAR)
+ .injectiveFrom(STRING_INJECTIVE_SOURCES)
.build();
castTo(VARCHAR)
.implicitFromFamily(CHARACTER_STRING)
.explicitFromFamily(PREDEFINED, CONSTRUCTED)
.explicitFrom(RAW, NULL, STRUCTURED_TYPE)
+ .injectiveFrom(CHAR, VARCHAR)
+ .injectiveFrom(STRING_INJECTIVE_SOURCES)
.build();
- castTo(BOOLEAN)
- .implicitFrom(BOOLEAN)
- .explicitFromFamily(CHARACTER_STRING, INTEGER_NUMERIC)
- .build();
+ //
-----------------------------------------------------------------------------------------
+ // Binary string types
+ //
-----------------------------------------------------------------------------------------
castTo(BINARY)
.implicitFrom(BINARY)
.explicitFromFamily(CHARACTER_STRING)
- .explicitFrom(VARBINARY)
- .explicitFrom(RAW)
+ .explicitFrom(VARBINARY, RAW)
+ .injectiveFrom(BINARY)
.build();
castTo(VARBINARY)
.implicitFromFamily(BINARY_STRING)
.explicitFromFamily(CHARACTER_STRING)
- .explicitFrom(BINARY)
- .explicitFrom(RAW)
+ .explicitFrom(BINARY, RAW)
+ .injectiveFrom(BINARY, VARBINARY)
.build();
- castTo(DECIMAL)
- .implicitFromFamily(NUMERIC)
- .explicitFromFamily(CHARACTER_STRING, INTERVAL)
- .explicitFrom(BOOLEAN, TIMESTAMP_WITHOUT_TIME_ZONE,
TIMESTAMP_WITH_LOCAL_TIME_ZONE)
- .build();
+ //
-----------------------------------------------------------------------------------------
+ // Exact numeric types
+ //
-----------------------------------------------------------------------------------------
castTo(TINYINT)
.implicitFrom(TINYINT)
.explicitFromFamily(NUMERIC, CHARACTER_STRING, INTERVAL)
.explicitFrom(BOOLEAN, TIMESTAMP_WITHOUT_TIME_ZONE,
TIMESTAMP_WITH_LOCAL_TIME_ZONE)
+ .injectiveFrom(TINYINT)
.build();
castTo(SMALLINT)
.implicitFrom(TINYINT, SMALLINT)
.explicitFromFamily(NUMERIC, CHARACTER_STRING, INTERVAL)
.explicitFrom(BOOLEAN, TIMESTAMP_WITHOUT_TIME_ZONE,
TIMESTAMP_WITH_LOCAL_TIME_ZONE)
+ .injectiveFrom(TINYINT, SMALLINT)
.build();
castTo(INTEGER)
.implicitFrom(TINYINT, SMALLINT, INTEGER)
.explicitFromFamily(NUMERIC, CHARACTER_STRING, INTERVAL)
.explicitFrom(BOOLEAN, TIMESTAMP_WITHOUT_TIME_ZONE,
TIMESTAMP_WITH_LOCAL_TIME_ZONE)
+ .injectiveFrom(TINYINT, SMALLINT, INTEGER)
.build();
castTo(BIGINT)
.implicitFrom(TINYINT, SMALLINT, INTEGER, BIGINT)
.explicitFromFamily(NUMERIC, CHARACTER_STRING, INTERVAL)
.explicitFrom(BOOLEAN, TIMESTAMP_WITHOUT_TIME_ZONE,
TIMESTAMP_WITH_LOCAL_TIME_ZONE)
+ .injectiveFrom(TINYINT, SMALLINT, INTEGER, BIGINT)
+ .build();
+
+ castTo(DECIMAL)
+ .implicitFromFamily(NUMERIC)
+ .explicitFromFamily(CHARACTER_STRING, INTERVAL)
+ .explicitFrom(BOOLEAN, TIMESTAMP_WITHOUT_TIME_ZONE,
TIMESTAMP_WITH_LOCAL_TIME_ZONE)
+ .injectiveFrom(TINYINT, SMALLINT, INTEGER, BIGINT, DECIMAL)
Review Comment:
let's exclude DECIMAL for now. It depends on precision and scale whether
this works or if we lose information.
##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/LogicalTypeCastsTest.java:
##########
@@ -273,4 +279,140 @@ void test(
.as("Supports explicit casting")
.isEqualTo(supportsExplicit);
}
+
+ /**
+ * Test data for injective cast tests. Each argument contains:
(sourceType, targetType,
+ * expectedInjective).
+ */
+ private static Stream<Arguments> injectiveCastTestData() {
+ return Stream.of(
+ // Integer widenings are injective
+ Arguments.of(new SmallIntType(), new BigIntType(), true),
+ Arguments.of(new IntType(), new BigIntType(), true),
+ Arguments.of(new TinyIntType(), new IntType(), true),
+ Arguments.of(new TinyIntType(), new SmallIntType(), true),
+
+ // Explicit casts to STRING from integer types are injective
+ Arguments.of(new TinyIntType(), VarCharType.STRING_TYPE, true),
+ Arguments.of(new SmallIntType(), VarCharType.STRING_TYPE,
true),
+ Arguments.of(new IntType(), VarCharType.STRING_TYPE, true),
+ Arguments.of(new BigIntType(), VarCharType.STRING_TYPE, true),
+
+ // FLOAT/DOUBLE to STRING are injective
+ Arguments.of(new FloatType(), VarCharType.STRING_TYPE, true),
+ Arguments.of(new DoubleType(), VarCharType.STRING_TYPE, true),
+
+ // Explicit casts to STRING from boolean are injective
+ Arguments.of(new BooleanType(), VarCharType.STRING_TYPE, true),
+
+ // Explicit casts to STRING from date/time types are injective
+ Arguments.of(new DateType(), VarCharType.STRING_TYPE, true),
+ Arguments.of(new TimestampType(3), VarCharType.STRING_TYPE,
true),
+ Arguments.of(new TimestampType(9), VarCharType.STRING_TYPE,
true),
+ Arguments.of(new LocalZonedTimestampType(3),
VarCharType.STRING_TYPE, true),
+ Arguments.of(new ZonedTimestampType(3),
VarCharType.STRING_TYPE, true),
+
+ // Explicit casts to CHAR are also injective for the same
source types
+ Arguments.of(new IntType(), new CharType(100), true),
+ Arguments.of(new BigIntType(), new CharType(100), true),
+
+ // CHAR → VARCHAR widening is injective
+ Arguments.of(new CharType(10), VarCharType.STRING_TYPE, true),
+
+ // BINARY → VARBINARY widening is injective
+ Arguments.of(new BinaryType(10), new VarBinaryType(100), true),
+
+ // Integer → DECIMAL is injective (exact representation)
+ Arguments.of(new TinyIntType(), new DecimalType(10, 0), true),
+ Arguments.of(new SmallIntType(), new DecimalType(10, 0), true),
+ Arguments.of(new IntType(), new DecimalType(10, 0), true),
+ Arguments.of(new BigIntType(), new DecimalType(20, 0), true),
+
+ // DECIMAL → DECIMAL identity is injective
+ Arguments.of(new DecimalType(10, 2), new DecimalType(10, 2),
true),
+
+ // FLOAT → DOUBLE widening is injective
+ Arguments.of(new FloatType(), new DoubleType(), true),
+
+ // Narrowing casts are NOT injective (lossy)
+ Arguments.of(VarCharType.STRING_TYPE, new IntType(), false),
+ Arguments.of(new BigIntType(), new IntType(), false),
+ Arguments.of(new DoubleType(), new FloatType(), false),
+
+ // TIMESTAMP → DATE is NOT injective (loses time-of-day
information)
+ // even though it is an implicit cast
+ Arguments.of(new TimestampType(3), new DateType(), false),
+
+ // DECIMAL to STRING is NOT considered injective
+ Arguments.of(new DecimalType(10, 2), VarCharType.STRING_TYPE,
false),
+
+ // BYTES to STRING is NOT injective (invalid UTF-8 sequences
collapse)
+ Arguments.of(new VarBinaryType(100), VarCharType.STRING_TYPE,
false),
+ Arguments.of(new BinaryType(100), VarCharType.STRING_TYPE,
false),
+
+ // TIME to STRING is NOT injective (not in the whitelist)
+ Arguments.of(new TimeType(3), VarCharType.STRING_TYPE, false),
+
+ // INT → FLOAT/DOUBLE are NOT injective (even though implicit)
+ // because floating point is not in the injective rules
+ Arguments.of(new IntType(), new FloatType(), false),
+ Arguments.of(new IntType(), new DoubleType(), false),
Review Comment:
nit: in theory this should work? int (and smaller) fits into double. but I'm
fine if we just don't want to support it.
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/logical/utils/LogicalTypeCasts.java:
##########
@@ -110,124 +110,186 @@ public final class LogicalTypeCasts {
private static final Map<LogicalTypeRoot, Set<LogicalTypeRoot>>
explicitCastingRules;
+ private static final Map<LogicalTypeRoot, Set<LogicalTypeRoot>>
injectiveCastingRules;
+
+ // Types with deterministic, unique string representations (for injective
casts to STRING)
+ private static final LogicalTypeRoot[] STRING_INJECTIVE_SOURCES = {
+ TINYINT,
+ SMALLINT,
+ INTEGER,
+ BIGINT,
+ FLOAT,
+ DOUBLE,
+ BOOLEAN,
+ DATE,
+ TIMESTAMP_WITHOUT_TIME_ZONE,
+ TIMESTAMP_WITH_TIME_ZONE,
Review Comment:
just for safety, I could imagine two timestamps pointing to the same time
with different time zones set.
--
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]
