tajo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From hyun...@apache.org
Subject git commit: TAJO-716: Using column names actually aliased in aggregation functions can cause planning error.
Date Thu, 27 Mar 2014 02:54:22 GMT
Repository: tajo
Updated Branches:
  refs/heads/master 858a8f178 -> b1e0e3499


TAJO-716: Using column names actually aliased in aggregation functions can cause planning
error.


Project: http://git-wip-us.apache.org/repos/asf/tajo/repo
Commit: http://git-wip-us.apache.org/repos/asf/tajo/commit/b1e0e349
Tree: http://git-wip-us.apache.org/repos/asf/tajo/tree/b1e0e349
Diff: http://git-wip-us.apache.org/repos/asf/tajo/diff/b1e0e349

Branch: refs/heads/master
Commit: b1e0e3499e2459e58b2cede4f4268ad26d3ed7d4
Parents: 858a8f1
Author: Hyunsik Choi <hyunsik@apache.org>
Authored: Thu Mar 27 11:54:06 2014 +0900
Committer: Hyunsik Choi <hyunsik@apache.org>
Committed: Thu Mar 27 11:54:06 2014 +0900

----------------------------------------------------------------------
 CHANGES.txt                                     |  3 ++
 .../tajo/engine/planner/ExprNormalizer.java     |  2 +-
 .../apache/tajo/engine/planner/LogicalPlan.java | 35 ++++++++++++++++----
 .../tajo/engine/planner/NamedExprsManager.java  | 22 +++++-------
 .../engine/planner/logical/RelationNode.java    |  2 ++
 .../tajo/engine/planner/logical/ScanNode.java   |  3 +-
 .../planner/logical/TableSubQueryNode.java      |  5 +++
 .../engine/planner/logical/join/JoinGraph.java  | 17 +++-------
 .../apache/tajo/engine/query/TestSortQuery.java | 16 ++++++++-
 .../TestSortQuery/testSortWithAlias2.sql        |  9 +++++
 .../TestSortQuery/testSortWithAlias3.sql        | 11 ++++++
 .../TestSortQuery/testSortWithAlias2.result     |  5 +++
 .../TestSortQuery/testSortWithAlias3.result     |  7 ++++
 .../testSortWithAscDescKeys.result              | 26 +++++++++++++++
 14 files changed, 129 insertions(+), 34 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tajo/blob/b1e0e349/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 643a616..0c818bd 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -283,6 +283,9 @@ Release 0.8.0 - unreleased
 
   BUG FIXES
 
+    TAJO-716: Using column names actually aliased in aggregation functions
+    can cause planning error. (hyunsik)
+
     TAJO-698: Error occurs when FUNCTION and IN statement are used together.
     (hyunsik)
 

http://git-wip-us.apache.org/repos/asf/tajo/blob/b1e0e349/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 5c24192..9c732b4 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
@@ -217,7 +217,7 @@ class ExprNormalizer extends SimpleAlgebraVisitor<ExprNormalizer.ExprNormalizedR
 
       // If parameters are all constants, we don't need to dissect an aggregation expression
into two parts:
       // function and parameter parts.
-      if (!OpType.isLiteral(param.getType())) {
+      if (!OpType.isLiteral(param.getType()) && param.getType() != OpType.Column)
{
         String referenceName = ctx.block.namedExprsMgr.addExpr(param);
         ctx.scalarExprs.add(new NamedExpr(param, referenceName));
         expr.getParams()[i] = new ColumnReferenceExpr(referenceName);

http://git-wip-us.apache.org/repos/asf/tajo/blob/b1e0e349/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/LogicalPlan.java
----------------------------------------------------------------------
diff --git a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/LogicalPlan.java
b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/LogicalPlan.java
index 63dd428..c6d10d8 100644
--- a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/LogicalPlan.java
+++ b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/LogicalPlan.java
@@ -290,6 +290,13 @@ public class LogicalPlan {
         throw new NoSuchColumnException(canonicalName);
       }
 
+      if (block.isAlreadyRenamedTableName(CatalogUtil.extractQualifier(canonicalName))) {
+        String changedName = CatalogUtil.buildFQName(
+            relationOp.getCanonicalName(),
+            CatalogUtil.extractSimpleName(canonicalName));
+        canonicalName = changedName;
+      }
+
       Schema schema = relationOp.getTableSchema();
       Column column = schema.getColumn(canonicalName);
       if (column == null) {
@@ -517,7 +524,8 @@ public class LogicalPlan {
     private NodeType rootType;
 
     // transient states
-    private final Map<String, RelationNode> nameToRelationMap = TUtil.newHashMap();
+    private final Map<String, RelationNode> canonicalNameToRelationMap = TUtil.newHashMap();
+    private final Map<String, List<String>> aliasMap = TUtil.newHashMap();
     private final Map<OpType, List<Expr>> operatorToExprMap = TUtil.newHashMap();
     /**
      * It's a map between nodetype and node. node types can be duplicated. So, latest node
type is only kept.
@@ -578,23 +586,38 @@ public class LogicalPlan {
     }
 
     public boolean existsRelation(String name) {
-      return nameToRelationMap.containsKey(name);
+      return canonicalNameToRelationMap.containsKey(name);
+    }
+
+    public boolean isAlreadyRenamedTableName(String name) {
+      return aliasMap.containsKey(name);
     }
 
     public RelationNode getRelation(String name) {
-      return nameToRelationMap.get(name);
+      if (canonicalNameToRelationMap.containsKey(name)) {
+        return canonicalNameToRelationMap.get(name);
+      }
+
+      if (aliasMap.containsKey(name)) {
+        return canonicalNameToRelationMap.get(aliasMap.get(name).get(0));
+      }
+
+      return null;
     }
 
     public void addRelation(RelationNode relation) {
-      nameToRelationMap.put(relation.getCanonicalName(), relation);
+      if (relation.hasAlias()) {
+        TUtil.putToNestedList(aliasMap, relation.getTableName(), relation.getCanonicalName());
+      }
+      canonicalNameToRelationMap.put(relation.getCanonicalName(), relation);
     }
 
     public Collection<RelationNode> getRelations() {
-      return this.nameToRelationMap.values();
+      return this.canonicalNameToRelationMap.values();
     }
 
     public boolean hasTableExpression() {
-      return this.nameToRelationMap.size() > 0;
+      return this.canonicalNameToRelationMap.size() > 0;
     }
 
     public void setSchema(Schema schema) {

http://git-wip-us.apache.org/repos/asf/tajo/blob/b1e0e349/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/NamedExprsManager.java
----------------------------------------------------------------------
diff --git a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/NamedExprsManager.java
b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/NamedExprsManager.java
index 90d829f..2fcf3bc 100644
--- a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/NamedExprsManager.java
+++ b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/NamedExprsManager.java
@@ -88,10 +88,6 @@ public class NamedExprsManager {
     return sequenceId++;
   }
 
-  private static String normalizeName(String name) {
-    return name;
-  }
-
   /**
    * Check whether the expression corresponding to a given name was evaluated.
    *
@@ -99,9 +95,8 @@ public class NamedExprsManager {
    * @return true if resolved. Otherwise, false.
    */
   public boolean isEvaluated(String name) {
-    String normalized = normalizeName(name);
-    if (nameToIdMap.containsKey(normalized)) {
-      int refId = nameToIdMap.get(normalized);
+    if (nameToIdMap.containsKey(name)) {
+      int refId = nameToIdMap.get(name);
       return evaluationStateMap.containsKey(refId) && evaluationStateMap.get(refId);
     } else {
       return false;
@@ -159,11 +154,10 @@ public class NamedExprsManager {
    * It specifies the alias as an reference name.
    */
   public String addExpr(Expr expr, String alias) throws PlanningException {
-    String normalized = normalizeName(alias);
 
     // if this name already exists, just returns the name.
-    if (nameToIdMap.containsKey(normalized)) {
-      return normalized;
+    if (nameToIdMap.containsKey(alias)) {
+      return alias;
     }
 
     // if the name is first
@@ -175,11 +169,13 @@ public class NamedExprsManager {
       idToExprBiMap.put(refId, expr);
     }
 
-    nameToIdMap.put(normalized, refId);
-    TUtil.putToNestedList(idToNamesMap, refId, normalized);
+    nameToIdMap.put(alias, refId);
     evaluationStateMap.put(refId, false);
 
-    return normalized;
+    // add the entry to idToNames map
+    TUtil.putToNestedList(idToNamesMap, refId, alias);
+
+    return alias;
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/tajo/blob/b1e0e349/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/RelationNode.java
----------------------------------------------------------------------
diff --git a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/RelationNode.java
b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/RelationNode.java
index 968da30..9e4575c 100644
--- a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/RelationNode.java
+++ b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/RelationNode.java
@@ -37,6 +37,8 @@ public abstract class RelationNode extends LogicalNode {
     assert(nodeType == NodeType.SCAN || nodeType == NodeType.PARTITIONS_SCAN || nodeType
== NodeType.TABLE_SUBQUERY);
   }
 
+  public abstract boolean hasAlias();
+
   public abstract String getTableName();
 
   public abstract String getCanonicalName();

http://git-wip-us.apache.org/repos/asf/tajo/blob/b1e0e349/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/ScanNode.java
----------------------------------------------------------------------
diff --git a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/ScanNode.java
b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/ScanNode.java
index 0f346b5..17ce4af 100644
--- a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/ScanNode.java
+++ b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/ScanNode.java
@@ -72,7 +72,8 @@ public class ScanNode extends RelationNode implements Projectable, Cloneable
{
 	public String getTableName() {
 	  return tableDesc.getName();
 	}
-	
+
+  @Override
 	public boolean hasAlias() {
 	  return alias != null;
 	}

http://git-wip-us.apache.org/repos/asf/tajo/blob/b1e0e349/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/TableSubQueryNode.java
----------------------------------------------------------------------
diff --git a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/TableSubQueryNode.java
b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/TableSubQueryNode.java
index b37a229..52f1027 100644
--- a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/TableSubQueryNode.java
+++ b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/TableSubQueryNode.java
@@ -46,6 +46,11 @@ public class TableSubQueryNode extends RelationNode implements Projectable
{
     }
   }
 
+  @Override
+  public boolean hasAlias() {
+    return false;
+  }
+
   public String getTableName() {
     return tableName;
   }

http://git-wip-us.apache.org/repos/asf/tajo/blob/b1e0e349/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/join/JoinGraph.java
----------------------------------------------------------------------
diff --git a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/join/JoinGraph.java
b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/join/JoinGraph.java
index b4f4d9d..2da1f4b 100644
--- a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/join/JoinGraph.java
+++ b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/join/JoinGraph.java
@@ -19,6 +19,7 @@
 package org.apache.tajo.engine.planner.logical.join;
 
 import com.google.common.collect.Sets;
+import org.apache.tajo.catalog.CatalogUtil;
 import org.apache.tajo.catalog.Column;
 import org.apache.tajo.engine.eval.AlgebraicUtil;
 import org.apache.tajo.engine.eval.EvalNode;
@@ -53,13 +54,8 @@ public class JoinGraph extends SimpleUndirectedGraph<String, JoinEdge>
{
     } else {
       if (namedExprsMgr.isAliasedName(leftExpr.getSimpleName())) {
         String columnName = namedExprsMgr.getOriginalName(leftExpr.getSimpleName());
-        String [] parts = columnName.split("\\.");
-
-        if (parts.length != 2) {
-          throw new PlanningException("Cannot expect a referenced relation: " + leftExpr);
-        }
-
-        relationNames[0] = parts[0];
+        String qualifier = CatalogUtil.extractQualifier(columnName);
+        relationNames[0] = qualifier;
       } else {
         throw new PlanningException("Cannot expect a referenced relation: " + leftExpr);
       }
@@ -70,11 +66,8 @@ public class JoinGraph extends SimpleUndirectedGraph<String, JoinEdge>
{
     } else {
       if (namedExprsMgr.isAliasedName(rightExpr.getSimpleName())) {
         String columnName = namedExprsMgr.getOriginalName(rightExpr.getSimpleName());
-        String [] parts = columnName.split("\\.");
-        if (parts.length != 2) {
-          throw new PlanningException("Cannot expect a referenced relation: " + leftExpr);
-        }
-        relationNames[1] = parts[0];
+        String qualifier = CatalogUtil.extractQualifier(columnName);
+        relationNames[1] = qualifier;
       } else {
         throw new PlanningException("Cannot expect a referenced relation: " + rightExpr);
       }

http://git-wip-us.apache.org/repos/asf/tajo/blob/b1e0e349/tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/query/TestSortQuery.java
----------------------------------------------------------------------
diff --git a/tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/query/TestSortQuery.java
b/tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/query/TestSortQuery.java
index 61206cd..206e638 100644
--- a/tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/query/TestSortQuery.java
+++ b/tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/query/TestSortQuery.java
@@ -49,6 +49,20 @@ public class TestSortQuery extends QueryTestCaseBase {
   }
 
   @Test
+  public final void testSortWithAlias2() throws Exception {
+    ResultSet res = executeQuery();
+    assertResultSet(res);
+    cleanupQuery(res);
+  }
+
+  @Test
+  public final void testSortWithAlias3() throws Exception {
+    ResultSet res = executeQuery();
+    System.out.println(resultSetToString(res));
+    cleanupQuery(res);
+  }
+
+  @Test
   public final void testSortWithExpr1() throws Exception {
     // select l_linenumber, l_orderkey as sortkey from lineitem order by l_orderkey + 1;
     ResultSet res = executeQuery();
@@ -126,7 +140,7 @@ public class TestSortQuery extends QueryTestCaseBase {
     executeDDL("create_table_with_asc_desc_keys.sql", "table2");
 
     ResultSet res = executeQuery();
-    System.out.println(resultSetToString(res));
+    assertResultSet(res);
     cleanupQuery(res);
   }
 }

http://git-wip-us.apache.org/repos/asf/tajo/blob/b1e0e349/tajo-core/tajo-core-backend/src/test/resources/queries/TestSortQuery/testSortWithAlias2.sql
----------------------------------------------------------------------
diff --git a/tajo-core/tajo-core-backend/src/test/resources/queries/TestSortQuery/testSortWithAlias2.sql
b/tajo-core/tajo-core-backend/src/test/resources/queries/TestSortQuery/testSortWithAlias2.sql
new file mode 100644
index 0000000..7a56cac
--- /dev/null
+++ b/tajo-core/tajo-core-backend/src/test/resources/queries/TestSortQuery/testSortWithAlias2.sql
@@ -0,0 +1,9 @@
+select
+  lineitem.l_orderkey as l_orderkey,
+  count(l_partkey) as cnt
+from
+  lineitem a
+group by
+  lineitem.l_orderkey
+order by
+  lineitem.l_orderkey;
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/tajo/blob/b1e0e349/tajo-core/tajo-core-backend/src/test/resources/queries/TestSortQuery/testSortWithAlias3.sql
----------------------------------------------------------------------
diff --git a/tajo-core/tajo-core-backend/src/test/resources/queries/TestSortQuery/testSortWithAlias3.sql
b/tajo-core/tajo-core-backend/src/test/resources/queries/TestSortQuery/testSortWithAlias3.sql
new file mode 100644
index 0000000..446138b
--- /dev/null
+++ b/tajo-core/tajo-core-backend/src/test/resources/queries/TestSortQuery/testSortWithAlias3.sql
@@ -0,0 +1,11 @@
+select
+  nation.n_nationkey as n_nationkey,
+  customer.c_name as c_name,
+  count(nation.n_nationkey) as cnt
+from
+  nation inner join customer on n_nationkey = c_nationkey
+group by
+  nation.n_nationkey,
+  customer.c_name
+order by
+  n_nationkey, c_name;
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/tajo/blob/b1e0e349/tajo-core/tajo-core-backend/src/test/resources/results/TestSortQuery/testSortWithAlias2.result
----------------------------------------------------------------------
diff --git a/tajo-core/tajo-core-backend/src/test/resources/results/TestSortQuery/testSortWithAlias2.result
b/tajo-core/tajo-core-backend/src/test/resources/results/TestSortQuery/testSortWithAlias2.result
new file mode 100644
index 0000000..32c93ed
--- /dev/null
+++ b/tajo-core/tajo-core-backend/src/test/resources/results/TestSortQuery/testSortWithAlias2.result
@@ -0,0 +1,5 @@
+l_orderkey,cnt
+-------------------------------
+1,2
+2,1
+3,2
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/tajo/blob/b1e0e349/tajo-core/tajo-core-backend/src/test/resources/results/TestSortQuery/testSortWithAlias3.result
----------------------------------------------------------------------
diff --git a/tajo-core/tajo-core-backend/src/test/resources/results/TestSortQuery/testSortWithAlias3.result
b/tajo-core/tajo-core-backend/src/test/resources/results/TestSortQuery/testSortWithAlias3.result
new file mode 100644
index 0000000..6d78f11
--- /dev/null
+++ b/tajo-core/tajo-core-backend/src/test/resources/results/TestSortQuery/testSortWithAlias3.result
@@ -0,0 +1,7 @@
+n_nationkey,c_name,cnt
+-------------------------------
+1,Customer#000000003,1
+3,Customer#000000005,1
+4,Customer#000000004,1
+13,Customer#000000002,1
+15,Customer#000000001,1
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/tajo/blob/b1e0e349/tajo-core/tajo-core-backend/src/test/resources/results/TestSortQuery/testSortWithAscDescKeys.result
----------------------------------------------------------------------
diff --git a/tajo-core/tajo-core-backend/src/test/resources/results/TestSortQuery/testSortWithAscDescKeys.result
b/tajo-core/tajo-core-backend/src/test/resources/results/TestSortQuery/testSortWithAscDescKeys.result
new file mode 100644
index 0000000..93c2fcb
--- /dev/null
+++ b/tajo-core/tajo-core-backend/src/test/resources/results/TestSortQuery/testSortWithAscDescKeys.result
@@ -0,0 +1,26 @@
+col1,col2
+-------------------------------
+1,155190
+1,67310
+1,63700
+1,24027
+1,15635
+1,2132
+2,106170
+3,183095
+3,128449
+3,62143
+3,29380
+3,19036
+3,4297
+4,88035
+5,123927
+5,108570
+5,37531
+6,139636
+7,163073
+7,157238
+7,151894
+7,145243
+7,94780
+7,79251


Mime
View raw message