hililiwei commented on a change in pull request #17749:
URL: https://github.com/apache/flink/pull/17749#discussion_r754827593
##########
File path:
flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/filesystem/DefaultPartTimeExtractor.java
##########
@@ -77,29 +79,49 @@
.toFormatter()
.withResolverStyle(ResolverStyle.LENIENT);
- @Nullable private final String pattern;
+ @Nullable private final String extractorPattern;
+ @Nullable private final String formatterPattern;
- public DefaultPartTimeExtractor(@Nullable String pattern) {
- this.pattern = pattern;
+ public DefaultPartTimeExtractor(
+ @Nullable String extractorPattern, @Nullable String
formatterPattern) {
+ this.extractorPattern = extractorPattern;
+ this.formatterPattern = formatterPattern;
}
@Override
public LocalDateTime extract(List<String> partitionKeys, List<String>
partitionValues) {
String timestampString;
- if (pattern == null) {
+ if (extractorPattern == null) {
timestampString = partitionValues.get(0);
} else {
- timestampString = pattern;
+ timestampString = extractorPattern;
for (int i = 0; i < partitionKeys.size(); i++) {
timestampString =
timestampString.replaceAll(
"\\$" + partitionKeys.get(i),
partitionValues.get(i));
}
}
- return toLocalDateTime(timestampString);
+ return toLocalDateTime(timestampString, this.formatterPattern);
}
- public static LocalDateTime toLocalDateTime(String timestampString) {
+ public static LocalDateTime toLocalDateTime(
+ String timestampString, @Nullable String formatterPattern) {
+
+ if (formatterPattern == null) {
+ return
DefaultPartTimeExtractor.toLocalDateTimeDefault(timestampString);
+ }
Review comment:
```
public static LocalDateTime toLocalDateTimeDefault(String
timestampString) {
try {
return LocalDateTime.parse(timestampString, TIMESTAMP_FORMATTER);
} catch (DateTimeParseException e) {
return LocalDateTime.of(
LocalDate.parse(timestampString, DATE_FORMATTER),
LocalTime.MIDNIGHT);
}
}
```
Yes, I agree with you. I've tried to delete it, but if we remove
toLocalDateTimeDefault, do we have to take out the exception handling
separately?
The toLocalDateTime method might look like this:
```
public static LocalDateTime toLocalDateTime(
String timestampString, @Nullable String formatterPattern) {
DateTimeFormatter dateTimeFormatter =
formatterPattern == null
? TIMESTAMP_FORMATTER
: DateTimeFormatter.ofPattern(formatterPattern,
Locale.ROOT);
try {
return LocalDateTime.parse(timestampString, dateTimeFormatter);
} catch (DateTimeParseException e) {
if (formatterPattern == null) {
dateTimeFormatter = DATE_FORMATTER;
}
return LocalDateTime.of(
LocalDate.parse(timestampString, dateTimeFormatter),
LocalTime.MIDNIGHT);
}
}
```
Personally, I feel that the catch block is a bit inappropriate. How do you
feel about this? Or do you have a better way? thx.
--
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]
