walterddr commented on a change in pull request #7804:
URL: https://github.com/apache/pinot/pull/7804#discussion_r762089070
##########
File path:
pinot-spi/src/main/java/org/apache/pinot/spi/data/DateTimeFormatPatternSpec.java
##########
@@ -52,7 +66,28 @@ public DateTimeFormatPatternSpec(String timeFormat, String
sdfPatternWithTz) {
String timezoneString = m.group(TIMEZONE_GROUP).trim();
_dateTimeZone =
DateTimeZone.forTimeZone(TimeZone.getTimeZone(timezoneString));
}
+ validateSdfPattern(_sdfPattern, requireLexicographicOrdering);
_dateTimeFormatter =
DateTimeFormat.forPattern(_sdfPattern).withZone(_dateTimeZone).withLocale(DEFAULT_LOCALE);
+
+ }
+ }
+
+ private static void validateSdfPattern(String sdfPattern, boolean
requireLexicographicOrdering) {
Review comment:
hmm i dont think this will work: SchemaUtils.validate only validates
Schema. this means either
1. we disallow all non-lexicographic ordering on date/time column at all
2. we still cant tell whether a non-lexicographic ordering date/time column
will cause problem in segment generation because the info is in TableConfig.
The only place I can think of that make sense is
TableConfigUtils.validate(tableconfig, schema).
but it is not guarantee to be validated because the API said schema is
nullable and if null, schema related checks are skipped.
--
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
