lvyanquan commented on code in PR #4492:
URL: https://github.com/apache/flink-cdc/pull/4492#discussion_r3756719729
##########
flink-cdc-runtime/src/main/java/org/apache/flink/cdc/runtime/parser/JaninoCompiler.java:
##########
@@ -564,7 +569,9 @@ private static boolean isIdentifierNullable(Context
context, SqlIdentifier sqlId
}
private static boolean isBasicCallNullable(Context context, SqlBasicCall
sqlBasicCall) {
- if (sqlBasicCall.getOperator().getName().equalsIgnoreCase("IFNULL")) {
+ if (sqlBasicCall.getOperator().getName().equalsIgnoreCase("IFNULL")
Review Comment:
Thanks for the fix — the precedence behavior is now correct and well tested.
One structural concern though: the current approach guards each built-in
branch with a negative check (`!findUserDefinedFunction(...).isPresent()`),
scattered across four places (the `TRY_CAST` entry, `case NULLIF:` in the
SqlKind switch, the IFNULL/NULLIF branches here, and `isBasicCallNullable`).
This inverts the established convention in this method — the `orElseGet`
fallback below is structurally "UDF first, built-in fallback", whereas the fix
is "built-in first, exclude UDFs case by case".
Would you consider consolidating this into a single positive dispatch point
at the top of function-call translation? Something like:
```java
Optional<UserDefinedFunctionDescriptor> udf =
findUserDefinedFunction(context, functionName);
if (udf.isPresent()) {
return generateUdfInvocation(context, udf.get(), atoms);
}
// built-in special cases (TRY_CAST / IF / IFNULL / NULLIF / ...) below
```
--
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]
