You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iotdb.apache.org by hu...@apache.org on 2023/03/22 14:56:54 UTC

[iotdb] branch lmh/fixLimitBug updated: fix bug & add UT

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

hui pushed a commit to branch lmh/fixLimitBug
in repository https://gitbox.apache.org/repos/asf/iotdb.git


The following commit(s) were added to refs/heads/lmh/fixLimitBug by this push:
     new d22f4d06dd fix bug & add UT
d22f4d06dd is described below

commit d22f4d06ddaf3bfb117cab6f806747b1f26dfe3e
Author: liuminghui233 <54...@qq.com>
AuthorDate: Wed Mar 22 22:56:02 2023 +0800

    fix bug & add UT
---
 .../execution/operator/source/SeriesScanUtil.java  | 32 ++++-----
 .../series/SeriesScanLimitOffsetPushDownTest.java  | 83 ++++++++++++++++++++--
 2 files changed, 88 insertions(+), 27 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 0dbd911adf..be9c590aa5 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
@@ -637,7 +637,7 @@ public class SeriesScanUtil {
         firstPageReader.setFilter(queryFilter);
       }
       firstPageReader.setLimitOffset(paginationController);
-      TsBlock tsBlock = firstPageReader.getAllSatisfiedPageData(orderUtils.getAscending());
+      TsBlock tsBlock = firstPageReader.getAllSatisfiedPageData(orderUtils.getAscending(), true);
       firstPageReader = null;
 
       return tsBlock;
@@ -719,12 +719,7 @@ public class SeriesScanUtil {
                   timeValuePair.getTimestamp(), firstPageReader.getStatistics())) {
                 // current timeValuePair is overlapped with firstPageReader, add it to merged reader
                 // and update endTime to the max end time
-                mergeReader.addReader(
-                    getPointReader(
-                        firstPageReader.getAllSatisfiedPageData(orderUtils.getAscending())),
-                    firstPageReader.version,
-                    orderUtils.getOverlapCheckTime(firstPageReader.getStatistics()),
-                    context);
+                putPageReaderToMergeReader(firstPageReader);
                 currentPageEndPointTime =
                     updateEndPointTime(currentPageEndPointTime, firstPageReader);
                 firstPageReader = null;
@@ -745,11 +740,7 @@ public class SeriesScanUtil {
               } else if (orderUtils.isOverlapped(
                   timeValuePair.getTimestamp(), seqPageReaders.get(0).getStatistics())) {
                 VersionPageReader pageReader = seqPageReaders.remove(0);
-                mergeReader.addReader(
-                    getPointReader(pageReader.getAllSatisfiedPageData(orderUtils.getAscending())),
-                    pageReader.version,
-                    orderUtils.getOverlapCheckTime(pageReader.getStatistics()),
-                    context);
+                putPageReaderToMergeReader(pageReader);
                 currentPageEndPointTime = updateEndPointTime(currentPageEndPointTime, pageReader);
               }
             }
@@ -945,7 +936,7 @@ public class SeriesScanUtil {
 
   private void putPageReaderToMergeReader(VersionPageReader pageReader) throws IOException {
     mergeReader.addReader(
-        getPointReader(pageReader.getAllSatisfiedPageData(orderUtils.getAscending())),
+        getPointReader(pageReader.getAllSatisfiedPageData(orderUtils.getAscending(), false)),
         pageReader.version,
         orderUtils.getOverlapCheckTime(pageReader.getStatistics()),
         context);
@@ -1147,19 +1138,20 @@ public class SeriesScanUtil {
       return ((IAlignedPageReader) data).getTimeStatistics();
     }
 
-    TsBlock getAllSatisfiedPageData(boolean ascending) throws IOException {
+    TsBlock getAllSatisfiedPageData(boolean ascending, boolean isSeq) throws IOException {
       long startTime = System.nanoTime();
       try {
-        paginationController.setEnable(ascending);
+        paginationController.setEnable(isSeq && ascending);
         TsBlock tsBlock = data.getAllSatisfiedData();
-        paginationController.setEnable(true);
 
-        if (ascending) {
-          return tsBlock;
-        } else {
+        if (!ascending) {
           tsBlock.reverse();
-          return paginationController.applyTsBlock(tsBlock);
+
+          paginationController.setEnable(isSeq);
+          tsBlock = paginationController.applyTsBlock(tsBlock);
+          paginationController.setEnable(true);
         }
+        return tsBlock;
       } finally {
         QUERY_METRICS.recordSeriesScanCost(
             isAligned
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());
+  }
 }