lvyanquan commented on code in PR #4492:
URL: https://github.com/apache/flink-cdc/pull/4492#discussion_r3711713084
##########
flink-cdc-runtime/src/main/java/org/apache/flink/cdc/runtime/functions/impl/LogicalFunctions.java:
##########
@@ -102,4 +104,45 @@ public static Object coalesce(Object... objects) {
}
return null;
}
+
+ public static <T> T ifNull(T value, T replacement) {
+ return value != null ? value : replacement;
+ }
+
+ public static <T> T nullIf(T value, Object comparison) {
+ return valuesEqualForNullIf(value, comparison) ? null : value;
+ }
+
+ private static boolean valuesEqualForNullIf(Object value, Object
comparison) {
+ if (value == null || comparison == null) {
+ return false;
+ }
+ if (!(value instanceof Number) || !(comparison instanceof Number)) {
+ return Objects.deepEquals(value, comparison);
+ }
+
+ Number left = (Number) value;
+ Number right = (Number) comparison;
+ if (left instanceof Double || right instanceof Double) {
+ double leftValue = left.doubleValue();
+ double rightValue = right.doubleValue();
+ if (Double.isFinite(leftValue) && Double.isFinite(rightValue)) {
+ return
BigDecimal.valueOf(leftValue).compareTo(BigDecimal.valueOf(rightValue)) == 0;
+ }
+ return Double.compare(leftValue, rightValue) == 0;
+ }
+ if (left instanceof Float || right instanceof Float) {
+ return Float.compare(left.floatValue(), right.floatValue()) == 0;
Review Comment:
Float.compare and Double.compare do not match SQL numeric equality
semantics. In particular, they treat -0.0 and 0.0 as different and treat two
NaN values as equal. As a result, NULLIF(-0.0, 0.0) incorrectly returns the
first value, while NULLIF(NaN, NaN) incorrectly returns NULL.
Please compare floating-point operands using primitive numeric equality
after coercing them to the common numeric type, rather than using Float.compare
or Double.compare.
Tests covering signed zero, NaN, and infinity should also be added.
##########
flink-cdc-runtime/src/main/java/org/apache/flink/cdc/runtime/functions/impl/LogicalFunctions.java:
##########
@@ -102,4 +104,45 @@ public static Object coalesce(Object... objects) {
}
return null;
}
+
+ public static <T> T ifNull(T value, T replacement) {
+ return value != null ? value : replacement;
+ }
+
+ public static <T> T nullIf(T value, Object comparison) {
+ return valuesEqualForNullIf(value, comparison) ? null : value;
+ }
+
+ private static boolean valuesEqualForNullIf(Object value, Object
comparison) {
+ if (value == null || comparison == null) {
+ return false;
+ }
+ if (!(value instanceof Number) || !(comparison instanceof Number)) {
+ return Objects.deepEquals(value, comparison);
+ }
+
+ Number left = (Number) value;
+ Number right = (Number) comparison;
+ if (left instanceof Double || right instanceof Double) {
+ double leftValue = left.doubleValue();
+ double rightValue = right.doubleValue();
+ if (Double.isFinite(leftValue) && Double.isFinite(rightValue)) {
+ return
BigDecimal.valueOf(leftValue).compareTo(BigDecimal.valueOf(rightValue)) == 0;
+ }
+ return Double.compare(leftValue, rightValue) == 0;
+ }
+ if (left instanceof Float || right instanceof Float) {
+ return Float.compare(left.floatValue(), right.floatValue()) == 0;
Review Comment:
The Float branch in `valuesEqualForNullIf` downgrades the other side via
`floatValue()`, so `nullIf(16_777_217L, 16_777_217f)` wrongly returns null —
beyond float's 24-bit mantissa (2²⁴), values get rounded and compared equal,
contradicting both SQL semantics (which promotes to DOUBLE) and the `=`
operator here. Since `float → double` is lossless, please promote to double
like the Double branch above does. A regression test for `NULLIF(BIGINT,
FLOAT)` with values beyond 2²⁴ would be appreciated.
##########
flink-cdc-runtime/src/main/java/org/apache/flink/cdc/runtime/functions/impl/CastingFunctions.java:
##########
@@ -264,7 +264,51 @@ public static BigDecimal castToBigDecimal(Object object,
int precision, int scal
}
public static LocalDateTime castToTimestamp(Object object, String
timezone) {
- ZoneId zoneId = ZoneId.of(timezone);
+ return castToTimestamp(object, ZoneId.of(timezone), false);
+ }
+
+ public static String tryCastToString(Object object) {
+ return castToString(object);
+ }
+
+ public static Boolean tryCastToBoolean(Object object) {
+ return castToBoolean(object);
+ }
+
+ public static Byte tryCastToByte(Object object) {
+ return castToByte(object);
+ }
+
+ public static Short tryCastToShort(Object object) {
+ return castToShort(object);
+ }
+
+ public static Integer tryCastToInteger(Object object) {
+ return castToInteger(object);
+ }
+
+ public static Long tryCastToLong(Object object) {
+ return castToLong(object);
+ }
+
+ public static Float tryCastToFloat(Object object) {
+ return castToFloat(object);
+ }
+
+ public static Double tryCastToDouble(Object object) {
+ return castToDouble(object);
+ }
+
+ public static BigDecimal tryCastToBigDecimal(Object object, int precision,
int scale) {
Review Comment:
Except for TIMESTAMP, all newly added tryCastToXxx methods simply delegate
to the existing castToXxx implementations. Therefore, TRY_CAST cannot reliably
detect conversion failures or distinguish its behavior from CAST.
--
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]
