tibrewalpratik17 commented on code in PR #11030:
URL: https://github.com/apache/pinot/pull/11030#discussion_r1252354689
##########
pinot-spi/src/main/java/org/apache/pinot/spi/data/DateTimeGranularitySpec.java:
##########
@@ -43,21 +49,41 @@ public class DateTimeGranularitySpec {
*/
public DateTimeGranularitySpec(String granularity) {
Preconditions.checkArgument(StringUtils.isNotEmpty(granularity), "Must
provide granularity");
- String[] granularityTokens = StringUtil.split(granularity, SEPARATOR, 2);
- Preconditions.checkArgument(granularityTokens.length >= NUM_TOKENS,
- "Invalid granularity: %s, must be of format 'size:timeUnit",
granularity);
+
+ int sizePosition;
+ int timeUnitPosition;
+ String[] granularityTokens;
+
+ if (Character.isDigit(granularity.charAt(0))) {
+ // Colon format
+ granularityTokens = StringUtil.split(granularity, COLON_SEPARATOR, 2);
+ Preconditions.checkArgument(granularityTokens.length >= NUM_TOKENS,
+ "Invalid granularity: %s, must be of format 'size:timeUnit",
granularity);
Review Comment:
We can merge both the `Preconditions.checkArgument` behind a boolean flag
and throw an error something like:
```suggestion
Preconditions.checkArgument(granularityTokens.length >= NUM_TOKENS,
"Invalid granularity: %s, must be of format 'size:timeUnit or
timeUnit|size", granularity);
```
##########
pinot-spi/src/main/java/org/apache/pinot/spi/data/DateTimeGranularitySpec.java:
##########
@@ -83,9 +109,9 @@ public TimeUnit getTimeUnit() {
/**
* Converts a granularity to millis.
* <ul>
- * <li>1) granularityToMillis(1:HOURS) = 3600000 (60*60*1000)</li>
- * <li>2) granularityToMillis(1:MILLISECONDS) = 1</li>
- * <li>3) granularityToMillis(15:MINUTES) = 900000 (15*60*1000)</li>
Review Comment:
let's not remove this but mark as old format?
##########
pinot-spi/src/main/java/org/apache/pinot/spi/data/DateTimeGranularitySpec.java:
##########
@@ -29,10 +29,16 @@
* Class to represent granularity from {@link DateTimeFieldSpec}
*/
public class DateTimeGranularitySpec {
- // 'size:timeUnit'
- private static final char SEPARATOR = ':';
- private static final int SIZE_POSITION = 0;
- private static final int TIME_UNIT_POSITION = 1;
+ // Colon format:
Review Comment:
nit: let's add the format in the comment as it was originally present
--
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]
