You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by pb...@apache.org on 2019/05/28 22:53:05 UTC
[phoenix] 15/18: PHOENIX-4820 Optimize OrderBy for
ClientAggregatePlan
This is an automated email from the ASF dual-hosted git repository.
pboado pushed a commit to branch 4.x-cdh5.16
in repository https://gitbox.apache.org/repos/asf/phoenix.git
commit 460da6136a75245b119d4f0393e08e9f61d579d5
Author: chenglei <ch...@apache.org>
AuthorDate: Sat Jan 5 01:58:00 2019 +0000
PHOENIX-4820 Optimize OrderBy for ClientAggregatePlan
---
.../org/apache/phoenix/end2end/AggregateIT.java | 104 +++++++
.../apache/phoenix/compile/GroupByCompiler.java | 8 +-
.../apache/phoenix/compile/OrderByCompiler.java | 18 +-
.../phoenix/compile/OrderPreservingTracker.java | 53 ++--
.../org/apache/phoenix/compile/QueryCompiler.java | 12 +-
.../org/apache/phoenix/compile/RowProjector.java | 15 +-
.../phoenix/expression/BaseCompoundExpression.java | 11 +-
.../apache/phoenix/expression/BaseExpression.java | 11 +
.../phoenix/expression/BaseSingleExpression.java | 5 +
.../phoenix/expression/DelegateExpression.java | 5 +
.../org/apache/phoenix/expression/Expression.java | 6 +
.../expression/ProjectedColumnExpression.java | 8 +-
.../expression/function/RandomFunction.java | 5 +
.../expression/visitor/CloneExpressionVisitor.java | 6 +-
.../CloneNonDeterministicExpressionVisitor.java | 31 --
.../org/apache/phoenix/util/ExpressionUtil.java | 160 +++++++++-
.../apache/phoenix/compile/QueryCompilerTest.java | 324 +++++++++++++++++++++
.../expression/ArithmeticOperationTest.java | 8 +-
18 files changed, 705 insertions(+), 85 deletions(-)
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/AggregateIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/AggregateIT.java
index 8916d4d..d52025e 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/AggregateIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/AggregateIT.java
@@ -227,5 +227,109 @@ public class AggregateIT extends BaseAggregateIT {
assertEquals(4, rs.getLong(1));
}
}
+
+ @Test
+ public void testOrderByOptimizeForClientAggregatePlanBug4820() throws Exception {
+ doTestOrderByOptimizeForClientAggregatePlanBug4820(false,false);
+ doTestOrderByOptimizeForClientAggregatePlanBug4820(false,true);
+ doTestOrderByOptimizeForClientAggregatePlanBug4820(true,false);
+ doTestOrderByOptimizeForClientAggregatePlanBug4820(true,true);
+ }
+
+ private void doTestOrderByOptimizeForClientAggregatePlanBug4820(boolean desc ,boolean salted) throws Exception {
+ Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+ Connection conn = null;
+ try {
+ conn = DriverManager.getConnection(getUrl(), props);
+ String tableName = generateUniqueName();
+ String sql = "create table " + tableName + "( "+
+ " pk1 varchar not null , " +
+ " pk2 varchar not null, " +
+ " pk3 varchar not null," +
+ " v1 varchar, " +
+ " v2 varchar, " +
+ " CONSTRAINT TEST_PK PRIMARY KEY ( "+
+ "pk1 "+(desc ? "desc" : "")+", "+
+ "pk2 "+(desc ? "desc" : "")+", "+
+ "pk3 "+(desc ? "desc" : "")+
+ " )) "+(salted ? "SALT_BUCKETS =4" : "split on('b')");
+ conn.createStatement().execute(sql);
+
+ conn.createStatement().execute("UPSERT INTO "+tableName+" VALUES ('a11','a12','a13','a14','a15')");
+ conn.createStatement().execute("UPSERT INTO "+tableName+" VALUES ('a21','a22','a23','a24','a25')");
+ conn.createStatement().execute("UPSERT INTO "+tableName+" VALUES ('a31','a32','a33','a34','a35')");
+ conn.createStatement().execute("UPSERT INTO "+tableName+" VALUES ('b11','b12','b13','b14','b15')");
+ conn.createStatement().execute("UPSERT INTO "+tableName+" VALUES ('b21','b22','b23','b24','b25')");
+ conn.createStatement().execute("UPSERT INTO "+tableName+" VALUES ('b31','b32','b33','b34','b35')");
+ conn.commit();
+
+ sql = "select a.ak3 "+
+ "from (select pk1 ak1,pk2 ak2,pk3 ak3, substr(v1,1,1) av1,substr(v2,1,1) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "group by a.ak3,a.av1 order by a.ak3 desc,a.av1";
+ ResultSet rs = conn.prepareStatement(sql).executeQuery();
+ assertResultSet(rs, new Object[][]{{"b33"},{"b23"},{"b13"},{"a33"},{"a23"},{"a13"}});
+
+ sql = "select a.ak3 "+
+ "from (select pk1 ak1,pk2 ak2,pk3 ak3, substr(v1,1,1) av1,substr(v2,1,1) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "group by a.ak3,a.av1 order by a.ak3,a.av1";
+ rs = conn.prepareStatement(sql).executeQuery();
+ assertResultSet(rs, new Object[][]{{"a13"},{"a23"},{"a33"},{"b13"},{"b23"},{"b33"}});
+
+ sql = "select a.ak3 "+
+ "from (select pk1 ak1,pk2 ak2,pk3 ak3,substr(v1,1,1) av1,substr(v2,1,1) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where a.av1 = 'a' group by a.av1,a.ak3 order by a.ak3 desc";
+ rs = conn.prepareStatement(sql).executeQuery();
+ assertResultSet(rs, new Object[][]{{"a33"},{"a23"},{"a13"}});
+
+ sql = "select a.ak3 "+
+ "from (select pk1 ak1,pk2 ak2,pk3 ak3,substr(v1,1,1) av1,substr(v2,1,1) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where a.av1 = 'a' group by a.av1,a.ak3 order by a.ak3";
+ rs = conn.prepareStatement(sql).executeQuery();
+ assertResultSet(rs, new Object[][]{{"a13"},{"a23"},{"a33"}});
+
+ sql = "select a.ak3 "+
+ "from (select pk1 ak1,pk2 ak2,pk3 ak3,substr(v1,1,1) av1,substr(v2,1,1) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where a.av1 = 'b' and a.av2= 'b' group by CASE WHEN a.av1 > a.av2 THEN a.av1 ELSE a.av2 END,a.ak3,a.ak2 order by a.ak3 desc,a.ak2 desc";
+ rs = conn.prepareStatement(sql).executeQuery();
+ assertResultSet(rs, new Object[][]{{"b33"},{"b23"},{"b13"}});
+
+ sql = "select a.ak3 "+
+ "from (select pk1 ak1,pk2 ak2,pk3 ak3,substr(v1,1,1) av1,substr(v2,1,1) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where a.av1 = 'b' and a.av2= 'b' group by CASE WHEN a.av1 > a.av2 THEN a.av1 ELSE a.av2 END,a.ak3,a.ak2 order by a.ak3,a.ak2 desc";
+ rs = conn.prepareStatement(sql).executeQuery();
+ assertResultSet(rs, new Object[][]{{"b13"},{"b23"},{"b33"}});
+
+ tableName = generateUniqueName();
+ sql = "create table " + tableName + "( "+
+ " pk1 double not null , " +
+ " pk2 double not null, " +
+ " pk3 double not null," +
+ " v1 varchar, " +
+ " CONSTRAINT TEST_PK PRIMARY KEY ( "+
+ "pk1 "+(desc ? "desc" : "")+", "+
+ "pk2 "+(desc ? "desc" : "")+", "+
+ "pk3 "+(desc ? "desc" : "")+
+ " )) "+(salted ? "SALT_BUCKETS =4" : "split on(2.3)");
+ conn.createStatement().execute(sql);
+ conn.createStatement().execute("UPSERT INTO "+tableName+" VALUES (2.1,2.11,2.12,'e')");
+ conn.createStatement().execute("UPSERT INTO "+tableName+" VALUES (2.2,2.21,2.23,'d')");
+ conn.createStatement().execute("UPSERT INTO "+tableName+" VALUES (2.3,2.31,2.32,'c')");
+ conn.createStatement().execute("UPSERT INTO "+tableName+" VALUES (2.4,2.41,2.42,'b')");
+ conn.createStatement().execute("UPSERT INTO "+tableName+" VALUES (2.5,2.51,2.52,'a')");
+ conn.commit();
+
+ sql = "select a.av1 "+
+ "from (select pk1 ak1,pk2 ak2,pk3 ak3, substr(v1,1,1) av1 from "+tableName+" order by pk1,pk2 limit 10) a "+
+ "where cast(a.ak1 as integer)=2 group by a.ak1,a.av1 order by a.av1";
+ rs = conn.prepareStatement(sql).executeQuery();
+ assertResultSet(rs, new Object[][]{{"a"},{"b"},{"c"},{"d"},{"e"}});
+
+ } finally {
+ if(conn != null) {
+ conn.close();
+ }
+ }
+ }
+
}
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/compile/GroupByCompiler.java b/phoenix-core/src/main/java/org/apache/phoenix/compile/GroupByCompiler.java
index 4777c29..2bdea9a 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/compile/GroupByCompiler.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/compile/GroupByCompiler.java
@@ -143,7 +143,13 @@ public class GroupByCompiler {
boolean isOrderPreserving = this.isOrderPreserving;
int orderPreservingColumnCount = 0;
if (isOrderPreserving) {
- OrderPreservingTracker tracker = new OrderPreservingTracker(context, GroupBy.EMPTY_GROUP_BY, Ordering.UNORDERED, expressions.size(), tupleProjector);
+ OrderPreservingTracker tracker = new OrderPreservingTracker(
+ context,
+ GroupBy.EMPTY_GROUP_BY,
+ Ordering.UNORDERED,
+ expressions.size(),
+ tupleProjector,
+ null);
for (int i = 0; i < expressions.size(); i++) {
Expression expression = expressions.get(i);
tracker.track(expression);
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/compile/OrderByCompiler.java b/phoenix-core/src/main/java/org/apache/phoenix/compile/OrderByCompiler.java
index b83c7a8..3c3f429 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/compile/OrderByCompiler.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/compile/OrderByCompiler.java
@@ -88,7 +88,8 @@ public class OrderByCompiler {
Integer offset,
RowProjector rowProjector,
TupleProjector tupleProjector,
- boolean isInRowKeyOrder) throws SQLException {
+ boolean isInRowKeyOrder,
+ Expression whereExpression) throws SQLException {
List<OrderByNode> orderByNodes = statement.getOrderBy();
if (orderByNodes.isEmpty()) {
return OrderBy.EMPTY_ORDER_BY;
@@ -105,9 +106,22 @@ public class OrderByCompiler {
} else {
compiler = new ExpressionCompiler(context, groupBy);
}
+
+ if(groupBy != GroupBy.EMPTY_GROUP_BY) {
+ //if there is groupBy,the groupBy.expressions are viewed as new rowKey columns,so
+ //tupleProjector and isInRowKeyOrder is cleared
+ tupleProjector = null;
+ isInRowKeyOrder = true;
+ }
// accumulate columns in ORDER BY
OrderPreservingTracker tracker =
- new OrderPreservingTracker(context, groupBy, Ordering.ORDERED, orderByNodes.size(), tupleProjector);
+ new OrderPreservingTracker(
+ context,
+ groupBy,
+ Ordering.ORDERED,
+ orderByNodes.size(),
+ tupleProjector,
+ whereExpression);
LinkedHashSet<OrderByExpression> orderByExpressions = Sets.newLinkedHashSetWithExpectedSize(orderByNodes.size());
for (OrderByNode node : orderByNodes) {
ParseNode parseNode = node.getNode();
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/compile/OrderPreservingTracker.java b/phoenix-core/src/main/java/org/apache/phoenix/compile/OrderPreservingTracker.java
index d1175f6..29b3794 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/compile/OrderPreservingTracker.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/compile/OrderPreservingTracker.java
@@ -18,8 +18,8 @@ import java.util.List;
import org.apache.phoenix.compile.GroupByCompiler.GroupBy;
import org.apache.phoenix.execute.TupleProjector;
import org.apache.phoenix.expression.CoerceExpression;
-import org.apache.phoenix.expression.Determinism;
import org.apache.phoenix.expression.Expression;
+import org.apache.phoenix.expression.KeyValueColumnExpression;
import org.apache.phoenix.expression.LiteralExpression;
import org.apache.phoenix.expression.ProjectedColumnExpression;
import org.apache.phoenix.expression.RowKeyColumnExpression;
@@ -30,6 +30,7 @@ import org.apache.phoenix.expression.visitor.StatelessTraverseAllExpressionVisit
import org.apache.phoenix.expression.visitor.StatelessTraverseNoExpressionVisitor;
import org.apache.phoenix.schema.PTable;
import org.apache.phoenix.schema.SortOrder;
+import org.apache.phoenix.util.ExpressionUtil;
import com.google.common.collect.Iterators;
import com.google.common.collect.Lists;
@@ -76,16 +77,22 @@ public class OrderPreservingTracker {
private final Ordering ordering;
private final int pkPositionOffset;
private final List<Info> orderPreservingInfos;
- private final TupleProjector projector;
private boolean isOrderPreserving = true;
private Boolean isReverse = null;
private int orderPreservingColumnCount = 0;
+ private Expression whereExpression;
public OrderPreservingTracker(StatementContext context, GroupBy groupBy, Ordering ordering, int nNodes) {
- this(context, groupBy, ordering, nNodes, null);
+ this(context, groupBy, ordering, nNodes, null, null);
}
- public OrderPreservingTracker(StatementContext context, GroupBy groupBy, Ordering ordering, int nNodes, TupleProjector projector) {
+ public OrderPreservingTracker(
+ StatementContext context,
+ GroupBy groupBy,
+ Ordering ordering,
+ int nNodes,
+ TupleProjector projector,
+ Expression whereExpression) {
this.context = context;
if (groupBy.isEmpty()) {
PTable table = context.getResolver().getTables().get(0).getTable();
@@ -103,7 +110,7 @@ public class OrderPreservingTracker {
this.visitor = new TrackOrderPreservingExpressionVisitor(projector);
this.orderPreservingInfos = Lists.newArrayListWithExpectedSize(nNodes);
this.ordering = ordering;
- this.projector = projector;
+ this.whereExpression = whereExpression;
}
public void track(Expression node) {
@@ -208,20 +215,14 @@ public class OrderPreservingTracker {
// not by the original row key order of the table (see PHOENIX-3451).
// We check each GROUP BY expression to see if it only references columns that are
// matched by equality constraints, in which case the expression itself would be constant.
- // FIXME: this only recognizes row key columns that are held constant, not all columns.
- // FIXME: we should optimize out any GROUP BY or ORDER BY expression which is deemed to
- // be held constant based on the WHERE clause.
if (!groupBy.isEmpty()) {
for (int pos = startPos; pos < endPos; pos++) {
- IsConstantVisitor visitor = new IsConstantVisitor(this.projector, ranges);
+ IsConstantVisitor visitor = new IsConstantVisitor(ranges, whereExpression);
List<Expression> groupByExpressions = groupBy.getExpressions();
if (pos >= groupByExpressions.size()) { // sanity check - shouldn't be necessary
return false;
}
Expression groupByExpression = groupByExpressions.get(pos);
- if ( groupByExpression.getDeterminism().ordinal() > Determinism.PER_STATEMENT.ordinal() ) {
- return false;
- }
Boolean isConstant = groupByExpression.accept(visitor);
if (!Boolean.TRUE.equals(isConstant)) {
return false;
@@ -248,17 +249,17 @@ public class OrderPreservingTracker {
*
*/
private static class IsConstantVisitor extends StatelessTraverseAllExpressionVisitor<Boolean> {
- private final TupleProjector projector;
private final ScanRanges scanRanges;
+ private final Expression whereExpression;
- public IsConstantVisitor(TupleProjector projector, ScanRanges scanRanges) {
- this.projector = projector;
- this.scanRanges = scanRanges;
- }
+ public IsConstantVisitor(ScanRanges scanRanges, Expression whereExpression) {
+ this.scanRanges = scanRanges;
+ this.whereExpression = whereExpression;
+ }
@Override
public Boolean defaultReturn(Expression node, List<Boolean> returnValues) {
- if (node.getDeterminism().ordinal() > Determinism.PER_STATEMENT.ordinal() ||
+ if (!ExpressionUtil.isContantForStatement(node) ||
returnValues.size() < node.getChildren().size()) {
return Boolean.FALSE;
}
@@ -281,16 +282,12 @@ public class OrderPreservingTracker {
}
@Override
- public Boolean visit(ProjectedColumnExpression node) {
- if (projector == null) {
- return super.visit(node);
- }
- Expression expression = projector.getExpressions()[node.getPosition()];
- // Only look one level down the projection.
- if (expression instanceof ProjectedColumnExpression) {
- return super.visit(node);
- }
- return expression.accept(this);
+ public Boolean visit(KeyValueColumnExpression keyValueColumnExpression) {
+ return ExpressionUtil.isColumnExpressionConstant(keyValueColumnExpression, whereExpression);
+ }
+ @Override
+ public Boolean visit(ProjectedColumnExpression projectedColumnExpression) {
+ return ExpressionUtil.isColumnExpressionConstant(projectedColumnExpression, whereExpression);
}
}
/**
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/compile/QueryCompiler.java b/phoenix-core/src/main/java/org/apache/phoenix/compile/QueryCompiler.java
index 603da0b..6e36158 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/compile/QueryCompiler.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/compile/QueryCompiler.java
@@ -562,8 +562,16 @@ public class QueryCompiler {
groupBy = groupBy.compile(context, innerPlanTupleProjector);
context.setResolver(resolver); // recover resolver
RowProjector projector = ProjectionCompiler.compile(context, select, groupBy, asSubquery ? Collections.<PDatum>emptyList() : targetColumns, where);
- OrderBy orderBy = OrderByCompiler.compile(context, select, groupBy, limit, offset, projector,
- groupBy == GroupBy.EMPTY_GROUP_BY ? innerPlanTupleProjector : null, isInRowKeyOrder);
+ OrderBy orderBy = OrderByCompiler.compile(
+ context,
+ select,
+ groupBy,
+ limit,
+ offset,
+ projector,
+ innerPlanTupleProjector,
+ isInRowKeyOrder,
+ where);
context.getAggregationManager().compile(context, groupBy);
// Final step is to build the query plan
if (!asSubquery) {
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/compile/RowProjector.java b/phoenix-core/src/main/java/org/apache/phoenix/compile/RowProjector.java
index 2123788..8532e0c 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/compile/RowProjector.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/compile/RowProjector.java
@@ -22,9 +22,8 @@ import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
-import org.apache.phoenix.expression.Determinism;
import org.apache.phoenix.expression.Expression;
-import org.apache.phoenix.expression.visitor.CloneNonDeterministicExpressionVisitor;
+import org.apache.phoenix.expression.visitor.CloneExpressionVisitor;
import org.apache.phoenix.schema.ColumnNotFoundException;
import org.apache.phoenix.util.SchemaUtil;
@@ -97,17 +96,17 @@ public class RowProjector {
this.isProjectEmptyKeyValue = isProjectEmptyKeyValue;
this.isProjectAll = isProjectAll;
this.hasUDFs = hasUDFs;
- boolean hasPerInvocationExpression = false;
+ boolean cloneRequired = false;
if (!hasUDFs) {
for (int i = 0; i < this.columnProjectors.size(); i++) {
Expression expression = this.columnProjectors.get(i).getExpression();
- if (expression.getDeterminism() == Determinism.PER_INVOCATION) {
- hasPerInvocationExpression = true;
+ if (expression.isCloneExpression()) {
+ cloneRequired = true;
break;
}
}
}
- this.cloneRequired = hasPerInvocationExpression || hasUDFs;
+ this.cloneRequired = cloneRequired || hasUDFs;
}
public RowProjector cloneIfNecessary() {
@@ -118,8 +117,8 @@ public class RowProjector {
for (int i = 0; i < this.columnProjectors.size(); i++) {
ColumnProjector colProjector = columnProjectors.get(i);
Expression expression = colProjector.getExpression();
- if (expression.getDeterminism() == Determinism.PER_INVOCATION) {
- CloneNonDeterministicExpressionVisitor visitor = new CloneNonDeterministicExpressionVisitor();
+ if (expression.isCloneExpression()) {
+ CloneExpressionVisitor visitor = new CloneExpressionVisitor();
Expression clonedExpression = expression.accept(visitor);
clonedColProjectors.add(new ExpressionProjector(colProjector.getName(),
colProjector.getTableName(),
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/BaseCompoundExpression.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/BaseCompoundExpression.java
index 5fc8361..e26c902 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/expression/BaseCompoundExpression.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/BaseCompoundExpression.java
@@ -35,6 +35,7 @@ public abstract class BaseCompoundExpression extends BaseExpression {
private boolean isStateless;
private Determinism determinism;
private boolean requiresFinalEvaluation;
+ private boolean cloneExpression;
public BaseCompoundExpression() {
init(Collections.<Expression>emptyList());
@@ -49,6 +50,7 @@ public abstract class BaseCompoundExpression extends BaseExpression {
boolean isStateless = true;
boolean isNullable = false;
boolean requiresFinalEvaluation = false;
+ boolean cloneExpression = false;
this.determinism = Determinism.ALWAYS;
for (int i = 0; i < children.size(); i++) {
Expression child = children.get(i);
@@ -56,10 +58,12 @@ public abstract class BaseCompoundExpression extends BaseExpression {
isStateless &= child.isStateless();
this.determinism = this.determinism.combine(child.getDeterminism());
requiresFinalEvaluation |= child.requiresFinalEvaluation();
+ cloneExpression |= child.isCloneExpression();
}
this.isStateless = isStateless;
this.isNullable = isNullable;
this.requiresFinalEvaluation = requiresFinalEvaluation;
+ this.cloneExpression = cloneExpression;
}
@Override
@@ -72,7 +76,12 @@ public abstract class BaseCompoundExpression extends BaseExpression {
public Determinism getDeterminism() {
return determinism;
}
-
+
+ @Override
+ public boolean isCloneExpression() {
+ return this.cloneExpression;
+ }
+
@Override
public boolean isStateless() {
return isStateless;
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/BaseExpression.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/BaseExpression.java
index efdceac..ccb6073 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/expression/BaseExpression.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/BaseExpression.java
@@ -255,4 +255,15 @@ public abstract class BaseExpression implements Expression {
return false;
}
+ @Override
+ public boolean isCloneExpression() {
+ return isCloneExpressionByDeterminism(this);
+ }
+
+ protected static boolean isCloneExpressionByDeterminism(BaseExpression expression) {
+ if(expression.getDeterminism() == Determinism.PER_INVOCATION) {
+ return true;
+ }
+ return false;
+ }
}
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/BaseSingleExpression.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/BaseSingleExpression.java
index fbe8040..c636319 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/expression/BaseSingleExpression.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/BaseSingleExpression.java
@@ -118,4 +118,9 @@ public abstract class BaseSingleExpression extends BaseExpression {
public Determinism getDeterminism() {
return children.get(0).getDeterminism();
}
+
+ @Override
+ public boolean isCloneExpression() {
+ return children.get(0).isCloneExpression();
+ }
}
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/DelegateExpression.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/DelegateExpression.java
index 3ca93dd..fd783c3 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/expression/DelegateExpression.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/DelegateExpression.java
@@ -105,4 +105,9 @@ public class DelegateExpression implements Expression {
return delegate.requiresFinalEvaluation();
}
+ @Override
+ public boolean isCloneExpression() {
+ return delegate.isCloneExpression();
+ }
+
}
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/Expression.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/Expression.java
index aeea0c8..d490ced 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/expression/Expression.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/Expression.java
@@ -88,4 +88,10 @@ public interface Expression extends PDatum, Writable {
* @return
*/
boolean requiresFinalEvaluation();
+
+ /**
+ * Determines if expression needs to be cloned in {@link org.apache.phoenix.compile.RowProjector}
+ * @return
+ */
+ boolean isCloneExpression();
}
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/ProjectedColumnExpression.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/ProjectedColumnExpression.java
index f851efa..e60bf00 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/expression/ProjectedColumnExpression.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/ProjectedColumnExpression.java
@@ -153,12 +153,12 @@ public class ProjectedColumnExpression extends ColumnExpression {
}
@Override
- public Determinism getDeterminism() {
- return Determinism.PER_INVOCATION;
+ public ProjectedColumnExpression clone() {
+ return new ProjectedColumnExpression(this.column, this.columns, this.position, this.displayName);
}
@Override
- public ProjectedColumnExpression clone() {
- return new ProjectedColumnExpression(this.column, this.columns, this.position, this.displayName);
+ public boolean isCloneExpression() {
+ return true;
}
}
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RandomFunction.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RandomFunction.java
index 01a4eed..d9048f4 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RandomFunction.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RandomFunction.java
@@ -118,6 +118,11 @@ public class RandomFunction extends ScalarFunction {
}
@Override
+ public boolean isCloneExpression() {
+ return isCloneExpressionByDeterminism(this);
+ }
+
+ @Override
public boolean isStateless() {
return true;
}
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/visitor/CloneExpressionVisitor.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/visitor/CloneExpressionVisitor.java
index c6d7c9e..b7ea4ab 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/expression/visitor/CloneExpressionVisitor.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/visitor/CloneExpressionVisitor.java
@@ -51,7 +51,7 @@ import org.apache.phoenix.expression.function.ScalarFunction;
import org.apache.phoenix.expression.function.SingleAggregateFunction;
import org.apache.phoenix.expression.function.UDFExpression;
-public abstract class CloneExpressionVisitor extends TraverseAllExpressionVisitor<Expression> {
+public class CloneExpressionVisitor extends TraverseAllExpressionVisitor<Expression> {
public CloneExpressionVisitor() {
}
@@ -215,5 +215,7 @@ public abstract class CloneExpressionVisitor extends TraverseAllExpressionVisito
return isCloneNode(node, l) ? new ArrayElemRefExpression(l) : node;
}
- public abstract boolean isCloneNode(Expression node, List<Expression> children);
+ public boolean isCloneNode(Expression node, List<Expression> children) {
+ return node.isCloneExpression();
+ }
}
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/visitor/CloneNonDeterministicExpressionVisitor.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/visitor/CloneNonDeterministicExpressionVisitor.java
deleted file mode 100644
index 9a56e36..0000000
--- a/phoenix-core/src/main/java/org/apache/phoenix/expression/visitor/CloneNonDeterministicExpressionVisitor.java
+++ /dev/null
@@ -1,31 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.phoenix.expression.visitor;
-
-import java.util.List;
-
-import org.apache.phoenix.expression.Determinism;
-import org.apache.phoenix.expression.Expression;
-
-public class CloneNonDeterministicExpressionVisitor extends CloneExpressionVisitor {
-
- @Override
- public boolean isCloneNode(Expression node, List<Expression> children) {
- return Determinism.PER_INVOCATION.compareTo(node.getDeterminism()) <= 0;
- }
-}
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/util/ExpressionUtil.java b/phoenix-core/src/main/java/org/apache/phoenix/util/ExpressionUtil.java
index 881b0e1..e737721 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/util/ExpressionUtil.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/util/ExpressionUtil.java
@@ -10,12 +10,20 @@
package org.apache.phoenix.util;
import java.sql.SQLException;
+import java.util.Iterator;
import java.util.List;
+import org.apache.hadoop.hbase.filter.CompareFilter.CompareOp;
import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
+import org.apache.phoenix.expression.AndExpression;
+import org.apache.phoenix.expression.ColumnExpression;
+import org.apache.phoenix.expression.ComparisonExpression;
import org.apache.phoenix.expression.Determinism;
import org.apache.phoenix.expression.Expression;
+import org.apache.phoenix.expression.IsNullExpression;
import org.apache.phoenix.expression.LiteralExpression;
+import org.apache.phoenix.expression.visitor.StatelessTraverseAllExpressionVisitor;
+import org.apache.phoenix.expression.visitor.StatelessTraverseNoExpressionVisitor;
import org.apache.phoenix.schema.ColumnRef;
import org.apache.phoenix.schema.PColumn;
import org.apache.phoenix.schema.TableRef;
@@ -27,10 +35,19 @@ public class ExpressionUtil {
}
public static boolean isConstant(Expression expression) {
- return (expression.isStateless() && (expression.getDeterminism() == Determinism.ALWAYS
- || expression.getDeterminism() == Determinism.PER_STATEMENT));
+ return (expression.isStateless() && isContantForStatement(expression));
}
+ /**
+ * this method determines if expression is constant if all children of it are constants.
+ * @param expression
+ * @return
+ */
+ public static boolean isContantForStatement(Expression expression) {
+ return (expression.getDeterminism() == Determinism.ALWAYS
+ || expression.getDeterminism() == Determinism.PER_STATEMENT);
+ }
+
public static LiteralExpression getConstantExpression(Expression expression, ImmutableBytesWritable ptr)
throws SQLException {
Object value = null;
@@ -68,4 +85,143 @@ public class ExpressionUtil {
return false;
}
+ /**
+ * check the whereExpression to see if the columnExpression is constant.
+ * eg. for "where a =3 and b > 9", a is constant,but b is not.
+ * @param columnExpression
+ * @param whereExpression
+ * @return
+ */
+ public static boolean isColumnExpressionConstant(ColumnExpression columnExpression, Expression whereExpression) {
+ if(whereExpression == null) {
+ return false;
+ }
+ IsColumnConstantExpressionVisitor isColumnConstantExpressionVisitor =
+ new IsColumnConstantExpressionVisitor(columnExpression);
+ whereExpression.accept(isColumnConstantExpressionVisitor);
+ return isColumnConstantExpressionVisitor.isConstant();
+ }
+
+ private static class IsColumnConstantExpressionVisitor extends StatelessTraverseNoExpressionVisitor<Void> {
+ private final Expression columnExpression ;
+ private Expression firstRhsConstantExpression = null;
+ private int rhsConstantCount = 0;
+ private boolean isNullExpressionVisited = false;
+
+ public IsColumnConstantExpressionVisitor(Expression columnExpression) {
+ this.columnExpression = columnExpression;
+ }
+ /**
+ * only consider and,for "where a = 3 or b = 9", neither a or b is constant.
+ */
+ @Override
+ public Iterator<Expression> visitEnter(AndExpression andExpression) {
+ if(rhsConstantCount > 1) {
+ return null;
+ }
+ return andExpression.getChildren().iterator();
+ }
+ /**
+ * <pre>
+ * We just consider {@link ComparisonExpression} because:
+ * 1.for {@link InListExpression} as "a in ('2')", the {@link InListExpression} is rewritten to
+ * {@link ComparisonExpression} in {@link InListExpression#create}
+ * 2.for {@link RowValueConstructorExpression} as "(a,b)=(1,2)",{@link RowValueConstructorExpression}
+ * is rewritten to {@link ComparisonExpression} in {@link ComparisonExpression#create}
+ * 3.not consider {@link CoerceExpression}, because for "where cast(a as integer)=2", when a is double,
+ * a is not constant.
+ * </pre>
+ */
+ @Override
+ public Iterator<Expression> visitEnter(ComparisonExpression comparisonExpression) {
+ if(rhsConstantCount > 1) {
+ return null;
+ }
+ if(comparisonExpression.getFilterOp() != CompareOp.EQUAL) {
+ return null;
+ }
+ Expression lhsExpresssion = comparisonExpression.getChildren().get(0);
+ if(!this.columnExpression.equals(lhsExpresssion)) {
+ return null;
+ }
+ Expression rhsExpression = comparisonExpression.getChildren().get(1);
+ if(rhsExpression == null) {
+ return null;
+ }
+ Boolean isConstant = rhsExpression.accept(new IsCompositeLiteralExpressionVisitor());
+ if(isConstant != null && isConstant.booleanValue()) {
+ checkConstantValue(rhsExpression);
+ }
+ return null;
+ }
+
+ public boolean isConstant() {
+ return this.rhsConstantCount == 1;
+ }
+
+ @Override
+ public Iterator<Expression> visitEnter(IsNullExpression isNullExpression) {
+ if(rhsConstantCount > 1) {
+ return null;
+ }
+ if(isNullExpression.isNegate()) {
+ return null;
+ }
+ Expression lhsExpresssion = isNullExpression.getChildren().get(0);
+ if(!this.columnExpression.equals(lhsExpresssion)) {
+ return null;
+ }
+ this.checkConstantValue(null);
+ return null;
+ }
+
+ private void checkConstantValue(Expression rhsExpression) {
+ if(!this.isNullExpressionVisited && this.firstRhsConstantExpression == null) {
+ this.firstRhsConstantExpression = rhsExpression;
+ rhsConstantCount++;
+ if(rhsExpression == null) {
+ this.isNullExpressionVisited = true;
+ }
+ return;
+ }
+
+ if(!isExpressionEquals(this.isNullExpressionVisited ? null : this.firstRhsConstantExpression, rhsExpression)) {
+ rhsConstantCount++;
+ return;
+ }
+ }
+
+ private static boolean isExpressionEquals(Expression oldExpression,Expression newExpression) {
+ if(oldExpression == null) {
+ if(newExpression == null) {
+ return true;
+ }
+ return ExpressionUtil.isNull(newExpression, new ImmutableBytesWritable());
+ }
+ if(newExpression == null) {
+ return ExpressionUtil.isNull(oldExpression, new ImmutableBytesWritable());
+ }
+ return oldExpression.equals(newExpression);
+ }
+ }
+
+ private static class IsCompositeLiteralExpressionVisitor extends StatelessTraverseAllExpressionVisitor<Boolean> {
+ @Override
+ public Boolean defaultReturn(Expression expression, List<Boolean> childResultValues) {
+ if (!ExpressionUtil.isContantForStatement(expression) ||
+ childResultValues.size() < expression.getChildren().size()) {
+ return Boolean.FALSE;
+ }
+ for (Boolean childResultValue : childResultValues) {
+ if (!childResultValue) {
+ return Boolean.FALSE;
+ }
+ }
+ return Boolean.TRUE;
+ }
+ @Override
+ public Boolean visit(LiteralExpression literalExpression) {
+ return Boolean.TRUE;
+ }
+ }
}
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 154dd7a..68954b8 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
@@ -2951,6 +2951,129 @@ public class QueryCompilerTest extends BaseConnectionlessQueryTest {
}
}
}
+
+ @Test
+ public void testOrderPreservingGroupByForNotPkColumns() throws Exception {
+ try (Connection conn= DriverManager.getConnection(getUrl())) {
+ conn.createStatement().execute("CREATE TABLE test (\n" +
+ " pk1 varchar, \n" +
+ " pk2 varchar, \n" +
+ " pk3 varchar, \n" +
+ " pk4 varchar, \n" +
+ " v1 varchar, \n" +
+ " v2 varchar,\n" +
+ " CONSTRAINT pk PRIMARY KEY (\n" +
+ " pk1,\n" +
+ " pk2,\n" +
+ " pk3,\n" +
+ " pk4\n" +
+ " )\n" +
+ " )");
+ String[] queries = new String[] {
+ "SELECT pk3 FROM test WHERE v2 = 'a' GROUP BY substr(v2,0,1),pk3 ORDER BY pk3",
+ "SELECT pk3 FROM test WHERE pk1 = 'c' and v2 = substr('abc',1,1) GROUP BY v2,pk3 ORDER BY pk3",
+ "SELECT pk3 FROM test WHERE v1 = 'a' and v2 = 'b' GROUP BY length(v1)+length(v2),pk3 ORDER BY pk3",
+ "SELECT pk3 FROM test WHERE pk1 = 'a' and v2 = 'b' GROUP BY length(pk1)+length(v2),pk3 ORDER BY pk3",
+ "SELECT pk3 FROM test WHERE v1 = 'a' and v2 = substr('abc',2,1) GROUP BY pk4,CASE WHEN v1 > v2 THEN v1 ELSE v2 END,pk3 ORDER BY pk4,pk3",
+ "SELECT pk3 FROM test WHERE pk1 = 'a' and v2 = substr('abc',2,1) GROUP BY pk4,CASE WHEN pk1 > v2 THEN pk1 ELSE v2 END,pk3 ORDER BY pk4,pk3",
+ "SELECT pk3 FROM test WHERE pk1 = 'a' and pk2 = 'b' and v1 = 'c' GROUP BY CASE WHEN pk1 > pk2 THEN v1 WHEN pk1 = pk2 THEN pk1 ELSE pk2 END,pk3 ORDER BY pk3"
+ };
+ int index = 0;
+ for (String query : queries) {
+ QueryPlan plan = getQueryPlan(conn, query);
+ assertTrue((index + 1) + ") " + queries[index], plan.getOrderBy().getOrderByExpressions().isEmpty());
+ index++;
+ }
+ }
+ }
+
+ @Test
+ public void testOrderPreservingGroupByForClientAggregatePlan() throws Exception {
+ Connection conn = null;
+ try {
+ conn = DriverManager.getConnection(getUrl());
+ String tableName = "test_table";
+ String sql = "create table " + tableName + "( "+
+ " pk1 varchar not null , " +
+ " pk2 varchar not null, " +
+ " pk3 varchar not null," +
+ " v1 varchar, " +
+ " v2 varchar, " +
+ " CONSTRAINT TEST_PK PRIMARY KEY ( "+
+ "pk1,"+
+ "pk2,"+
+ "pk3 ))";
+ conn.createStatement().execute(sql);
+
+ String[] queries = new String[] {
+ "select a.ak3 "+
+ "from (select substr(pk1,1,1) ak1,substr(pk2,1,1) ak2,substr(pk3,1,1) ak3,substr(v1,1,1) av1,substr(v2,1,1) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "group by a.ak3,a.av1 order by a.ak3,a.av1",
+
+ "select a.ak3 "+
+ "from (select substr(pk1,1,1) ak1,substr(pk2,1,1) ak2,substr(pk3,1,1) ak3,substr(v1,1,1) av1,substr(v2,1,1) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where a.av2 = 'a' GROUP BY substr(a.av2,0,1),ak3 ORDER BY ak3",
+
+ //for InListExpression
+ "select a.ak3 "+
+ "from (select substr(pk1,1,1) ak1,substr(pk2,1,1) ak2,substr(pk3,1,1) ak3,substr(v1,1,1) av1,substr(v2,1,1) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where a.av2 in('a') GROUP BY substr(a.av2,0,1),ak3 ORDER BY ak3",
+
+ "select a.ak3 "+
+ "from (select substr(pk1,1,1) ak1,substr(pk2,1,1) ak2,substr(pk3,1,1) ak3,substr(v1,1,1) av1,substr(v2,1,1) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where a.ak1 = 'c' and a.av2 = substr('abc',1,1) GROUP BY a.av2,a.ak3 ORDER BY a.ak3",
+
+ //for RVC
+ "select a.ak3 "+
+ "from (select substr(pk1,1,1) ak1,substr(pk2,1,1) ak2,substr(pk3,1,1) ak3,substr(v1,1,1) av1,substr(v2,1,1) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where (a.ak1,a.av2) = ('c', substr('abc',1,1)) GROUP BY a.av2,a.ak3 ORDER BY a.ak3",
+
+ "select a.ak3 "+
+ "from (select substr(pk1,1,1) ak1,substr(pk2,1,1) ak2,substr(pk3,1,1) ak3,substr(v1,1,1) av1,substr(v2,1,1) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where a.av1 = 'a' and a.av2 = 'b' GROUP BY length(a.av1)+length(a.av2),a.ak3 ORDER BY a.ak3",
+
+ "select a.ak3 "+
+ "from (select substr(pk1,1,1) ak1,substr(pk2,1,1) ak2,substr(pk3,1,1) ak3,substr(v1,1,1) av1,substr(v2,1,1) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where a.ak1 = 'a' and a.av2 = 'b' GROUP BY length(a.ak1)+length(a.av2),a.ak3 ORDER BY a.ak3",
+
+ "select a.ak3 "+
+ "from (select substr(pk1,1,1) ak1,substr(pk2,1,1) ak2,substr(pk3,1,1) ak3, coalesce(pk3,'1') ak4, substr(v1,1,1) av1,substr(v2,1,1) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where a.av1 = 'a' and a.av2 = substr('abc',2,1) GROUP BY a.ak4,CASE WHEN a.av1 > a.av2 THEN a.av1 ELSE a.av2 END,a.ak3 ORDER BY a.ak4,a.ak3",
+
+ "select a.ak3 "+
+ "from (select rand() ak1,length(pk2) ak2,length(pk3) ak3,length(v1) av1,length(v2) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where a.ak1 = 0.0 and a.av2 = (5+3*2) GROUP BY a.ak3,CASE WHEN a.ak1 > a.av2 THEN a.ak1 ELSE a.av2 END,a.av1 ORDER BY a.ak3,a.av1",
+
+ "select a.ak3 "+
+ "from (select rand() ak1,length(pk2) ak2,length(pk3) ak3,length(v1) av1,length(v2) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where a.ak1 = 0.0 and a.av2 = length(substr('abc',1,1)) GROUP BY a.ak3,CASE WHEN a.ak1 > a.av2 THEN a.ak1 ELSE a.av2 END,a.av1 ORDER BY a.ak3,a.av1",
+
+ "select a.ak3 "+
+ "from (select rand() ak1,length(pk2) ak2,length(pk3) ak3,length(v1) av1,length(v2) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where a.ak1 = 0.0 and a.av2 = length(substr('abc',1,1)) GROUP BY a.ak3,CASE WHEN coalesce(a.ak1,1) > coalesce(a.av2,2) THEN coalesce(a.ak1,1) ELSE coalesce(a.av2,2) END,a.av1 ORDER BY a.ak3,a.av1",
+
+ //for IS NULL
+ "select a.ak3 "+
+ "from (select rand() ak1,length(pk2) ak2,length(pk3) ak3,length(v1) av1,length(v2) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where a.ak1 is null and a.av2 = length(substr('abc',1,1)) GROUP BY a.ak3,CASE WHEN coalesce(a.ak1,1) > coalesce(a.av2,2) THEN coalesce(a.ak1,1) ELSE coalesce(a.av2,2) END,a.av1 ORDER BY a.ak3,a.av1",
+
+ "select a.ak3 "+
+ "from (select rand() ak1,length(pk2) ak2,length(pk3) ak3,length(v1) av1,length(v2) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where a.ak1 = 0.0 and a.av2 is null GROUP BY a.ak3,CASE WHEN coalesce(a.ak1,1) > coalesce(a.av2,2) THEN coalesce(a.ak1,1) ELSE coalesce(a.av2,2) END,a.av1 ORDER BY a.ak3,a.av1",
+ };
+ int index = 0;
+ for (String query : queries) {
+ QueryPlan plan = TestUtil.getOptimizeQueryPlan(conn, query);
+ assertTrue((index + 1) + ") " + queries[index], plan.getOrderBy()== OrderBy.FWD_ROW_KEY_ORDER_BY);
+ index++;
+ }
+ }
+ finally {
+ if(conn != null) {
+ conn.close();
+ }
+ }
+ }
@Test
public void testNotOrderPreservingGroupBy() throws Exception {
@@ -2988,6 +3111,207 @@ public class QueryCompilerTest extends BaseConnectionlessQueryTest {
}
@Test
+ public void testNotOrderPreservingGroupByForNotPkColumns() throws Exception {
+ try (Connection conn= DriverManager.getConnection(getUrl())) {
+ conn.createStatement().execute("CREATE TABLE test (\n" +
+ " pk1 varchar,\n" +
+ " pk2 varchar,\n" +
+ " pk3 varchar,\n" +
+ " pk4 varchar,\n" +
+ " v1 varchar,\n" +
+ " v2 varchar,\n" +
+ " CONSTRAINT pk PRIMARY KEY (\n" +
+ " pk1,\n" +
+ " pk2,\n" +
+ " pk3,\n" +
+ " pk4\n" +
+ " )\n" +
+ " )");
+ String[] queries = new String[] {
+ "SELECT pk3 FROM test WHERE (pk1 = 'a' and pk2 = 'b') or v1 ='c' GROUP BY pk4,CASE WHEN pk1 > pk2 THEN coalesce(v1,'1') ELSE pk2 END,pk3 ORDER BY pk4,pk3",
+ "SELECT pk3 FROM test WHERE pk1 = 'a' or pk2 = 'b' GROUP BY CASE WHEN pk1 > pk2 THEN v1 WHEN pk1 = pk2 THEN pk1 ELSE pk2 END,pk3 ORDER BY pk3",
+ "SELECT pk3 FROM test WHERE pk1 = 'a' and (pk2 = 'b' or v1 = 'c') GROUP BY CASE WHEN pk1 > pk2 THEN v1 WHEN pk1 = pk2 THEN pk1 ELSE pk2 END,pk3 ORDER BY pk3",
+ "SELECT v2 FROM test GROUP BY v1,v2 ORDER BY v2",
+ "SELECT pk3 FROM test WHERE v1 = 'a' GROUP BY v1,v2,pk3 ORDER BY pk3",
+ "SELECT length(pk3) FROM test WHERE v1 = 'a' GROUP BY RAND()+length(v1),length(v2),length(pk3) ORDER BY length(v2),length(pk3)",
+ "SELECT length(pk3) FROM test WHERE v1 = 'a' and v2 = 'b' GROUP BY CASE WHEN v1 > v2 THEN length(v1) ELSE RAND(1) END,length(pk3) ORDER BY length(pk3)",
+ };
+ int index = 0;
+ for (String query : queries) {
+ QueryPlan plan = getQueryPlan(conn, query);
+ assertFalse((index + 1) + ") " + queries[index], plan.getOrderBy().getOrderByExpressions().isEmpty());
+ index++;
+ }
+ }
+ }
+
+ @Test
+ public void testNotOrderPreservingGroupByForClientAggregatePlan() throws Exception {
+ Connection conn = null;
+ try {
+ conn = DriverManager.getConnection(getUrl());
+ String tableName = "table_test";
+ String sql = "create table " + tableName + "( "+
+ " pk1 varchar not null , " +
+ " pk2 varchar not null, " +
+ " pk3 varchar not null," +
+ " v1 varchar, " +
+ " v2 varchar, " +
+ " CONSTRAINT TEST_PK PRIMARY KEY ( "+
+ "pk1,"+
+ "pk2,"+
+ "pk3 ))";
+ conn.createStatement().execute(sql);
+
+ String[] queries = new String[] {
+ "select a.ak3 "+
+ "from (select substr(pk1,1,1) ak1,substr(pk2,1,1) ak2,substr(pk3,1,1) ak3,coalesce(pk3,'1') ak4, substr(v1,1,1) av1,substr(v2,1,1) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where (a.ak1 = 'a' and a.ak2 = 'b') or a.av1 ='c' GROUP BY a.ak4,CASE WHEN a.ak1 > a.ak2 THEN coalesce(a.av1,'1') ELSE a.ak2 END,a.ak3 ORDER BY a.ak4,a.ak3",
+
+ "select a.ak3 "+
+ "from (select substr(pk1,1,1) ak1,substr(pk2,1,1) ak2,substr(pk3,1,1) ak3,coalesce(pk3,'1') ak4, substr(v1,1,1) av1,substr(v2,1,1) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where a.ak1 = 'a' or a.ak2 = 'b' GROUP BY CASE WHEN a.ak1 > a.ak2 THEN a.av1 WHEN a.ak1 = a.ak2 THEN a.ak1 ELSE a.ak2 END,a.ak3 ORDER BY a.ak3",
+
+ //for in
+ "select a.ak3 "+
+ "from (select substr(pk1,1,1) ak1,substr(pk2,1,1) ak2,substr(pk3,1,1) ak3,coalesce(pk3,'1') ak4, substr(v1,1,1) av1,substr(v2,1,1) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where a.ak1 in ( 'a','b') GROUP BY CASE WHEN a.ak1 > a.ak2 THEN a.av1 WHEN a.ak1 = a.ak2 THEN a.ak1 ELSE a.ak2 END,a.ak3 ORDER BY a.ak3",
+
+ "select a.ak3 "+
+ "from (select substr(pk1,1,1) ak1,substr(pk2,1,1) ak2,substr(pk3,1,1) ak3,coalesce(pk3,'1') ak4, substr(v1,1,1) av1,substr(v2,1,1) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where a.ak1 = 'a' and (a.ak2 = 'b' or a.av1 = 'c') GROUP BY CASE WHEN a.ak1 > a.ak2 THEN a.av1 WHEN a.ak1 = a.ak2 THEN a.ak1 ELSE a.ak2 END,a.ak3 ORDER BY a.ak3",
+
+ "select a.av2 "+
+ "from (select substr(pk1,1,1) ak1,substr(pk2,1,1) ak2,substr(pk3,1,1) ak3,coalesce(pk3,'1') ak4, substr(v1,1,1) av1,substr(v2,1,1) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "GROUP BY a.av1,a.av2 ORDER BY a.av2",
+
+ "select a.ak3 "+
+ "from (select substr(pk1,1,1) ak1,substr(pk2,1,1) ak2,substr(pk3,1,1) ak3,coalesce(pk3,'1') ak4, substr(v1,1,1) av1,substr(v2,1,1) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where a.av1 = 'a' GROUP BY a.av1,a.av2,a.ak3 ORDER BY a.ak3",
+
+ "select length(a.ak3) "+
+ "from (select substr(pk1,1,1) ak1,substr(pk2,1,1) ak2,substr(pk3,1,1) ak3,coalesce(pk3,'1') ak4, substr(v1,1,1) av1,substr(v2,1,1) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where a.av1 = 'a' GROUP BY RAND()+length(a.av1),length(a.av2),length(a.ak3) ORDER BY length(a.av2),length(a.ak3)",
+
+ "select length(a.ak3) "+
+ "from (select substr(pk1,1,1) ak1,substr(pk2,1,1) ak2,substr(pk3,1,1) ak3,coalesce(pk3,'1') ak4, substr(v1,1,1) av1,substr(v2,1,1) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where a.av1 = 'a' and a.av2 = 'b' GROUP BY CASE WHEN a.av1 > a.av2 THEN length(a.av1) ELSE RAND(1) END,length(a.ak3) ORDER BY length(a.ak3)",
+
+ "select a.ak3 "+
+ "from (select rand() ak1,length(pk2) ak2,length(pk3) ak3,length(v1) av1,length(v2) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where a.ak1 > 0.0 and a.av2 = (5+3*2) GROUP BY a.ak3,CASE WHEN a.ak1 > a.av2 THEN a.ak1 ELSE a.av2 END,a.av1 ORDER BY a.ak3,a.av1",
+
+ //for CoerceExpression
+ "select a.ak3 "+
+ "from (select rand() ak1,length(pk2) ak2,length(pk3) ak3,length(v1) av1,length(v2) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where CAST(a.ak1 AS INTEGER) = 0 and a.av2 = (5+3*2) GROUP BY a.ak3,a.ak1,a.av1 ORDER BY a.ak3,a.av1",
+
+ "select a.ak3 "+
+ "from (select rand() ak1,length(pk2) ak2,length(pk3) ak3,length(v1) av1,length(v2) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where a.ak1 = 0.0 or a.av2 = length(substr('abc',1,1)) GROUP BY a.ak3,CASE WHEN coalesce(a.ak1,1) > coalesce(a.av2,2) THEN coalesce(a.ak1,1) ELSE coalesce(a.av2,2) END,a.av1 ORDER BY a.ak3,a.av1",
+
+ //for IS NULL
+ "select a.ak3 "+
+ "from (select rand() ak1,length(pk2) ak2,length(pk3) ak3,length(v1) av1,length(v2) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where a.ak1 is not null and a.av2 = length(substr('abc',1,1)) GROUP BY a.ak3,CASE WHEN coalesce(a.ak1,1) > coalesce(a.av2,2) THEN coalesce(a.ak1,1) ELSE coalesce(a.av2,2) END,a.av1 ORDER BY a.ak3,a.av1",
+
+ "select a.ak3 "+
+ "from (select rand() ak1,length(pk2) ak2,length(pk3) ak3,length(v1) av1,length(v2) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where a.ak1 is null or a.av2 = length(substr('abc',1,1)) GROUP BY a.ak3,CASE WHEN coalesce(a.ak1,1) > coalesce(a.av2,2) THEN coalesce(a.ak1,1) ELSE coalesce(a.av2,2) END,a.av1 ORDER BY a.ak3,a.av1",
+
+ "select a.ak3 "+
+ "from (select rand() ak1,length(pk2) ak2,length(pk3) ak3,length(v1) av1,length(v2) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where a.ak1 is null and a.av2 = length(substr('abc',1,1)) and a.ak1 = 0.0 GROUP BY a.ak3,CASE WHEN coalesce(a.ak1,1) > coalesce(a.av2,2) THEN coalesce(a.ak1,1) ELSE coalesce(a.av2,2) END,a.av1 ORDER BY a.ak3,a.av1",
+
+ "select a.ak3 "+
+ "from (select rand() ak1,length(pk2) ak2,length(pk3) ak3,length(v1) av1,length(v2) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where a.ak1 is null and a.av2 = length(substr('abc',1,1)) or a.ak1 = 0.0 GROUP BY a.ak3,CASE WHEN coalesce(a.ak1,1) > coalesce(a.av2,2) THEN coalesce(a.ak1,1) ELSE coalesce(a.av2,2) END,a.av1 ORDER BY a.ak3,a.av1",
+ };
+ int index = 0;
+ for (String query : queries) {
+ QueryPlan plan = TestUtil.getOptimizeQueryPlan(conn, query);
+ assertTrue((index + 1) + ") " + queries[index], plan.getOrderBy().getOrderByExpressions().size() > 0);
+ index++;
+ }
+ }
+ finally {
+ if(conn != null) {
+ conn.close();
+ }
+ }
+ }
+
+ @Test
+ public void testOrderByOptimizeForClientAggregatePlanAndDesc() throws Exception {
+ Connection conn = null;
+ try {
+ conn = DriverManager.getConnection(getUrl());
+ String tableName = "test_table";
+ String sql = "create table " + tableName + "( "+
+ " pk1 varchar not null, " +
+ " pk2 varchar not null, " +
+ " pk3 varchar not null, " +
+ " v1 varchar, " +
+ " v2 varchar, " +
+ " CONSTRAINT TEST_PK PRIMARY KEY ( "+
+ "pk1 desc,"+
+ "pk2 desc,"+
+ "pk3 desc))";
+ conn.createStatement().execute(sql);
+
+ String[] queries = new String[] {
+ "select a.ak3 "+
+ "from (select pk1 ak1,pk2 ak2,pk3 ak3, substr(v1,1,1) av1,substr(v2,1,1) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "group by a.ak3,a.av1 order by a.ak3 desc,a.av1",
+
+ "select a.ak3 "+
+ "from (select pk1 ak1,pk2 ak2,pk3 ak3,substr(v1,1,1) av1,substr(v2,1,1) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where a.av1 = 'a' group by a.av1,a.ak3 order by a.ak3 desc",
+
+ "select a.ak3 "+
+ "from (select pk1 ak1,pk2 ak2,pk3 ak3,substr(v1,1,1) av1,substr(v2,1,1) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where a.av1 = 'a' and a.av2= 'b' group by CASE WHEN a.av1 > a.av2 THEN a.av1 ELSE a.av2 END,a.ak3,a.ak2 order by a.ak3 desc,a.ak2 desc"
+ };
+
+ int index = 0;
+ for (String query : queries) {
+ QueryPlan plan = TestUtil.getOptimizeQueryPlan(conn, query);
+ assertTrue((index + 1) + ") " + queries[index], plan.getOrderBy()== OrderBy.FWD_ROW_KEY_ORDER_BY);
+ index++;
+ }
+
+ queries = new String[] {
+ "select a.ak3 "+
+ "from (select pk1 ak1,pk2 ak2,pk3 ak3,substr(v1,1,1) av1,substr(v2,1,1) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "group by a.ak3,a.av1 order by a.ak3,a.av1",
+
+ "select a.ak3 "+
+ "from (select pk1 ak1,pk2 ak2,pk3 ak3,substr(v1,1,1) av1,substr(v2,1,1) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where a.av1 = 'a' group by a.av1,a.ak3 order by a.ak3",
+
+ "select a.ak3 "+
+ "from (select pk1 ak1,pk2 ak2,pk3 ak3,substr(v1,1,1) av1,substr(v2,1,1) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where a.av1 = 'a' and a.av2= 'b' group by CASE WHEN a.av1 > a.av2 THEN a.av1 ELSE a.av2 END,a.ak3,a.ak2 order by a.ak3,a.ak2",
+
+ "select a.ak3 "+
+ "from (select pk1 ak1,pk2 ak2,pk3 ak3,substr(v1,1,1) av1,substr(v2,1,1) av2 from "+tableName+" order by pk2,pk3 limit 10) a "+
+ "where a.av1 = 'a' and a.av2= 'b' group by CASE WHEN a.av1 > a.av2 THEN a.av1 ELSE a.av2 END,a.ak3,a.ak2 order by a.ak3 asc,a.ak2 desc"
+ };
+ index = 0;
+ for (String query : queries) {
+ QueryPlan plan = TestUtil.getOptimizeQueryPlan(conn, query);
+ assertTrue((index + 1) + ") " + queries[index], plan.getOrderBy().getOrderByExpressions().size() > 0);
+ index++;
+ }
+ }
+ finally {
+ if(conn != null) {
+ conn.close();
+ }
+ }
+ }
+
+ @Test
public void testGroupByDescColumnBug3451() throws Exception {
try (Connection conn= DriverManager.getConnection(getUrl())) {
diff --git a/phoenix-core/src/test/java/org/apache/phoenix/expression/ArithmeticOperationTest.java b/phoenix-core/src/test/java/org/apache/phoenix/expression/ArithmeticOperationTest.java
index 7876fce..1b830f2 100644
--- a/phoenix-core/src/test/java/org/apache/phoenix/expression/ArithmeticOperationTest.java
+++ b/phoenix-core/src/test/java/org/apache/phoenix/expression/ArithmeticOperationTest.java
@@ -30,7 +30,7 @@ import java.util.List;
import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
import org.apache.phoenix.exception.DataExceedsCapacityException;
import org.apache.phoenix.expression.function.RandomFunction;
-import org.apache.phoenix.expression.visitor.CloneNonDeterministicExpressionVisitor;
+import org.apache.phoenix.expression.visitor.CloneExpressionVisitor;
import org.apache.phoenix.schema.types.PDataType;
import org.apache.phoenix.schema.types.PDecimal;
import org.apache.phoenix.schema.types.PInteger;
@@ -273,7 +273,7 @@ public class ArithmeticOperationTest {
e2 = new DoubleSubtractExpression(children);
e3 = new DoubleAddExpression(Arrays.<Expression>asList(e1, e2));
e4 = new DoubleAddExpression(Arrays.<Expression>asList(new RandomFunction(Arrays.<Expression>asList(LiteralExpression.newConstant(null))), e3));
- CloneNonDeterministicExpressionVisitor visitor = new CloneNonDeterministicExpressionVisitor();
+ CloneExpressionVisitor visitor = new CloneExpressionVisitor();
Expression clone = e4.accept(visitor);
assertTrue(clone != e4);
e4.evaluate(null, ptr1);
@@ -281,7 +281,7 @@ public class ArithmeticOperationTest {
assertNotEquals(ptr1, ptr2);
e4 = new DoubleAddExpression(Arrays.<Expression>asList(new RandomFunction(Arrays.<Expression>asList(LiteralExpression.newConstant(1))), e3));
- visitor = new CloneNonDeterministicExpressionVisitor();
+ visitor = new CloneExpressionVisitor();
clone = e4.accept(visitor);
assertTrue(clone == e4);
e4.evaluate(null, ptr1);
@@ -294,7 +294,7 @@ public class ArithmeticOperationTest {
boolean evaluated = e.evaluate(null, ptr);
assertTrue(evaluated);
assertEquals(value, type.toObject(ptr.get()));
- CloneNonDeterministicExpressionVisitor visitor = new CloneNonDeterministicExpressionVisitor();
+ CloneExpressionVisitor visitor = new CloneExpressionVisitor();
Expression clone = e.accept(visitor);
evaluated = clone.evaluate(null, ptr);
assertTrue(evaluated);