clintropolis commented on a change in pull request #6365: new feature: allows
user to specify timeZone in TimestampSpec
URL: https://github.com/apache/incubator-druid/pull/6365#discussion_r257089986
##########
File path:
core/src/main/java/org/apache/druid/data/input/impl/TimestampSpec.java
##########
@@ -60,12 +66,30 @@ public TimestampSpec(
@JsonProperty("column") String timestampColumn,
@JsonProperty("format") String format,
// this value should never be set for production data
+ @JsonProperty("missingValue") DateTime missingValue,
+ @JsonProperty("timeZone") String timeZone
+ )
+ {
+ this.timestampColumn = (timestampColumn == null) ? DEFAULT_COLUMN :
timestampColumn;
+ this.timestampFormat = format == null ? DEFAULT_FORMAT : format;
+ this.timeZone = StringUtils.isBlank(timeZone) ? DateTimes.UTC_TIMEZONE :
timeZone;
+ this.timestampConverter =
TimestampParser.createObjectTimestampParser(timestampFormat, this.timeZone);
+ this.missingValue = missingValue == null
+ ? DEFAULT_MISSING_VALUE
+ : missingValue;
+ }
+
+ @VisibleForTesting
+ public TimestampSpec(
Review comment:
I think if you want to go this route, you would just have the "test"
constructor call the other "full" constructor, something like this:
```
@VisibleForTesting
public TimestampSpec(
String timestampColumn,
String format,
DateTime missingValue
)
{
this(timestampColumn, format, missingValue, null);
}
```
The `@JsonProperty` annotations on the arguments would not be necessary
since the other constructor is the `@JsonCreator`.
Additionally, you would probably want to undo all of the tests calling the 4
argument constructor or there isn't really any reason to have this here.
I think you should either remove this and leave the PR as is, or fixup the
tests to call the test constructor, but I don't have strong opinions either way
so up to you which approach you'd like to do.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
