You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "klsince (via GitHub)" <gi...@apache.org> on 2025/05/12 17:21:22 UTC

Re: [PR] Check for timeColumn type when metadataTTL or deletedKeysTTL is set for Upsert Tables [pinot]

klsince commented on code in PR #15761:
URL: https://github.com/apache/pinot/pull/15761#discussion_r2085121378


##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/ConcurrentMapPartitionUpsertMetadataManagerForConsistentDeletes.java:
##########
@@ -293,6 +293,9 @@ public void doRemoveExpiredPrimaryKeys() {
     double largestSeenComparisonValue = _largestSeenComparisonValue.get();
     double deletedKeysThreshold =
         _deletedKeysTTL > 0 ? largestSeenComparisonValue - _deletedKeysTTL : Double.NEGATIVE_INFINITY;
+    if (_deletedKeysTTL <= 0) {

Review Comment:
   when using this class, the deletedKeysTTL is already assured to be positive: https://github.com/apache/pinot/blob/master/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BaseTableUpsertMetadataManager.java#L100 so we don't have to check it here
   



##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java:
##########
@@ -819,6 +819,14 @@ static void validateTTLForUpsertConfig(TableConfig tableConfig, Schema schema) {
       Preconditions.checkState(comparisonColumnDataType.isNumeric(),
           "MetadataTTL / DeletedKeysTTL must have comparison column: %s in numeric type, found: %s", comparisonColumn,
           comparisonColumnDataType);
+    } else {
+      String comparisonColumn = tableConfig.getValidationConfig().getTimeColumnName();
+      if (comparisonColumn != null && !comparisonColumn.isEmpty()) {

Review Comment:
   I assume the sanity check on the existence of timeColumn must have been done for upsert table.



-- 
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-unsubscribe@pinot.apache.org

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


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