deemoliu commented on code in PR #10915: URL: https://github.com/apache/pinot/pull/10915#discussion_r1258697995
########## pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java: ########## @@ -595,6 +595,39 @@ && isRoutingStrategyAllowedForUpsert(tableConfig.getRoutingConfig()), } } validateAggregateMetricsForUpsertConfig(tableConfig); + validateTTLForUpsertConfig(tableConfig, schema); + } + + /** + * Validates whether the comparison columns is compatible with Upsert TTL feature. + * Validation fails when one of the comparison columns is not a numeric value. + */ + @VisibleForTesting + static void validateTTLForUpsertConfig(TableConfig tableConfig, Schema schema) { + if (tableConfig.getUpsertMode() == UpsertConfig.Mode.NONE + || tableConfig.getUpsertConfig().getUpsertTTLInComparisonTimeUnit() == 0) { + return; + } + + // comparison columns should hold timestamp values in numeric values + List<String> comparisonColumns = tableConfig.getUpsertConfig().getComparisonColumns(); + if (comparisonColumns != null && !comparisonColumns.isEmpty()) { + + // currently we only support 1 comparison column since we need to fetch endTime in comparisonValue time unit from + // columnMetadata. If we have multiple comparison columns, we can only use the first comparison column as filter. + Preconditions.checkState(comparisonColumns.size() <= 1, Review Comment: @Jackie-Jiang i think it's possible user doesn't configure any comparison column and use timeColumn as comparison column by default. so i used <= instead of == -- 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