Return-Path: X-Original-To: apmail-tajo-commits-archive@minotaur.apache.org Delivered-To: apmail-tajo-commits-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id B010E10C23 for ; Thu, 27 Mar 2014 02:54:25 +0000 (UTC) Received: (qmail 77730 invoked by uid 500); 27 Mar 2014 02:54:25 -0000 Delivered-To: apmail-tajo-commits-archive@tajo.apache.org Received: (qmail 77691 invoked by uid 500); 27 Mar 2014 02:54:23 -0000 Mailing-List: contact commits-help@tajo.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@tajo.apache.org Delivered-To: mailing list commits@tajo.apache.org Received: (qmail 77684 invoked by uid 99); 27 Mar 2014 02:54:22 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 27 Mar 2014 02:54:22 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id 409A0836F9A; Thu, 27 Mar 2014 02:54:22 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit From: hyunsik@apache.org To: commits@tajo.apache.org Message-Id: <555a63fcf65d4b2ab7a19d0d329c2a5f@git.apache.org> X-Mailer: ASF-Git Admin Mailer 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 +0000 (UTC) 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 Authored: Thu Mar 27 11:54:06 2014 +0900 Committer: Hyunsik Choi 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 nameToRelationMap = TUtil.newHashMap(); + private final Map canonicalNameToRelationMap = TUtil.newHashMap(); + private final Map> aliasMap = TUtil.newHashMap(); private final Map> 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 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 { } 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 { } 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