You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by GitBox <gi...@apache.org> on 2020/08/19 19:07:05 UTC
[GitHub] [hudi] vinothchandar commented on a change in pull request #1987: [HUDI-1177]: fixed TaskNotSerializableException in TimestampBasedKeyGenerator
vinothchandar commented on a change in pull request #1987:
URL: https://github.com/apache/hudi/pull/1987#discussion_r473257409
##########
File path: hudi-spark/src/main/java/org/apache/hudi/keygen/parser/HoodieDateTimeParserImpl.java
##########
@@ -95,7 +86,15 @@ public String getOutputDateFormat() {
@Override
public DateTimeFormatter getInputFormatter() {
- return this.inputFormatter;
+ TimestampType timestampType = TimestampType.valueOf(config.getString(Config.TIMESTAMP_TYPE_FIELD_PROP));
+ if (timestampType == TimestampType.DATE_STRING || timestampType == TimestampType.MIXED) {
+ DataSourceUtils.checkRequiredProperties(config,
+ Collections.singletonList(Config.TIMESTAMP_INPUT_DATE_FORMAT_PROP));
+ this.configInputDateFormatList = config.getString(Config.TIMESTAMP_INPUT_DATE_FORMAT_PROP, "");
+ return getInputDateFormatter();
+ }
+
+ return null;
Review comment:
throw an exception? returning null is like playing with fire :)
##########
File path: hudi-spark/src/main/java/org/apache/hudi/keygen/TimestampBasedKeyGenerator.java
##########
@@ -153,7 +147,8 @@ public String getPartitionPath(GenericRecord record) {
* @return the parsed partition path based on data type
* @throws ParseException on any parse exception
*/
- private String getPartitionPath(Object partitionVal) throws ParseException {
+ private String getPartitionPath(Object partitionVal) {
+ DateTimeFormatter inputFormatter = parser.getInputFormatter();
Review comment:
let's move any other per record allocation here.
##########
File path: hudi-spark/src/main/java/org/apache/hudi/keygen/TimestampBasedKeyGenerator.java
##########
@@ -153,7 +147,8 @@ public String getPartitionPath(GenericRecord record) {
* @return the parsed partition path based on data type
* @throws ParseException on any parse exception
*/
- private String getPartitionPath(Object partitionVal) throws ParseException {
+ private String getPartitionPath(Object partitionVal) {
+ DateTimeFormatter inputFormatter = parser.getInputFormatter();
Review comment:
let's make these `transient` and lazily init this as needed.
```
private void initIfNeeded() {
if (inputFormatter != null) {
this.inputFormatter = parser.getInputFormatter();
this.partitionFormatter = DateTimeFormat.forPattern(outputDateFormat);
}
}
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org