Copilot commented on code in PR #16726:
URL: https://github.com/apache/pinot/pull/16726#discussion_r2317628435


##########
pinot-server/src/test/java/org/apache/pinot/server/api/TablesResourceTest.java:
##########
@@ -413,6 +452,13 @@ private void downLoadAndVerifyValidDocIdsSnapshot(String 
tableNameWithType, Immu
     FileUtils.writeByteArrayToFile(validDocIdsSnapshotFile,
         
RoaringBitmapUtils.serialize(validDocIdsSnapshot.getMutableRoaringBitmap()));
 
+    // Create the queryableDocIds snapshot file needed for SNAPSHOT_WITH_DELETE
+    File queryableDocIdsSnapshotFile =
+        new 
File(SegmentDirectoryPaths.findSegmentDirectory(segment.getSegmentMetadata().getIndexDir()),
+            V1Constants.QUERYABLE_DOC_IDS_SNAPSHOT_FILE_NAME);
+    FileUtils.writeByteArrayToFile(queryableDocIdsSnapshotFile,
+        
RoaringBitmapUtils.serialize(queryableDocIds.getMutableRoaringBitmap()));

Review Comment:
   [nitpick] This code for creating queryable doc IDs snapshot files is 
duplicated in both `downLoadAndVerifyValidDocIdsSnapshot` and 
`downLoadAndVerifyValidDocIdsSnapshotBitmap` methods. Consider extracting this 
logic into a private helper method to reduce code duplication.



##########
pinot-server/src/test/java/org/apache/pinot/server/api/TablesResourceTest.java:
##########
@@ -521,6 +575,60 @@ private void 
downLoadAndVerifyValidDocIdsSnapshotBitmap(String tableNameWithType
         queryableDocIds.getMutableRoaringBitmap());
   }
 
+  @Test
+  public void testValidDocIdsMetadataGetForSnapshotWithDelete()
+      throws IOException {
+    IndexSegment segment = _realtimeIndexSegments.get(0);
+    // Verify the content of the downloaded snapshot from a realtime table.
+    
downLoadAndVerifyValidDocIdsSnapshot(TableNameBuilder.REALTIME.tableNameWithType(TABLE_NAME),
+        (ImmutableSegmentImpl) segment);
+    
downLoadAndVerifyValidDocIdsSnapshotBitmap(TableNameBuilder.REALTIME.tableNameWithType(TABLE_NAME),
+        (ImmutableSegmentImpl) segment);

Review Comment:
   [nitpick] The setup code calling `downLoadAndVerifyValidDocIdsSnapshot` and 
`downLoadAndVerifyValidDocIdsSnapshotBitmap` is duplicated across three test 
methods. Consider extracting this setup into a private helper method or using a 
test setup method to reduce duplication.
   ```suggestion
     /**
      * Helper method to perform the common setup for valid doc ids snapshot 
and bitmap verification.
      */
     private void verifyValidDocIdsSnapshotAndBitmap(String tableNameWithType, 
ImmutableSegmentImpl segment) throws IOException {
       downLoadAndVerifyValidDocIdsSnapshot(tableNameWithType, segment);
       downLoadAndVerifyValidDocIdsSnapshotBitmap(tableNameWithType, segment);
     }
   
     @Test
     public void testValidDocIdsMetadataGetForSnapshotWithDelete()
         throws IOException {
       IndexSegment segment = _realtimeIndexSegments.get(0);
       // Verify the content of the downloaded snapshot from a realtime table.
       
verifyValidDocIdsSnapshotAndBitmap(TableNameBuilder.REALTIME.tableNameWithType(TABLE_NAME),
           (ImmutableSegmentImpl) segment);
   ```



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