This is an automated email from the ASF dual-hosted git repository. larsh pushed a commit to branch 5.1 in repository https://gitbox.apache.org/repos/asf/phoenix.git
The following commit(s) were added to refs/heads/5.1 by this push: new 148800c PHOENIX-6408 LIMIT on local index query with uncovered columns in the WHERE returns wrong result. 148800c is described below commit 148800c70b44147c28a2014a9b88ecda7d567ea8 Author: Lars <la...@apache.org> AuthorDate: Tue Mar 9 19:36:48 2021 -0800 PHOENIX-6408 LIMIT on local index query with uncovered columns in the WHERE returns wrong result. --- .../org/apache/phoenix/end2end/index/LocalIndexIT.java | 17 +++++++++++++++++ .../phoenix/coprocessor/BaseScannerRegionObserver.java | 1 + .../org/apache/phoenix/iterate/BaseResultIterators.java | 9 ++++++++- .../java/org/apache/phoenix/iterate/ExplainTable.java | 11 ++++++++++- .../apache/phoenix/iterate/RegionScannerFactory.java | 8 ++++++++ 5 files changed, 44 insertions(+), 2 deletions(-) diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/index/LocalIndexIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/index/LocalIndexIT.java index 90d172e..0a071dd 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/index/LocalIndexIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/index/LocalIndexIT.java @@ -153,6 +153,23 @@ public class LocalIndexIT extends BaseLocalIndexIT { rs.next(); assertEquals(6, rs.getInt(1)); rs.close(); + + rs = conn.createStatement().executeQuery("SELECT * FROM "+tableName+" WHERE v1 > 0 AND v3 > 5 LIMIT 2"); + assertTrue(rs.next()); + assertTrue(rs.next()); + assertFalse(rs.next()); + rs.close(); + + rs = conn.createStatement().executeQuery("SELECT * FROM "+tableName+" WHERE v1 > 0 AND v3 > 5 LIMIT 1"); + assertTrue(rs.next()); + assertFalse(rs.next()); + rs.close(); + + rs = conn.createStatement().executeQuery("SELECT * FROM "+tableName+" WHERE v3 > 5 ORDER BY v1 LIMIT 2"); + assertTrue(rs.next()); + assertTrue(rs.next()); + assertFalse(rs.next()); + rs.close(); } @Test diff --git a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/BaseScannerRegionObserver.java b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/BaseScannerRegionObserver.java index 1244276..c7dfe9a 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/BaseScannerRegionObserver.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/BaseScannerRegionObserver.java @@ -81,6 +81,7 @@ abstract public class BaseScannerRegionObserver extends CompatBaseScannerRegionO public static final String INDEX_REBUILD_DISABLE_LOGGING_BEYOND_MAXLOOKBACK_AGE = "_IndexRebuildDisableLoggingBeyondMaxLookbackAge"; public static final String LOCAL_INDEX_FILTER = "_LocalIndexFilter"; + public static final String LOCAL_INDEX_LIMIT = "_LocalIndexLimit"; public static final String LOCAL_INDEX_FILTER_STR = "_LocalIndexFilterStr"; /* diff --git a/phoenix-core/src/main/java/org/apache/phoenix/iterate/BaseResultIterators.java b/phoenix-core/src/main/java/org/apache/phoenix/iterate/BaseResultIterators.java index ac8cce4..68c00a0 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/iterate/BaseResultIterators.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/iterate/BaseResultIterators.java @@ -280,7 +280,14 @@ public abstract class BaseResultIterators extends ExplainTable implements Result } if (perScanLimit != null) { - ScanUtil.andFilterAtEnd(scan, new PageFilter(perScanLimit)); + if (scan.getAttribute(BaseScannerRegionObserver.LOCAL_INDEX_FILTER) == null) { + ScanUtil.andFilterAtEnd(scan, new PageFilter(perScanLimit)); + } else { + // if we have a local index filter and a limit, handle the limit after the filter + // we cast the limit to a long even though it passed as an Integer so that + // if we need extend this in the future the serialization is unchanged + scan.setAttribute(BaseScannerRegionObserver.LOCAL_INDEX_LIMIT, Bytes.toBytes((long)perScanLimit)); + } } if(offset!=null){ diff --git a/phoenix-core/src/main/java/org/apache/phoenix/iterate/ExplainTable.java b/phoenix-core/src/main/java/org/apache/phoenix/iterate/ExplainTable.java index cf5e021..163364c 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/iterate/ExplainTable.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/iterate/ExplainTable.java @@ -226,8 +226,17 @@ public abstract class ExplainTable { if (offset != null) { planSteps.add(" SERVER OFFSET " + offset); } + Long limit = null; if (pageFilter != null) { - planSteps.add(" SERVER " + pageFilter.getPageSize() + " ROW LIMIT"); + limit = pageFilter.getPageSize(); + } else { + byte[] limitBytes = scan.getAttribute(BaseScannerRegionObserver.LOCAL_INDEX_LIMIT); + if (limitBytes != null) { + limit = Bytes.toLong(limitBytes); + } + } + if (limit != null) { + planSteps.add(" SERVER " + limit + " ROW LIMIT"); } if (explainPlanAttributesBuilder != null) { explainPlanAttributesBuilder.setServerOffset(offset); diff --git a/phoenix-core/src/main/java/org/apache/phoenix/iterate/RegionScannerFactory.java b/phoenix-core/src/main/java/org/apache/phoenix/iterate/RegionScannerFactory.java index 20f68cb..8d70ab0 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/iterate/RegionScannerFactory.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/iterate/RegionScannerFactory.java @@ -130,6 +130,7 @@ public abstract class RegionScannerFactory { private boolean useNewValueColumnQualifier = EncodedColumnsUtil.useNewValueColumnQualifier(scan); final long pageSizeMs = getPageSizeMsForRegionScanner(scan); Expression extraWhere = null; + long extraLimit = -1; { // for local indexes construct the row filter for uncovered columns if it exists @@ -146,6 +147,10 @@ public abstract class RegionScannerFactory { throw new RuntimeException(io); } } + byte[] limitBytes = scan.getAttribute(BaseScannerRegionObserver.LOCAL_INDEX_LIMIT); + if (limitBytes != null) { + extraLimit = Bytes.toLong(limitBytes); + } } } @@ -257,6 +262,9 @@ public abstract class RegionScannerFactory { result.add(arrayElementCell); } } + if (extraLimit >= 0 && --extraLimit == 0) { + return false; + } // There is a scanattribute set to retrieve the specific array element return next; } catch (Throwable t) {