yuxiqian commented on code in PR #3449:
URL: https://github.com/apache/flink-cdc/pull/3449#discussion_r1704771418
##########
flink-cdc-runtime/src/main/java/org/apache/flink/cdc/runtime/parser/JaninoCompiler.java:
##########
@@ -300,7 +315,19 @@ private static Java.Rvalue generateOtherFunctionOperation(
}
}
- private static Java.Rvalue
generateNoOperandTimestampFunctionOperation(String operationName) {
+ private static Java.Rvalue
generateTimezoneFreeTemporalFunctionOperation(String operationName) {
+ List<Java.Rvalue> timestampFunctionParam = new ArrayList<>();
+ timestampFunctionParam.add(
+ new Java.AmbiguousName(Location.NOWHERE, new String[]
{DEFAULT_EPOCH_TIME}));
+ return new Java.MethodInvocation(
+ Location.NOWHERE,
+ null,
+ StringUtils.convertToCamelCase(operationName),
+ timestampFunctionParam.toArray(new Java.Rvalue[0]));
Review Comment:
I wonder if is it necessary to initialize `timestampFunctionParam` here,
since it seems did the same thing:
```java
new Java.Rvalue[] {
new Java.AmbiguousName(Location.NOWHERE, new String[] {
DEFAULT_EPOCH_TIME
})
}
```
##########
flink-cdc-runtime/src/main/java/org/apache/flink/cdc/runtime/parser/JaninoCompiler.java:
##########
@@ -313,6 +340,27 @@ private static Java.Rvalue
generateNoOperandTimestampFunctionOperation(String op
timestampFunctionParam.toArray(new Java.Rvalue[0]));
}
+ private static Java.Rvalue
generateTimezoneFreeTemporalConversionFunctionOperation(
+ String operationName) {
+ return new Java.MethodInvocation(
+ Location.NOWHERE,
+ null,
+ StringUtils.convertToCamelCase(operationName),
+ new ArrayList<>().toArray(new Java.Rvalue[0]));
Review Comment:
```suggestion
new Java.Rvalue[0]);
```
##########
flink-cdc-runtime/src/main/java/org/apache/flink/cdc/runtime/parser/JaninoCompiler.java:
##########
@@ -359,6 +407,15 @@ private static Java.Rvalue generateTypeConvertMethod(
case "VARCHAR":
case "STRING":
return new Java.MethodInvocation(Location.NOWHERE, null,
"castToString", atoms);
+ case "TIMESTAMP":
+ List<Java.Rvalue> timestampFunctionParam = new
ArrayList<>(Arrays.asList(atoms));
+ timestampFunctionParam.add(
+ new Java.AmbiguousName(Location.NOWHERE, new String[]
{DEFAULT_TIME_ZONE}));
+ return new Java.MethodInvocation(
+ Location.NOWHERE,
+ null,
+ "castToTimestamp",
+ timestampFunctionParam.toArray(new Java.Rvalue[0]));
Review Comment:
I remember that there's a `ArrayUtils#add(T[], T)` method in Apache commons
that doesn't require initializing mutable lists and keeps code clearer. Is it
possible to use it here?
--
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
