tibrewalpratik17 commented on code in PR #11779:
URL: https://github.com/apache/pinot/pull/11779#discussion_r1356593308


##########
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:
   Sure I see you have raised #11791 to address this. Will rebase once merged. 
   @deemoliu do you see any other issue with this overall design or are we good 
to move forward with this?



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

Reply via email to