Jackie-Jiang commented on code in PR #11791:
URL: https://github.com/apache/pinot/pull/11791#discussion_r1358998624


##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java:
##########
@@ -138,15 +138,27 @@ public void addSegment(ImmutableSegment segment) {
       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");
       Number maxComparisonValue =
           (Number) 
segment.getSegmentMetadata().getColumnMetadataMap().get(_comparisonColumns.get(0)).getMaxValue();
       if (maxComparisonValue.doubleValue() < _largestSeenComparisonValue - 
_metadataTTL) {
         _logger.info("Skip adding segment: {} because it's out of TTL", 
segmentName);
         MutableRoaringBitmap validDocIdsSnapshot = 
immutableSegment.loadValidDocIdsFromSnapshot();
         if (validDocIdsSnapshot != null) {
-          immutableSegment.enableUpsert(this, new 
ThreadSafeMutableRoaringBitmap(validDocIdsSnapshot), null);
+          ThreadSafeMutableRoaringBitmap queryableDocIds = null;
+          if (_deleteRecordColumn != null) {
+            queryableDocIds = new ThreadSafeMutableRoaringBitmap();
+            UpsertUtils.RecordInfoReader recordInfoReader =
+                new UpsertUtils.RecordInfoReader(segment, _primaryKeyColumns, 
_comparisonColumns, _deleteRecordColumn);

Review Comment:
   Ideally, we only need to go over the deleted record column without paying 
the overhead of reading primary key, comparison column etc. I'd suggest adding 
a method `MutableRoaringBitmap getQueryableDocIds(IndexSegment segment, String 
deleteRecordColumn, MutableRoaringBitmap validDocIds)`



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