This is an automated email from the ASF dual-hosted git repository. jackietien pushed a commit to branch LimitPushDownBug in repository https://gitbox.apache.org/repos/asf/iotdb.git
commit ca076e8bfe0f808322b81b35d2d5ebb9f9f7daf0 Author: JackieTien97 <[email protected]> AuthorDate: Thu Mar 23 20:17:36 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); + } }
