tajo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From hyun...@apache.org
Subject git commit: TAJO-718: A group-by clause with the same columns but aliased causes planning error. (hyunsik)
Date Fri, 28 Mar 2014 13:10:58 GMT
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 <hyunsik@apache.org>
Authored: Fri Mar 28 22:07:13 2014 +0900
Committer: Hyunsik Choi <hyunsik@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);
+  }
 }


Mime
View raw message