hanyuzheng7 commented on code in PR #25759:
URL: https://github.com/apache/flink/pull/25759#discussion_r1887309875
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/TimeFunctionsITCase.java:
##########
@@ -732,6 +764,39 @@ private Stream<TestSetSpec> floorTestCases() {
$("f2").floor(TimeIntervalUnit.MILLENNIUM),
"FLOOR(f2 TO MILLENNIUM)",
LocalDateTime.of(2001, 1, 1, 0, 0),
- TIMESTAMP().nullable()));
+ TIMESTAMP().nullable())
+ .testResult(
+ $("f2").cast(TIMESTAMP_LTZ(3))
+ .floor(TimeIntervalUnit.SECOND)
+ .cast(STRING()),
+ "CAST(FLOOR(CAST(f2 AS TIMESTAMP_LTZ(3)) TO
SECOND) AS STRING)",
+ formatAsTimestamp(LocalDateTime.of(2020, 2,
29, 1, 56, 59, 0)),
+ STRING().nullable())
+ .testResult(
+ $("f2").cast(TIMESTAMP_LTZ(3))
+ .floor(TimeIntervalUnit.MINUTE)
+ .cast(STRING()),
+ "CAST(FLOOR(CAST(f2 AS TIMESTAMP_LTZ(3)) TO
MINUTE) AS STRING)",
+ formatAsTimestamp(LocalDateTime.of(2020, 2,
29, 1, 56, 0, 0)),
+ STRING().nullable())
+ .testResult(
+ $("f2").cast(TIMESTAMP_LTZ(3))
+ .floor(TimeIntervalUnit.HOUR)
+ .cast(STRING()),
+ "CAST(FLOOR(CAST(f2 AS TIMESTAMP_LTZ(3)) TO
HOUR) AS STRING)",
+ formatAsTimestamp(LocalDateTime.of(2020, 2,
29, 1, 0, 0, 0)),
+ STRING().nullable())
+ .testResult(
+ $("f2").cast(TIMESTAMP_LTZ(3))
+ .floor(TimeIntervalUnit.MILLISECOND)
+ .cast(STRING()),
+ "CAST(FLOOR(CAST(f2 AS TIMESTAMP_LTZ(3)) TO
MILLISECOND) AS STRING)",
+ formatAsTimestamp(
+ LocalDateTime.of(2020, 2, 29, 1, 56,
59, 987_000_000)),
+ STRING().nullable()));
+ }
+
+ private static String formatAsTimestamp(LocalDateTime dateTime) {
+ return dateTime.format(DateTimeFormatter.ofPattern("yyyy-MM-dd'
'HH:mm:ss.SSS"));
Review Comment:
Thank you for your suggestion. I have a question: if we extract
`DateTimeFormatter.ofPattern("yyyy-MM-dd' 'HH:mm:ss.SSS")` as a separate
constant, should we still keep the `formatAsTimestamp` method?
For example, is it better to write:
```
private static final DateTimeFormatter TIMESTAMP_FORMATTER =
DateTimeFormatter.ofPattern("yyyy-MM-dd' 'HH:mm:ss.SSS");
private static String formatAsTimestamp(LocalDateTime dateTime) {
return dateTime.format(TIMESTAMP_FORMATTER);
}
.testResult(
$("f2").cast(TIMESTAMP_LTZ(3))
.floor(TimeIntervalUnit.MILLISECOND)
.cast(STRING()),
"CAST(FLOOR(CAST(f2 AS TIMESTAMP_LTZ(3)) TO MILLISECOND) AS STRING)",
formatAsTimestamp(
LocalDateTime.of(2020, 2, 29, 1, 56, 59, 987_000_000)
),
STRING().nullable()
);
```
Or should we directly use TIMESTAMP_FORMATTER like this:
```
private static final DateTimeFormatter TIMESTAMP_FORMATTER =
DateTimeFormatter.ofPattern("yyyy-MM-dd' 'HH:mm:ss.SSS");
.testResult(
$("f2").cast(TIMESTAMP_LTZ(3))
.floor(TimeIntervalUnit.MILLISECOND)
.cast(STRING()),
"CAST(FLOOR(CAST(f2 AS TIMESTAMP_LTZ(3)) TO MILLISECOND) AS STRING)",
LocalDateTime.of(2020, 2, 29, 1, 56, 59, 987_000_000)
.format(TIMESTAMP_FORMATTER),
STRING().nullable()
);
```
Which approach is more readable and maintainable in this scenario?
I prefer the second one directly use `TIMESTAMP_FORMATTER`.
--
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
