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: [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]
