deemoliu commented on code in PR #11779: URL: https://github.com/apache/pinot/pull/11779#discussion_r1355858602
########## pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java: ########## @@ -129,20 +133,23 @@ public void addSegment(ImmutableSegment segment) { return; } Preconditions.checkArgument(segment instanceof ImmutableSegmentImpl, - "Got unsupported segment implementation: {} for segment: {}, table: {}", segment.getClass(), segmentName, - _tableNameWithType); + "Got unsupported segment implementation: {} for segment: {}, table: {}", segment.getClass(), + segmentName, _tableNameWithType); ImmutableSegmentImpl immutableSegment = (ImmutableSegmentImpl) segment; // Skip adding segment that has max comparison value smaller than (largestSeenComparisonValue - TTL) - if (_largestSeenComparisonValue > 0) { + // if _enableMetadataTTLForDeletedRecord is true then update _largestSeenComparisonValue to max comparison value + // and do not skip adding segment + if (_largestSeenComparisonValue > 0 || _enableMetadataTTLForDeletedRecord) { Preconditions.checkState(_enableSnapshot, "Upsert TTL must have snapshot enabled"); Preconditions.checkState(_comparisonColumns.size() == 1, "Upsert TTL does not work with multiple comparison columns"); - // TODO: Support deletion for TTL. Need to construct queryableDocIds when adding segments out of TTL. - Preconditions.checkState(_deleteRecordColumn == null, "Upsert TTL doesn't work with record deletion"); + Preconditions.checkState(_deleteRecordColumn == null || _enableMetadataTTLForDeletedRecord, + "Upsert TTL doesn't work with record deletion without enableMetadataTTLForDeletedRecord flag"); Review Comment: Hi @tibrewalpratik17 this is a todo, we should mentioned upsert ttl with deletion is `to be implemented` instead mentioned that `Upsert TTL doesn't work with record deletion without enableMetadataTTLForDeletedRecord flag`. Let me take create a patch to address this todo, then you can add this `enableMetadataTTLForDeletedRecord` check -- 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