rionmonster commented on code in PR #28437:
URL: https://github.com/apache/flink/pull/28437#discussion_r3430511431
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/inference/TypeInferenceUtil.java:
##########
@@ -155,11 +158,45 @@ private static CallContext castArgumentsInternal(
"Invalid argument type at position %d. Data
type %s expected but %s passed.",
pos, expectedType, actualType));
}
+
+ // Beyond type-level castability, a constant literal must also fit
the expected type.
+ if (throwOnInferInputFailure) {
+ validateLiteralFitsExpectedType(callContext, pos,
expectedType);
+ }
}
return castCallContext;
}
+ /**
+ * Validates that a constant literal argument fits the expected type on
the value level and not
+ * only on the type level. For example, {@code 123.456} is implicitly
castable to a {@code
+ * DECIMAL(2, 2)} but would otherwise be silently reduced to {@code NULL}
during constant
+ * folding instead of raising a type error.
+ */
+ private static void validateLiteralFitsExpectedType(
+ CallContext callContext, int pos, DataType expectedType) {
+ if (!expectedType.getLogicalType().is(LogicalTypeRoot.DECIMAL)
Review Comment:
Good call, thanks for pointing that out @weiqingy!
Yes, decimal-only was the intended scope here. FLINK-39122 specifically
covers the PTF case where an overflowing BigDecimal constant for a DECIMAL(p,
s) argument gets reduced to NULL during constant folding instead of failing
validation.
I agree that the current method name and related docs are probably too
general in this case. It's probably worth scoping it specifically to decimal
literals for now, which could be later replaced for any other value-level
overflows or logical types. I'll go ahead and make those changes.
--
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]
