You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by ch...@apache.org on 2022/10/01 13:29:31 UTC
[phoenix] branch 5.1 updated: PHOENIX-6798 Eliminate unnecessary reversed scan for AggregatePlan (#1511)
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 <ch...@apache.org>
AuthorDate: Sat Oct 1 21:29:25 2022 +0800
PHOENIX-6798 Eliminate unnecessary reversed scan for AggregatePlan (#1511)
Signed-off-by: Geoffrey Jacoby <gj...@apache.org>
---
.../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();
+ }
+ }
}