[GitHub] [pinot] Jackie-Jiang commented on a change in pull request #7385: add more integ test cases for index removal

[GitBox <gi...@apache.org>]

Jackie-Jiang commented on a change in pull request #7385:
URL: https://github.com/apache/pinot/pull/7385#discussion_r700441435



##########
File path: pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/OfflineClusterIntegrationTest.java
##########
@@ -503,53 +503,93 @@ public void testLiteralOnlyFunc()
   @Test
   public void testRangeIndexTriggering()
       throws Exception {
+    String offlineTableName = getTableName();

Review comment:
       This is not `offlineTableName`, but `rawTableName`. Same for other places

##########
File path: pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/OfflineClusterIntegrationTest.java
##########
@@ -355,9 +354,10 @@ public void testInvertedIndexTriggering()
       }
     }, 600_000L, "Failed to cleanup obsolete index");
     // The table size after removing the index might not get back the original one,
-    // i.e. tableSizeWithDefaultIndex. Because entries in index_map file are reordered,
-    // and the file might get a different size. tableSizeAfterRemovingIndex should be
-    // close to tableSizeWithDefaultIndex, but their relationship is not deterministic.
+    // i.e. tableSizeWithDefaultIndex. Because indices in column.psf file are reordered,

Review comment:
       After index removal the size should be deterministic. We can store a member variable for the size, and compare that across these triggering tests

##########
File path: pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/OfflineClusterIntegrationTest.java
##########
@@ -90,12 +90,12 @@
       "SELECT COUNT(*) FROM mytable WHERE DivActualElapsedTime = 305";
 
   // For range index triggering test
-  private static final List<String> UPDATED_RANGE_INDEX_COLUMNS = Collections.singletonList("DivActualElapsedTime");
+  private static final List<String> UPDATED_RANGE_INDEX_COLUMNS = Arrays.asList("Origin", "DivActualElapsedTime");

Review comment:
       We might want to test add/remove at the same time?




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