You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by ya...@apache.org on 2021/03/12 06:06:29 UTC
[phoenix] 07/07: PHOENIX-6408 LIMIT on local index query with
uncovered columns in the WHERE returns wrong result.
This is an automated email from the ASF dual-hosted git repository.
yanxinyi pushed a commit to branch 4.16
in repository https://gitbox.apache.org/repos/asf/phoenix.git
commit c619d8e793fc4e13ca634dbf05bc418f0e0f9517
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 a3f3ed1..2de2c94 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
@@ -152,6 +152,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 7bb84f7..b4c204b 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
@@ -98,6 +98,7 @@ abstract public class BaseScannerRegionObserver extends BaseRegionObserver {
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 9190a33..882cfaa 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
@@ -281,7 +281,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 6be45d3..414f294 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
@@ -132,6 +132,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
@@ -148,6 +149,10 @@ public abstract class RegionScannerFactory {
throw new RuntimeException(io);
}
}
+ byte[] limitBytes = scan.getAttribute(BaseScannerRegionObserver.LOCAL_INDEX_LIMIT);
+ if (limitBytes != null) {
+ extraLimit = Bytes.toLong(limitBytes);
+ }
}
}
@@ -259,6 +264,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) {