You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tajo.apache.org by hy...@apache.org on 2014/03/28 14:10:58 UTC
git commit: TAJO-718: A group-by clause with the same columns but
aliased causes planning error. (hyunsik)
Repository: tajo
Updated Branches:
refs/heads/master 53f7ffa54 -> 8c1f04042
TAJO-718: A group-by clause with the same columns but aliased causes planning error. (hyunsik)
Project: http://git-wip-us.apache.org/repos/asf/tajo/repo
Commit: http://git-wip-us.apache.org/repos/asf/tajo/commit/8c1f0404
Tree: http://git-wip-us.apache.org/repos/asf/tajo/tree/8c1f0404
Diff: http://git-wip-us.apache.org/repos/asf/tajo/diff/8c1f0404
Branch: refs/heads/master
Commit: 8c1f0404240bad92a7f48e858d9dc28a17ccc20f
Parents: 53f7ffa
Author: Hyunsik Choi <hy...@apache.org>
Authored: Fri Mar 28 22:07:13 2014 +0900
Committer: Hyunsik Choi <hy...@apache.org>
Committed: Fri Mar 28 22:07:13 2014 +0900
----------------------------------------------------------------------
CHANGES.txt | 9 +++-
.../tajo/engine/planner/ExprNormalizer.java | 7 +--
.../tajo/engine/planner/LogicalPlanner.java | 14 ++++--
.../planner/rewrite/ProjectionPushDownRule.java | 51 ++++++++++++++++----
.../tajo/engine/query/TestCaseByCases.java | 7 +++
5 files changed, 67 insertions(+), 21 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/tajo/blob/8c1f0404/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index f50b0ee..bc8be9b 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -289,8 +289,11 @@ Release 0.8.0 - unreleased
BUG FIXES
- TAJO-679: TimestampDatum, TimeDatum, DateDatum should be able to be compared
- with NullDatum. (Alvin Henrick via jihoon)
+ TAJO-718: A group-by clause with the same columns but aliased causes
+ planning error. (hyunsik)
+
+ TAJO-679: TimestampDatum, TimeDatum, DateDatum should be able to be
+ compared with NullDatum. (Alvin Henrick via jihoon)
TAJO-716: Using column names actually aliased in aggregation functions
can cause planning error. (hyunsik)
@@ -300,6 +303,8 @@ Release 0.8.0 - unreleased
TAJO-692: Missing Null handling for INET4 in RowStoreUtil. (jihoon)
+ TAJO-712: Fix some bugs after database is supported. (hyunsik)
+
TAJO-701: Invalid bytes when creating BlobDatum with offset. (jinho)
TAJO-708: Test failure after a successful test. (jihoon)
http://git-wip-us.apache.org/repos/asf/tajo/blob/8c1f0404/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/ExprNormalizer.java
----------------------------------------------------------------------
diff --git a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/ExprNormalizer.java b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/ExprNormalizer.java
index 9c732b4..9030629 100644
--- a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/ExprNormalizer.java
+++ b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/ExprNormalizer.java
@@ -245,12 +245,9 @@ class ExprNormalizer extends SimpleAlgebraVisitor<ExprNormalizer.ExprNormalizedR
@Override
public Expr visitColumnReference(ExprNormalizedResult ctx, Stack<Expr> stack, ColumnReferenceExpr expr)
throws PlanningException {
- // normalize column references.
+ // if a column reference is not qualified, it finds and sets the qualified column name.
if (!(expr.hasQualifier() && CatalogUtil.isFQTableName(expr.getQualifier()))) {
- if (ctx.block.namedExprsMgr.contains(expr.getCanonicalName())) {
- NamedExpr namedExpr = ctx.block.namedExprsMgr.getNamedExpr(expr.getCanonicalName());
- return new ColumnReferenceExpr(namedExpr.getAlias());
- } else {
+ if (!ctx.block.namedExprsMgr.contains(expr.getCanonicalName())) {
String normalized = ctx.plan.getNormalizedColumnName(ctx.block, expr);
expr.setName(normalized);
}
http://git-wip-us.apache.org/repos/asf/tajo/blob/8c1f0404/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/LogicalPlanner.java
----------------------------------------------------------------------
diff --git a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/LogicalPlanner.java b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/LogicalPlanner.java
index c479f1c..15fe6c0 100644
--- a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/LogicalPlanner.java
+++ b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/LogicalPlanner.java
@@ -367,10 +367,13 @@ public class LogicalPlanner extends BaseAlgebraVisitor<LogicalPlanner.PlanContex
}
} else if (projectable instanceof GroupbyNode) {
GroupbyNode groupbyNode = (GroupbyNode) projectable;
- for (Column c : groupbyNode.getGroupingColumns()) {
- if (!projectable.getInSchema().contains(c)) {
- throw new PlanningException(String.format("Cannot get the field \"%s\" at node (%d)",
- c, projectable.getPID()));
+ // It checks if all column references within each target can be evaluated with the input schema.
+ int groupingColumnNum = groupbyNode.getGroupingColumns().length;
+ for (int i = 0; i < groupingColumnNum; i++) {
+ Set<Column> columns = EvalTreeUtil.findUniqueColumns(groupbyNode.getTargets()[i].getEvalTree());
+ if (!projectable.getInSchema().containsAll(columns)) {
+ throw new PlanningException(String.format("Cannot get the field(s) \"%s\" at node (%d)",
+ TUtil.collectionToString(columns), projectable.getPID()));
}
}
if (groupbyNode.hasAggFunctions()) {
@@ -610,6 +613,7 @@ public class LogicalPlanner extends BaseAlgebraVisitor<LogicalPlanner.PlanContex
Expr groupingKey = aggregation.getGroupSet()[0].getGroupingSets()[i];
normalizedResults[i] = normalizer.normalize(context, groupingKey);
}
+
String [] groupingKeyRefNames = new String[groupingKeyNum];
for (int i = 0; i < groupingKeyNum; i++) {
groupingKeyRefNames[i] = block.namedExprsMgr.addExpr(normalizedResults[i].baseExpr);
@@ -674,7 +678,7 @@ public class LogicalPlanner extends BaseAlgebraVisitor<LogicalPlanner.PlanContex
// Build grouping keys
for (int i = 0; i < groupingKeyNum; i++) {
- Target target = new Target(new FieldEval(groupingNode.getGroupingColumns()[i]));
+ Target target = block.namedExprsMgr.getTarget(groupingNode.getGroupingColumns()[i].getQualifiedName());
targets[i] = target;
}
http://git-wip-us.apache.org/repos/asf/tajo/blob/8c1f0404/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/rewrite/ProjectionPushDownRule.java
----------------------------------------------------------------------
diff --git a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/rewrite/ProjectionPushDownRule.java b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/rewrite/ProjectionPushDownRule.java
index 4f3c6d4..668ed68 100644
--- a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/rewrite/ProjectionPushDownRule.java
+++ b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/rewrite/ProjectionPushDownRule.java
@@ -486,6 +486,29 @@ public class ProjectionPushDownRule extends
LogicalNode child = super.visitSort(newContext, plan, block, node, stack);
+ // it rewrite sortkeys. This rewrite sets right column names and eliminates duplicated sort keys.
+ List<SortSpec> sortSpecs = new ArrayList<SortSpec>();
+ for (int i = 0; i < keyNames.length; i++) {
+ String sortKey = keyNames[i];
+ Target target = context.targetListMgr.getTarget(sortKey);
+ if (context.targetListMgr.isEvaluated(sortKey)) {
+ Column c = target.getNamedColumn();
+ SortSpec sortSpec = new SortSpec(c, node.getSortKeys()[i].isAscending(), node.getSortKeys()[i].isNullFirst());
+ if (!sortSpecs.contains(sortSpec)) {
+ sortSpecs.add(sortSpec);
+ }
+ } else {
+ if (target.getEvalTree().getType() == EvalType.FIELD) {
+ Column c = ((FieldEval)target.getEvalTree()).getColumnRef();
+ SortSpec sortSpec = new SortSpec(c, node.getSortKeys()[i].isAscending(), node.getSortKeys()[i].isNullFirst());
+ if (!sortSpecs.contains(sortSpec)) {
+ sortSpecs.add(sortSpec);
+ }
+ }
+ }
+ }
+ node.setSortSpecs(sortSpecs.toArray(new SortSpec[sortSpecs.size()]));
+
node.setInSchema(child.getOutSchema());
node.setOutSchema(child.getOutSchema());
return node;
@@ -550,31 +573,41 @@ public class ProjectionPushDownRule extends
node.setInSchema(child.getOutSchema());
List<Target> targets = Lists.newArrayList();
- if (groupingKeyNum > 0) {
+ if (groupingKeyNum > 0 && groupingKeyNames != null) {
// Restoring grouping key columns
- final Column [] groupingColumns = new Column[groupingKeyNum];
+ final List<Column> groupingColumns = new ArrayList<Column>();
for (int i = 0; i < groupingKeyNum; i++) {
String groupingKey = groupingKeyNames[i];
Target target = context.targetListMgr.getTarget(groupingKey);
+
+ // it rewrite grouping keys.
+ // This rewrite sets right column names and eliminates duplicated grouping keys.
if (context.targetListMgr.isEvaluated(groupingKey)) {
- groupingColumns[i] = target.getNamedColumn();
- targets.add(new Target(new FieldEval(target.getNamedColumn())));
+ Column c = target.getNamedColumn();
+ if (!groupingColumns.contains(c)) {
+ groupingColumns.add(c);
+ targets.add(new Target(new FieldEval(target.getNamedColumn())));
+ }
} else {
if (target.getEvalTree().getType() == EvalType.FIELD) {
- groupingColumns[i] = ((FieldEval)target.getEvalTree()).getColumnRef();
- targets.add(target);
- context.targetListMgr.markAsEvaluated(target);
+ Column c = ((FieldEval)target.getEvalTree()).getColumnRef();
+ if (!groupingColumns.contains(c)) {
+ groupingColumns.add(c);
+ targets.add(target);
+ context.targetListMgr.markAsEvaluated(target);
+ }
} else {
throw new PlanningException("Cannot evaluate this expression in grouping keys: " + target.getEvalTree());
}
}
}
- node.setGroupingColumns(groupingColumns);
+
+ node.setGroupingColumns(groupingColumns.toArray(new Column[groupingColumns.size()]));
}
// Getting projected targets
- if (node.hasAggFunctions()) {
+ if (node.hasAggFunctions() && aggEvalNames != null) {
AggregationFunctionCallEval [] aggEvals = new AggregationFunctionCallEval[aggEvalNames.length];
int i = 0;
for (Iterator<String> it = getFilteredReferences(aggEvalNames, TUtil.newList(aggEvalNames)); it.hasNext();) {
http://git-wip-us.apache.org/repos/asf/tajo/blob/8c1f0404/tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/query/TestCaseByCases.java
----------------------------------------------------------------------
diff --git a/tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/query/TestCaseByCases.java b/tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/query/TestCaseByCases.java
index 6472c68..2c42b2a 100644
--- a/tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/query/TestCaseByCases.java
+++ b/tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/query/TestCaseByCases.java
@@ -53,4 +53,11 @@ public class TestCaseByCases extends QueryTestCaseBase {
assertResultSet(res);
cleanupQuery(res);
}
+
+ @Test
+ public final void testTAJO718Case() throws Exception {
+ ResultSet res = executeQuery();
+ assertResultSet(res);
+ cleanupQuery(res);
+ }
}