Jackie-Jiang commented on a change in pull request #5706:
URL: https://github.com/apache/incubator-pinot/pull/5706#discussion_r454770490
##########
File path:
pinot-spi/src/main/java/org/apache/pinot/spi/data/DateTimeGranularitySpec.java
##########
@@ -87,33 +85,43 @@ public TimeUnit getTimeUnit() {
* <ul>
* <li>Convert a granularity to millis.
* This method should not do validation of outputGranularity.
- * The validation should be handled by caller using {@link
#isValidGranularity(String)}</li>
+ * The validation should be handled by caller using {@link
#isValidGranularity}</li>
* <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>
* </ul>
* </ul>
- * @return
*/
public Long granularityToMillis() {
return TimeUnit.MILLISECONDS.convert(_size, _timeUnit);
}
/**
* Check correctness of granularity of {@link DateTimeFieldSpec}
- * @param granularity
- * @return
*/
- public static boolean isValidGranularity(String granularity) {
- Preconditions.checkNotNull(granularity);
+ public static boolean isValidGranularity(String granularity, Logger
ctxLogger) {
Review comment:
Recommend using exception to pass the failure message instead of passing
in a logger. The problem of using logger is that the caller cannot control the
logging level. For user input, in most cases we don't want to log error which
usually stands for severe problems.
```suggestion
public static void validateGranularity(String granularity) {
```
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/util/SchemaUtils.java
##########
@@ -39,65 +46,99 @@
private static final Logger LOGGER =
LoggerFactory.getLogger(SchemaUtils.class);
- /**
- * Validates that for a field spec with transform function, the source
column name and destination column name are exclusive
- * i.e. do not allow using source column name for destination column
- */
- public static boolean validate(Schema schema) {
Review comment:
Let's keep this method for simplicity
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
