This is an automated email from the ASF dual-hosted git repository.
chenglei 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 228e194e42 PHOENIX-6798 Eliminate unnecessary reversed scan for
AggregatePlan (#1511)
228e194e42 is described below
commit 228e194e4279671f6de6e92937978042e7423bca
Author: chenglei <[email protected]>
AuthorDate: Sat Oct 1 21:29:25 2022 +0800
PHOENIX-6798 Eliminate unnecessary reversed scan for AggregatePlan (#1511)
Signed-off-by: Geoffrey Jacoby <[email protected]>
---
.../org/apache/phoenix/execute/AggregatePlan.java | 14 +++++++
.../org/apache/phoenix/execute/BaseQueryPlan.java | 19 ++++++----
.../apache/phoenix/compile/QueryCompilerTest.java | 43 +++++++++++++++++++++-
3 files changed, 66 insertions(+), 10 deletions(-)
diff --git
a/phoenix-core/src/main/java/org/apache/phoenix/execute/AggregatePlan.java
b/phoenix-core/src/main/java/org/apache/phoenix/execute/AggregatePlan.java
index 5d4d1d3ef5..427b6c0ecb 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/execute/AggregatePlan.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/execute/AggregatePlan.java
@@ -360,4 +360,18 @@ public class AggregatePlan extends BaseQueryPlan {
public List<OrderBy> getOutputOrderBys() {
return OrderBy.wrapForOutputOrderBys(this.actualOutputOrderBy);
}
+
+ @Override
+ protected void setScanReversedWhenOrderByIsReversed(Scan scan) {
+ /**
+ * For {@link AggregatePlan}, when {@link GroupBy#isOrderPreserving}
is false, we have no
+ * need to set the scan as reversed scan because we have to
hash-aggregate the scanned
+ * results from HBase in RegionServer Coprocessor before sending them
to client, only when
+ * {@link GroupBy#isOrderPreserving} is true and we depend on the
original HBase scanned
+ * order to get the query result, we need to set the scan as reversed
scan.
+ */
+ if (this.groupBy.isOrderPreserving()) {
+ super.setScanReversedWhenOrderByIsReversed(scan);
+ }
+ }
}
diff --git
a/phoenix-core/src/main/java/org/apache/phoenix/execute/BaseQueryPlan.java
b/phoenix-core/src/main/java/org/apache/phoenix/execute/BaseQueryPlan.java
index 23ef64e86a..9847ea99c9 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/execute/BaseQueryPlan.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/execute/BaseQueryPlan.java
@@ -28,10 +28,6 @@ import java.util.Map;
import java.util.Set;
import org.apache.commons.lang3.tuple.Pair;
-import org.apache.phoenix.compile.ExplainPlanAttributes;
-import org.apache.phoenix.compile.ExplainPlanAttributes
- .ExplainPlanAttributesBuilder;
-import org.apache.phoenix.thirdparty.com.google.common.base.Optional;
import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.client.Scan;
import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
@@ -41,6 +37,9 @@ import org.apache.hadoop.io.WritableUtils;
import org.apache.htrace.TraceScope;
import org.apache.phoenix.cache.ServerCacheClient.ServerCache;
import org.apache.phoenix.compile.ExplainPlan;
+import org.apache.phoenix.compile.ExplainPlanAttributes;
+import org.apache.phoenix.compile.ExplainPlanAttributes
+ .ExplainPlanAttributesBuilder;
import org.apache.phoenix.compile.FromCompiler;
import org.apache.phoenix.compile.GroupByCompiler.GroupBy;
import org.apache.phoenix.compile.OrderByCompiler.OrderBy;
@@ -77,6 +76,9 @@ import
org.apache.phoenix.schema.PTable.ImmutableStorageScheme;
import org.apache.phoenix.schema.PTable.IndexType;
import org.apache.phoenix.schema.PTableType;
import org.apache.phoenix.schema.TableRef;
+import org.apache.phoenix.thirdparty.com.google.common.base.Optional;
+import org.apache.phoenix.thirdparty.com.google.common.collect.ImmutableSet;
+import org.apache.phoenix.thirdparty.com.google.common.collect.Lists;
import org.apache.phoenix.trace.TracingIterator;
import org.apache.phoenix.trace.util.Tracing;
import org.apache.phoenix.util.ByteUtil;
@@ -87,9 +89,6 @@ import org.apache.phoenix.util.ScanUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import org.apache.phoenix.thirdparty.com.google.common.collect.ImmutableSet;
-import org.apache.phoenix.thirdparty.com.google.common.collect.Lists;
-
/**
@@ -237,6 +236,10 @@ public abstract class BaseQueryPlan implements QueryPlan {
return wrappedIterator;
}
+ protected void setScanReversedWhenOrderByIsReversed(Scan scan) {
+ ScanUtil.setReversed(scan);
+ }
+
public final ResultIterator iterator(final
Map<ImmutableBytesPtr,ServerCache> caches,
ParallelScanGrouper scanGrouper, Scan scan) throws SQLException {
if (scan == null) {
@@ -271,7 +274,7 @@ public abstract class BaseQueryPlan implements QueryPlan {
}
if (OrderBy.REV_ROW_KEY_ORDER_BY.equals(orderBy)) {
- ScanUtil.setReversed(scan);
+ setScanReversedWhenOrderByIsReversed(scan);
// After HBASE-16296 is resolved, we no longer need to set
// scan caching
}
diff --git
a/phoenix-core/src/test/java/org/apache/phoenix/compile/QueryCompilerTest.java
b/phoenix-core/src/test/java/org/apache/phoenix/compile/QueryCompilerTest.java
index 6714cee637..0a8e783725 100644
---
a/phoenix-core/src/test/java/org/apache/phoenix/compile/QueryCompilerTest.java
+++
b/phoenix-core/src/test/java/org/apache/phoenix/compile/QueryCompilerTest.java
@@ -99,6 +99,7 @@ import org.apache.phoenix.schema.types.PChar;
import org.apache.phoenix.schema.types.PDecimal;
import org.apache.phoenix.schema.types.PInteger;
import org.apache.phoenix.schema.types.PVarchar;
+import org.apache.phoenix.thirdparty.com.google.common.collect.Lists;
import org.apache.phoenix.util.ByteUtil;
import org.apache.phoenix.util.EnvironmentEdgeManager;
import org.apache.phoenix.util.PhoenixRuntime;
@@ -111,8 +112,6 @@ import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
-import org.apache.phoenix.thirdparty.com.google.common.collect.Lists;
-
/**
@@ -6902,4 +6901,44 @@ public class QueryCompilerTest extends
BaseConnectionlessQueryTest {
}
}
+ @Test
+ public void testEliminateUnnecessaryReversedScanBug6798() throws Exception
{
+ Connection conn = null;
+ try {
+ conn = DriverManager.getConnection(getUrl());
+ String tableName = generateUniqueName();
+
+ String sql =
+ "create table " + tableName + "(group_id integer not null,
"
+ + " keyword varchar not null, " + " cost integer, "
+ + " CONSTRAINT TEST_PK PRIMARY KEY
(group_id,keyword)) ";
+ conn.createStatement().execute(sql);
+
+ /**
+ * Test {@link GroupBy#isOrderPreserving} is false and {@link
OrderBy} is reversed.
+ */
+ sql =
+ "select keyword,sum(cost) from " + tableName
+ + " group by keyword order by keyword desc";
+ QueryPlan queryPlan = TestUtil.getOptimizeQueryPlan(conn, sql);
+ Scan scan = queryPlan.getContext().getScan();
+ assertTrue(!queryPlan.getGroupBy().isOrderPreserving());
+ assertTrue(queryPlan.getOrderBy() == OrderBy.REV_ROW_KEY_ORDER_BY);
+ assertTrue(!ScanUtil.isReversed(scan));
+
+ /**
+ * Test {@link GroupBy#isOrderPreserving} is true and {@link
OrderBy} is reversed.
+ */
+ sql =
+ "select keyword,sum(cost) from " + tableName
+ + " group by group_id,keyword order by group_id
desc,keyword desc";
+ queryPlan = TestUtil.getOptimizeQueryPlan(conn, sql);
+ scan = queryPlan.getContext().getScan();
+ assertTrue(queryPlan.getGroupBy().isOrderPreserving());
+ assertTrue(queryPlan.getOrderBy() == OrderBy.REV_ROW_KEY_ORDER_BY);
+ assertTrue(ScanUtil.isReversed(scan));
+ } finally {
+ conn.close();
+ }
+ }
}
