[incubator-pinot] branch master updated: Do not prune segments for selection queries over upsert table (#6158)

[jackie]

This is an automated email from the ASF dual-hosted git repository.

jackie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git


The following commit(s) were added to refs/heads/master by this push:
     new 0c19741  Do not prune segments for selection queries over upsert table 
(#6158)
0c19741 is described below

commit 0c197417b5aabc38358bf9a90f60309d1cadbe02
Author: Yupeng Fu <yupe...@users.noreply.github.com>
AuthorDate: Mon Oct 19 17:49:21 2020 -0700

    Do not prune segments for selection queries over upsert table (#6158)
    
    Part of a series of PRs for #4261
---
 .../query/pruner/SelectionQuerySegmentPruner.java  |  9 +++++--
 .../pruner/SelectionQuerySegmentPrunerTest.java    | 30 ++++++++++++++++++++++
 2 files changed, 37 insertions(+), 2 deletions(-)

diff --git 
a/pinot-core/src/main/java/org/apache/pinot/core/query/pruner/SelectionQuerySegmentPruner.java
 
b/pinot-core/src/main/java/org/apache/pinot/core/query/pruner/SelectionQuerySegmentPruner.java
index d5b5145..82d46b2 100644
--- 
a/pinot-core/src/main/java/org/apache/pinot/core/query/pruner/SelectionQuerySegmentPruner.java
+++ 
b/pinot-core/src/main/java/org/apache/pinot/core/query/pruner/SelectionQuerySegmentPruner.java
@@ -76,13 +76,18 @@ public class SelectionQuerySegmentPruner implements 
SegmentPruner {
     }
 
     // If LIMIT is not 0, only prune segments for selection queries without 
filter
-    // TODO: This does not take upsert valid doc ids filter into account, 
which can cause less than expected records
-    //       returned for upsert table
     FilterContext filter = queryContext.getFilter();
     if (filter != null) {
       return segmentDataManagers;
     }
 
+    // Do not prune for upsert table
+    if (!segmentDataManagers.isEmpty()) {
+      if (segmentDataManagers.get(0).getSegment().getValidDocIndex() != null) {
+        return segmentDataManagers;
+      }
+    }
+
     if (queryContext.getOrderByExpressions() == null) {
       return pruneSelectionOnly(tableDataManager, segmentDataManagers, 
queryContext);
     } else {
diff --git 
a/pinot-core/src/test/java/org/apache/pinot/core/query/pruner/SelectionQuerySegmentPrunerTest.java
 
b/pinot-core/src/test/java/org/apache/pinot/core/query/pruner/SelectionQuerySegmentPrunerTest.java
index 09e83be..ae1d6c7 100644
--- 
a/pinot-core/src/test/java/org/apache/pinot/core/query/pruner/SelectionQuerySegmentPrunerTest.java
+++ 
b/pinot-core/src/test/java/org/apache/pinot/core/query/pruner/SelectionQuerySegmentPrunerTest.java
@@ -30,6 +30,7 @@ import org.apache.pinot.core.query.request.ServerQueryRequest;
 import org.apache.pinot.core.query.request.context.QueryContext;
 import 
org.apache.pinot.core.query.request.context.utils.QueryContextConverterUtils;
 import org.apache.pinot.core.segment.index.metadata.SegmentMetadata;
+import org.apache.pinot.core.segment.index.readers.ValidDocIndexReader;
 import org.testng.annotations.Test;
 
 import static org.mockito.Mockito.mock;
@@ -64,6 +65,26 @@ public class SelectionQuerySegmentPrunerTest {
   }
 
   @Test
+  public void testSelectionOnlyUpsert() {
+    List<SegmentDataManager> segmentDataManagers = Arrays
+        .asList(getSegmentDataManager(null, null, 10, true), 
getSegmentDataManager(0L, 10L, 10, true),
+            getSegmentDataManager(-5L, 5L, 15, true));
+
+    // Should keep enough documents to fulfill the LIMIT requirement
+    ServerQueryRequest queryRequest = getQueryRequest("SELECT * FROM testTable 
LIMIT 5");
+    List<SegmentDataManager> result = _segmentPruner.prune(_tableDataManager, 
segmentDataManagers, queryRequest);
+    assertEquals(result.size(), 3);
+
+    queryRequest = getQueryRequest("SELECT * FROM testTable LIMIT 10");
+    result = _segmentPruner.prune(_tableDataManager, segmentDataManagers, 
queryRequest);
+    assertEquals(result.size(), 3);
+
+    queryRequest = getQueryRequest("SELECT * FROM testTable LIMIT 15");
+    result = _segmentPruner.prune(_tableDataManager, segmentDataManagers, 
queryRequest);
+    assertEquals(result.size(), 3);
+  }
+
+  @Test
   public void testSelectionOnly() {
     List<SegmentDataManager> segmentDataManagers = Arrays
         .asList(getSegmentDataManager(null, null, 10), 
getSegmentDataManager(0L, 10L, 10),
@@ -182,6 +203,11 @@ public class SelectionQuerySegmentPrunerTest {
   }
 
   private SegmentDataManager getSegmentDataManager(@Nullable Long minValue, 
@Nullable Long maxValue, int totalDocs) {
+    return getSegmentDataManager(minValue, maxValue, totalDocs, false);
+  }
+
+  private SegmentDataManager getSegmentDataManager(@Nullable Long minValue, 
@Nullable Long maxValue, int totalDocs,
+      boolean upsert) {
     SegmentDataManager segmentDataManager = mock(SegmentDataManager.class);
     IndexSegment segment = mock(IndexSegment.class);
     when(segmentDataManager.getSegment()).thenReturn(segment);
@@ -194,6 +220,10 @@ public class SelectionQuerySegmentPrunerTest {
     SegmentMetadata segmentMetadata = mock(SegmentMetadata.class);
     when(segment.getSegmentMetadata()).thenReturn(segmentMetadata);
     when(segmentMetadata.getTotalDocs()).thenReturn(totalDocs);
+    if (upsert) {
+      ValidDocIndexReader reader = mock(ValidDocIndexReader.class);
+      when(segment.getValidDocIndex()).thenReturn(reader);
+    }
     return segmentDataManager;
   }
 


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org