This is an automated email from the ASF dual-hosted git repository. hui pushed a commit to branch lmh/fixLimitBug1.1 in repository https://gitbox.apache.org/repos/asf/iotdb.git
commit 1226af97c9a3a8f200c11c32194a20468959632f Author: liuminghui233 <[email protected]> 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()); + } }
