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/27 03:54:22 UTC
git commit: TAJO-716: Using column names actually aliased in
aggregation functions can cause planning error.
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 <hy...@apache.org>
Authored: Thu Mar 27 11:54:06 2014 +0900
Committer: Hyunsik Choi <hy...@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