Jackie-Jiang commented on code in PR #9355:
URL: https://github.com/apache/pinot/pull/9355#discussion_r967431208
##########
pinot-spi/src/main/java/org/apache/pinot/spi/data/DateTimeFieldSpec.java:
##########
@@ -119,6 +137,15 @@ public DateTimeFormatSpec getFormatSpec() {
formatSpec = new DateTimeFormatSpec(_format);
_formatSpec = formatSpec;
}
+
+ if (_sampleValue != null) {
Review Comment:
We might not want to validate the sample value here. Suggest adding a method
`validateSampleValue()` and call it when uploading the schema.
##########
pinot-spi/src/main/java/org/apache/pinot/spi/data/DateTimeFieldSpec.java:
##########
@@ -70,13 +72,19 @@ public DateTimeFieldSpec() {
* 2) if a time column is defined in hoursSinceEpoch
(format=1:HOURS:EPOCH), and the data buckets are 1 hours,
* the granularity will be 1:HOURS
*/
- public DateTimeFieldSpec(String name, DataType dataType, String format,
String granularity) {
+ public DateTimeFieldSpec(String name, DataType dataType, String format,
String granularity,
+ @Nullable String sampleValue) {
super(name, dataType, true);
_format = format;
_granularity = granularity;
_formatSpec = new DateTimeFormatSpec(format);
_granularitySpec = new DateTimeGranularitySpec(granularity);
+ _sampleValue = sampleValue;
Review Comment:
Sample value should be an `Object` (e.g. user may put long number as the
sample value)
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/SegmentColumnarIndexCreator.java:
##########
@@ -765,6 +766,24 @@ private void writeMetadata()
"Invalid segment start/end time: %s (in millis: %s/%s) for time
column: %s, must be between: %s",
timeInterval, timeInterval.getStartMillis(),
timeInterval.getEndMillis(), timeColumnName,
TimeUtils.VALID_TIME_INTERVAL);
+ } else {
+ long now = System.currentTimeMillis();
Review Comment:
We don't want to change the start/end time within the segment metadata,
instead we want to enhance the time management code to gracefully handle the
segments with invalid time values.
Suggest using a separate PR for the time management change, and only keep
the sample value check in this PR.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
