KKcorps commented on code in PR #8779:
URL: https://github.com/apache/pinot/pull/8779#discussion_r882631465


##########
pinot-spi/src/main/java/org/apache/pinot/spi/data/DateTimeFormatSpec.java:
##########
@@ -47,28 +50,75 @@ public class DateTimeFormatSpec {
   public static final int MIN_FORMAT_TOKENS = 3;
   public static final int MAX_FORMAT_TOKENS = 4;
 
+  public static final int FORMAT_TIMEFORMAT_POSITION_PIPE = 0;
+  public static final int FORMAT_PATTERN_POSITION_PIPE = 1;

Review Comment:
   Better to divide these static variables into 2 groups one for EPOCH format 
and other for SDF format. These looks a bit odd clumped together. 
   
   You can also create class `EpochDateTimeFormat` and `SDFDateTimeFormat` that 
handle the processing part for this. It will also make it easier to add or 
change a specific format in the future without needing to change the variables



##########
pinot-spi/src/main/java/org/apache/pinot/spi/data/DateTimeFormatSpec.java:
##########
@@ -36,7 +37,9 @@
 public class DateTimeFormatSpec {
 
   public static final String NUMBER_REGEX = "[1-9][0-9]*";
+  public static final String COLON_REGEX = "([0-9])(:)(\\w+)(:)(\\w+)(:?)(.+)";

Review Comment:
   Can we name this variable to a better one. Also, add a brief comment on why 
this is needed (mention the backward compatibility)



-- 
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: commits-unsubscr...@pinot.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to