You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iotdb.apache.org by ja...@apache.org on 2023/03/24 00:52:01 UTC

[iotdb] branch rel/1.1 updated: [IOTDB-5717] Fix incorrect result when querying with limit push-downing

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

jackietien pushed a commit to branch rel/1.1
in repository https://gitbox.apache.org/repos/asf/iotdb.git


The following commit(s) were added to refs/heads/rel/1.1 by this push:
     new 494e866273 [IOTDB-5717] Fix incorrect result when querying with limit push-downing
494e866273 is described below

commit 494e866273049819915ab0a5d82fca7430bed9e7
Author: Jackie Tien <ja...@gmail.com>
AuthorDate: Fri Mar 24 08:51:54 2023 +0800

    [IOTDB-5717] Fix incorrect result when querying with limit push-downing
---
 .../execution/operator/source/SeriesScanUtil.java  | 12 +++-
 .../series/SeriesScanLimitOffsetPushDownTest.java  | 83 ++++++++++++++++++++--
 .../read/reader/series/PaginationController.java   | 23 ++++++
 3 files changed, 109 insertions(+), 9 deletions(-)

diff --git a/server/src/main/java/org/apache/iotdb/db/mpp/execution/operator/source/SeriesScanUtil.java b/server/src/main/java/org/apache/iotdb/db/mpp/execution/operator/source/SeriesScanUtil.java
index 755795447f..d42c2ff931 100644
--- a/server/src/main/java/org/apache/iotdb/db/mpp/execution/operator/source/SeriesScanUtil.java
+++ b/server/src/main/java/org/apache/iotdb/db/mpp/execution/operator/source/SeriesScanUtil.java
@@ -636,8 +636,16 @@ public class SeriesScanUtil {
       if (queryFilter != null) {
         firstPageReader.setFilter(queryFilter);
       }
-      firstPageReader.setLimitOffset(paginationController);
-      TsBlock tsBlock = firstPageReader.getAllSatisfiedPageData(orderUtils.getAscending());
+      TsBlock tsBlock;
+      if (orderUtils.getAscending()) {
+        firstPageReader.setLimitOffset(paginationController);
+        tsBlock = firstPageReader.getAllSatisfiedPageData(orderUtils.getAscending());
+      } else {
+        tsBlock =
+            paginationController.applyTsBlock(
+                firstPageReader.getAllSatisfiedPageData(orderUtils.getAscending()));
+      }
+
       firstPageReader = null;
 
       return tsBlock;
diff --git a/server/src/test/java/org/apache/iotdb/db/query/reader/series/SeriesScanLimitOffsetPushDownTest.java b/server/src/test/java/org/apache/iotdb/db/query/reader/series/SeriesScanLimitOffsetPushDownTest.java
index 5d58b164fe..e41a10084d 100644
--- a/server/src/test/java/org/apache/iotdb/db/query/reader/series/SeriesScanLimitOffsetPushDownTest.java
+++ b/server/src/test/java/org/apache/iotdb/db/query/reader/series/SeriesScanLimitOffsetPushDownTest.java
@@ -230,7 +230,8 @@ public class SeriesScanLimitOffsetPushDownTest {
     EnvironmentUtils.cleanAllDir();
   }
 
-  private SeriesScanUtil getSeriesScanUtil(long limit, long offset) throws IllegalPathException {
+  private SeriesScanUtil getSeriesScanUtil(long limit, long offset, Ordering scanOrder)
+      throws IllegalPathException {
     MeasurementPath scanPath = new MeasurementPath(TEST_PATH, TSDataType.INT32);
 
     SeriesScanOptions.Builder scanOptionsBuilder = new SeriesScanOptions.Builder();
@@ -240,7 +241,7 @@ public class SeriesScanLimitOffsetPushDownTest {
     SeriesScanUtil seriesScanUtil =
         new SeriesScanUtil(
             scanPath,
-            Ordering.ASC,
+            scanOrder,
             scanOptionsBuilder.build(),
             EnvironmentUtils.TEST_QUERY_FI_CONTEXT);
     seriesScanUtil.initQueryDataSource(new QueryDataSource(seqResources, unSeqResources));
@@ -249,7 +250,7 @@ public class SeriesScanLimitOffsetPushDownTest {
 
   @Test
   public void testSkipFile() throws IllegalPathException, IOException {
-    SeriesScanUtil seriesScanUtil = getSeriesScanUtil(5, 10);
+    SeriesScanUtil seriesScanUtil = getSeriesScanUtil(5, 10, Ordering.ASC);
 
     Assert.assertTrue(seriesScanUtil.hasNextFile());
     Assert.assertTrue(seriesScanUtil.hasNextChunk());
@@ -269,7 +270,7 @@ public class SeriesScanLimitOffsetPushDownTest {
 
   @Test
   public void testSkipChunk() throws IllegalPathException, IOException {
-    SeriesScanUtil seriesScanUtil = getSeriesScanUtil(5, 20);
+    SeriesScanUtil seriesScanUtil = getSeriesScanUtil(5, 20, Ordering.ASC);
 
     Assert.assertTrue(seriesScanUtil.hasNextFile());
     Assert.assertTrue(seriesScanUtil.hasNextChunk());
@@ -289,7 +290,7 @@ public class SeriesScanLimitOffsetPushDownTest {
 
   @Test
   public void testSkipPage() throws IllegalPathException, IOException {
-    SeriesScanUtil seriesScanUtil = getSeriesScanUtil(5, 30);
+    SeriesScanUtil seriesScanUtil = getSeriesScanUtil(5, 30, Ordering.ASC);
 
     Assert.assertTrue(seriesScanUtil.hasNextFile());
     Assert.assertTrue(seriesScanUtil.hasNextChunk());
@@ -309,7 +310,7 @@ public class SeriesScanLimitOffsetPushDownTest {
 
   @Test
   public void testSkipPoint1() throws IllegalPathException, IOException {
-    SeriesScanUtil seriesScanUtil = getSeriesScanUtil(10, 45);
+    SeriesScanUtil seriesScanUtil = getSeriesScanUtil(10, 45, Ordering.ASC);
 
     Assert.assertTrue(seriesScanUtil.hasNextFile());
     Assert.assertTrue(seriesScanUtil.hasNextChunk());
@@ -341,7 +342,7 @@ public class SeriesScanLimitOffsetPushDownTest {
 
   @Test
   public void testSkipPoint2() throws IllegalPathException, IOException {
-    SeriesScanUtil seriesScanUtil = getSeriesScanUtil(10, 55);
+    SeriesScanUtil seriesScanUtil = getSeriesScanUtil(10, 55, Ordering.ASC);
 
     Assert.assertTrue(seriesScanUtil.hasNextFile());
     Assert.assertTrue(seriesScanUtil.hasNextChunk());
@@ -365,4 +366,72 @@ public class SeriesScanLimitOffsetPushDownTest {
     Assert.assertFalse(seriesScanUtil.hasNextChunk());
     Assert.assertFalse(seriesScanUtil.hasNextFile());
   }
+
+  @Test
+  public void testSkipPointDesc1() throws IllegalPathException, IOException {
+    SeriesScanUtil seriesScanUtil = getSeriesScanUtil(10, 5, Ordering.DESC);
+
+    Assert.assertTrue(seriesScanUtil.hasNextFile());
+    Assert.assertTrue(seriesScanUtil.hasNextChunk());
+    Assert.assertTrue(seriesScanUtil.hasNextPage());
+
+    TsBlock tsBlock = seriesScanUtil.nextPage();
+    Assert.assertEquals(5, tsBlock.getPositionCount());
+
+    long expectedTime = 64;
+    for (int i = 0, size = tsBlock.getPositionCount(); i < size; i++) {
+      Assert.assertEquals(expectedTime--, tsBlock.getTimeByIndex(i));
+    }
+
+    Assert.assertTrue(seriesScanUtil.hasNextPage());
+    tsBlock = seriesScanUtil.nextPage();
+    Assert.assertEquals(5, tsBlock.getPositionCount());
+
+    expectedTime = 59;
+    for (int i = 0, size = tsBlock.getPositionCount(); i < size; i++) {
+      Assert.assertEquals(expectedTime--, tsBlock.getTimeByIndex(i));
+    }
+
+    Assert.assertFalse(seriesScanUtil.hasNextPage());
+    Assert.assertFalse(seriesScanUtil.hasNextChunk());
+    Assert.assertFalse(seriesScanUtil.hasNextFile());
+  }
+
+  @Test
+  public void testSkipPointDesc2() throws IllegalPathException, IOException {
+    SeriesScanUtil seriesScanUtil = getSeriesScanUtil(10, 25, Ordering.DESC);
+
+    Assert.assertTrue(seriesScanUtil.hasNextFile());
+    Assert.assertTrue(seriesScanUtil.hasNextChunk());
+    Assert.assertTrue(seriesScanUtil.hasNextPage());
+
+    TsBlock tsBlock = seriesScanUtil.nextPage();
+    Assert.assertEquals(0, tsBlock.getPositionCount());
+
+    Assert.assertFalse(seriesScanUtil.hasNextPage());
+
+    Assert.assertTrue(seriesScanUtil.hasNextChunk());
+    Assert.assertTrue(seriesScanUtil.hasNextPage());
+
+    tsBlock = seriesScanUtil.nextPage();
+    Assert.assertEquals(5, tsBlock.getPositionCount());
+
+    long expectedTime = 44;
+    for (int i = 0, size = tsBlock.getPositionCount(); i < size; i++) {
+      Assert.assertEquals(expectedTime--, tsBlock.getTimeByIndex(i));
+    }
+
+    Assert.assertTrue(seriesScanUtil.hasNextPage());
+    tsBlock = seriesScanUtil.nextPage();
+    Assert.assertEquals(5, tsBlock.getPositionCount());
+
+    expectedTime = 39;
+    for (int i = 0, size = tsBlock.getPositionCount(); i < size; i++) {
+      Assert.assertEquals(expectedTime--, tsBlock.getTimeByIndex(i));
+    }
+
+    Assert.assertFalse(seriesScanUtil.hasNextPage());
+    Assert.assertFalse(seriesScanUtil.hasNextChunk());
+    Assert.assertFalse(seriesScanUtil.hasNextFile());
+  }
 }
diff --git a/tsfile/src/main/java/org/apache/iotdb/tsfile/read/reader/series/PaginationController.java b/tsfile/src/main/java/org/apache/iotdb/tsfile/read/reader/series/PaginationController.java
index a35867645a..96e7075f19 100644
--- a/tsfile/src/main/java/org/apache/iotdb/tsfile/read/reader/series/PaginationController.java
+++ b/tsfile/src/main/java/org/apache/iotdb/tsfile/read/reader/series/PaginationController.java
@@ -19,6 +19,8 @@
 
 package org.apache.iotdb.tsfile.read.reader.series;
 
+import org.apache.iotdb.tsfile.read.common.block.TsBlock;
+
 public class PaginationController {
 
   public static final PaginationController UNLIMITED_PAGINATION_CONTROLLER =
@@ -63,4 +65,25 @@ public class PaginationController {
       curLimit--;
     }
   }
+
+  public void consumeLimit(long rowCount) {
+    if (hasLimit) {
+      curLimit -= rowCount;
+    }
+  }
+
+  public TsBlock applyTsBlock(TsBlock resultTsBlock) {
+
+    int fromIndex = 0, length = resultTsBlock.getPositionCount();
+    if (hasCurOffset()) {
+      fromIndex = (int) Math.min(curOffset, length);
+      length -= fromIndex;
+      consumeOffset(fromIndex);
+    }
+    if (hasLimit && curLimit > 0) {
+      length = (int) Math.min(curLimit, length);
+      consumeLimit(length);
+    }
+    return resultTsBlock.getRegion(fromIndex, length);
+  }
 }