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 2013/12/15 16:46:29 UTC
git commit: TAJO-415: Some complex queries causes NPE and unlimited
recursions. (hyunsik)
Updated Branches:
refs/heads/master 62c49c05f -> a90895a74
TAJO-415: Some complex queries causes NPE and unlimited recursions. (hyunsik)
Project: http://git-wip-us.apache.org/repos/asf/incubator-tajo/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-tajo/commit/a90895a7
Tree: http://git-wip-us.apache.org/repos/asf/incubator-tajo/tree/a90895a7
Diff: http://git-wip-us.apache.org/repos/asf/incubator-tajo/diff/a90895a7
Branch: refs/heads/master
Commit: a90895a74a13c654a8d12345bbbdd8d3b08c604c
Parents: 62c49c0
Author: Hyunsik Choi <hy...@apache.org>
Authored: Mon Dec 16 00:45:49 2013 +0900
Committer: Hyunsik Choi <hy...@apache.org>
Committed: Mon Dec 16 00:45:49 2013 +0900
----------------------------------------------------------------------
CHANGES.txt | 12 +++-
.../org/apache/tajo/catalog/FunctionDesc.java | 6 +-
.../org/apache/tajo/engine/eval/FieldEval.java | 2 +-
.../join/GreedyHeuristicJoinOrderAlgorithm.java | 13 +++-
.../planner/rewrite/FilterPushDownRule.java | 26 ++++----
.../planner/rewrite/ProjectionPushDownRule.java | 30 +++++----
.../tajo/master/querymaster/QueryUnit.java | 4 +-
.../tajo/master/querymaster/SubQuery.java | 9 ++-
.../tajo/engine/query/TestCaseByCases.java | 66 ++++++++++++++++++++
.../src/test/queries/tajo415_case.sql | 33 ++++++++++
10 files changed, 167 insertions(+), 34 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/a90895a7/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 2f7d1db..87bbec5 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -146,11 +146,17 @@ Release 0.8.0 - unreleased
BUG FIXES
- TAJO-414: Fix bug of bit operations in decode() method of DateDatum class. (Keuntae Park via jihoon)
+ TAJO-415: Some complex queries causes NPE and unlimited recursions.
+ (hyunsik)
+
+ TAJO-414: Fix bug of bit operations in decode() method of DateDatum class.
+ (Keuntae Park via jihoon)
- TAJO-407: PostgreSQL-style cast should be higher operator priority. (hyunsik)
+ TAJO-407: PostgreSQL-style cast should be higher operator priority.
+ (hyunsik)
- TAJO-411: Fix Bug: createFromInt8's DATE type should be TIMESTAMP. (DaeMyung Kang via jihoon)
+ TAJO-411: Fix Bug: createFromInt8's DATE type should be TIMESTAMP.
+ (DaeMyung Kang via jihoon)
TAJO-390: Queries on history are expired ealier than a given expiry time.
(hyoungjunkim via hyunsik)
http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/a90895a7/tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/FunctionDesc.java
----------------------------------------------------------------------
diff --git a/tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/FunctionDesc.java b/tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/FunctionDesc.java
index 10f05dd..c64ac45 100644
--- a/tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/FunctionDesc.java
+++ b/tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/FunctionDesc.java
@@ -18,6 +18,7 @@
package org.apache.tajo.catalog;
+import com.google.common.base.Objects;
import com.google.gson.annotations.Expose;
import org.apache.tajo.json.GsonObject;
import org.apache.tajo.catalog.function.Function;
@@ -99,8 +100,9 @@ public class FunctionDesc implements ProtoObject<FunctionDescProto>, Cloneable,
return this.returnType;
}
- public static DataType [] newNoNameSchema(DataType ... types) {
- return types.clone();
+ @Override
+ public int hashCode() {
+ return Objects.hashCode(signature, params);
}
@Override
http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/a90895a7/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/eval/FieldEval.java
----------------------------------------------------------------------
diff --git a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/eval/FieldEval.java b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/eval/FieldEval.java
index 8c24104..6a840e4 100644
--- a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/eval/FieldEval.java
+++ b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/eval/FieldEval.java
@@ -84,7 +84,7 @@ public class FieldEval extends EvalNode implements Cloneable {
return column;
}
- public String getTableId() {
+ public String getQualifier() {
return column.getQualifier();
}
http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/a90895a7/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/join/GreedyHeuristicJoinOrderAlgorithm.java
----------------------------------------------------------------------
diff --git a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/join/GreedyHeuristicJoinOrderAlgorithm.java b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/join/GreedyHeuristicJoinOrderAlgorithm.java
index 5ac8b57..bd1b8d3 100644
--- a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/join/GreedyHeuristicJoinOrderAlgorithm.java
+++ b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/join/GreedyHeuristicJoinOrderAlgorithm.java
@@ -193,6 +193,7 @@ public class GreedyHeuristicJoinOrderAlgorithm implements JoinOrderAlgorithm {
}
}
+ // TODO - costs of other operator operators (e.g., group-by and sort) should be computed in proper manners.
public static double getCost(LogicalNode node) {
switch (node.getType()) {
@@ -229,8 +230,18 @@ public class GreedyHeuristicJoinOrderAlgorithm implements JoinOrderAlgorithm {
return Long.MAX_VALUE;
}
+ case UNION:
+ UnionNode unionNode = (UnionNode) node;
+ return getCost(unionNode.getLeftChild()) + getCost(unionNode.getRightChild());
+
+ case EXCEPT:
+ case INTERSECT:
+ throw new UnsupportedOperationException("getCost() does not support EXCEPT or INTERSECT yet");
+
default:
- return getCost(node);
+ // all binary operators (join, union, except, and intersect) are handled in the above cases.
+ // So, we need to handle only unary nodes in default.
+ return getCost(((UnaryNode) node).getChild());
}
}
}
\ No newline at end of file
http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/a90895a7/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/rewrite/FilterPushDownRule.java
----------------------------------------------------------------------
diff --git a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/rewrite/FilterPushDownRule.java b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/rewrite/FilterPushDownRule.java
index 0971444..7673253 100644
--- a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/rewrite/FilterPushDownRule.java
+++ b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/rewrite/FilterPushDownRule.java
@@ -105,13 +105,13 @@ public class FilterPushDownRule extends BasicLogicalPlanVisitor<Set<EvalNode>, L
// if both are fields
if (joinQual.getLeftExpr().getType() == EvalType.FIELD && joinQual.getRightExpr().getType() == EvalType.FIELD) {
- String leftTableName = ((FieldEval) joinQual.getLeftExpr()).getTableId();
- String rightTableName = ((FieldEval) joinQual.getRightExpr()).getTableId();
+ String leftTableName = ((FieldEval) joinQual.getLeftExpr()).getQualifier();
+ String rightTableName = ((FieldEval) joinQual.getRightExpr()).getQualifier();
List<String> nullSuppliers = Lists.newArrayList();
- String [] leftLineage = PlannerUtil.getRelationLineage(joinNode.getLeftChild());
- String [] rightLineage = PlannerUtil.getRelationLineage(joinNode.getRightChild());
- Set<String> leftTableSet = Sets.newHashSet(leftLineage);
- Set<String> rightTableSet = Sets.newHashSet(rightLineage);
+ Set<String> leftTableSet = Sets.newHashSet(PlannerUtil.getRelationLineageWithinQueryBlock(plan,
+ joinNode.getLeftChild()));
+ Set<String> rightTableSet = Sets.newHashSet(PlannerUtil.getRelationLineageWithinQueryBlock(plan,
+ joinNode.getRightChild()));
// some verification
if (joinType == JoinType.FULL_OUTER) {
@@ -127,13 +127,13 @@ public class FilterPushDownRule extends BasicLogicalPlanVisitor<Set<EvalNode>, L
}
} else if (joinType == JoinType.LEFT_OUTER) {
- nullSuppliers.add(((ScanNode)joinNode.getRightChild()).getTableName());
+ nullSuppliers.add(((RelationNode)joinNode.getRightChild()).getCanonicalName());
//verify that this null supplier is indeed in the right sub-tree
if (!rightTableSet.contains(nullSuppliers.get(0))) {
throw new InvalidQueryException("Incorrect Logical Query Plan with regard to outer join");
}
} else if (joinType == JoinType.RIGHT_OUTER) {
- if (((ScanNode)joinNode.getRightChild()).getTableName().equals(rightTableName)) {
+ if (((RelationNode)joinNode.getRightChild()).getCanonicalName().equals(rightTableName)) {
nullSuppliers.add(leftTableName);
} else {
nullSuppliers.add(rightTableName);
@@ -144,12 +144,12 @@ public class FilterPushDownRule extends BasicLogicalPlanVisitor<Set<EvalNode>, L
throw new InvalidQueryException("Incorrect Logical Query Plan with regard to outer join");
}
}
-
+
// retain in this outer join node's JoinQual those selection predicates
// related to the outer join's null supplier(s)
List<EvalNode> matched2 = Lists.newArrayList();
for (EvalNode eval : cnf) {
-
+
Set<Column> columnRefs = EvalTreeUtil.findDistinctRefColumns(eval);
Set<String> tableNames = Sets.newHashSet();
// getting distinct table references
@@ -158,20 +158,20 @@ public class FilterPushDownRule extends BasicLogicalPlanVisitor<Set<EvalNode>, L
tableNames.add(col.getQualifier());
}
}
-
+
//if the predicate involves any of the null suppliers
boolean shouldKeep=false;
Iterator<String> it2 = nullSuppliers.iterator();
while(it2.hasNext()){
if(tableNames.contains(it2.next()) == true) {
- shouldKeep = true;
+ shouldKeep = true;
}
}
if(shouldKeep == true) {
matched2.add(eval);
}
-
+
}
//merge the retained predicates and establish them in the current outer join node. Then remove them from the cnf
http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/a90895a7/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 1b0586e..d0e9a6f 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
@@ -271,26 +271,34 @@ public class ProjectionPushDownRule extends BasicLogicalPlanVisitor<ProjectionPu
Stack<LogicalNode> newStack = new Stack<LogicalNode>();
newStack.push(node);
PushDownContext newContext = new PushDownContext(subBlock);
+
+ newContext.upperRequired = new HashSet<Column>();
+
if (subBlock.hasProjection() && subBlock.getProjection().isAllProjected()
&& context.upperRequired.size() == 0) {
newContext.targetListManager = new TargetListManager(plan, subBlock.getProjectionNode().getTargets());
} else {
- List<Target> projectedTarget = new ArrayList<Target>();
- for (Target target : subBlock.getTargetListManager().getUnresolvedTargets()) {
- for (Column column : context.upperRequired) {
- if (column.hasQualifier() && !node.getTableName().equals(column.getQualifier())) {
- continue;
- }
- if (target.getColumnSchema().getColumnName().equalsIgnoreCase(column.getColumnName())) {
- projectedTarget.add(target);
+ if (!subBlock.hasGrouping()) {
+ List<Target> projectedTarget = new ArrayList<Target>();
+ for (Target target : subBlock.getTargetListManager().getUnresolvedTargets()) {
+ for (Column column : context.upperRequired) {
+ if (column.hasQualifier() && !node.getTableName().equals(column.getQualifier())) {
+ continue;
+ }
+ if (target.getColumnSchema().getColumnName().equalsIgnoreCase(column.getColumnName())) {
+ projectedTarget.add(target);
+ }
}
}
+ newContext.targetListManager = new TargetListManager(plan,
+ projectedTarget.toArray(new Target[projectedTarget.size()]));
+
+ } else {
+ newContext.targetListManager = new TargetListManager(plan,
+ subBlock.getTargetListManager().getUnresolvedTargets());
}
- newContext.targetListManager = new TargetListManager(plan,
- projectedTarget.toArray(new Target[projectedTarget.size()]));
}
- newContext.upperRequired = new HashSet<Column>();
newContext.upperRequired.addAll(PlannerUtil.targetToSchema(newContext.targetListManager.getTargets()).getColumns());
LogicalNode child = visitChild(newContext, plan, subRoot, newStack);
http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/a90895a7/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryUnit.java
----------------------------------------------------------------------
diff --git a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryUnit.java b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryUnit.java
index 1d40616..28f93fc 100644
--- a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryUnit.java
+++ b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryUnit.java
@@ -177,7 +177,9 @@ public class QueryUnit implements EventHandler<TaskEvent> {
s.add(s.size(), binary.getRightChild());
} else if (node instanceof ScanNode) {
scan.add((ScanNode)node);
- }
+ } else if (node instanceof TableSubQueryNode) {
+ s.add(((TableSubQueryNode) node).getSubQuery());
+ }
}
}
http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/a90895a7/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/SubQuery.java
----------------------------------------------------------------------
diff --git a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/SubQuery.java b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/SubQuery.java
index edaa463..7fedd4f 100644
--- a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/SubQuery.java
+++ b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/SubQuery.java
@@ -695,7 +695,8 @@ public class SubQuery implements EventHandler<SubQueryEvent> {
return maxTaskNum;
}
- public static long getInputVolume(MasterPlan masterPlan, QueryMasterTask.QueryMasterTaskContext context, ExecutionBlock execBlock) {
+ public static long getInputVolume(MasterPlan masterPlan, QueryMasterTask.QueryMasterTaskContext context,
+ ExecutionBlock execBlock) {
Map<String, TableDesc> tableMap = context.getTableDescMap();
if (masterPlan.isLeaf(execBlock)) {
ScanNode outerScan = execBlock.getScanNodes()[0];
@@ -705,7 +706,11 @@ public class SubQuery implements EventHandler<SubQueryEvent> {
long aggregatedVolume = 0;
for (ExecutionBlock childBlock : masterPlan.getChilds(execBlock)) {
SubQuery subquery = context.getSubQuery(childBlock.getId());
- aggregatedVolume += subquery.getTableStat().getNumBytes();
+ if (subquery == null || subquery.getState() != SubQueryState.SUCCEEDED) {
+ aggregatedVolume += getInputVolume(masterPlan, context, childBlock);
+ } else {
+ aggregatedVolume += subquery.getTableStat().getNumBytes();
+ }
}
return aggregatedVolume;
http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/a90895a7/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
new file mode 100644
index 0000000..7f301be
--- /dev/null
+++ b/tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/query/TestCaseByCases.java
@@ -0,0 +1,66 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.tajo.engine.query;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+import org.apache.tajo.TpchTestBase;
+import org.apache.tajo.util.FileUtil;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.io.File;
+import java.io.IOException;
+import java.sql.ResultSet;
+import java.util.List;
+import java.util.Map;
+
+import static org.junit.Assert.assertTrue;
+
+public class TestCaseByCases {
+ private static TpchTestBase tpch;
+
+ public TestCaseByCases() throws IOException {
+ super();
+ }
+
+ @BeforeClass
+ public static void setUp() throws Exception {
+ tpch = TpchTestBase.getInstance();
+ }
+
+ @Test
+ public final void testTAJO415Case() throws Exception {
+ ResultSet res = tpch.execute(FileUtil.readTextFile(new File("src/test/queries/tajo415_case.sql")));
+ try {
+ Map<Integer, List<Integer>> result = Maps.newHashMap();
+ result.put(1, Lists.newArrayList(1, 1));
+ result.put(2, Lists.newArrayList(2, 1));
+ result.put(3, Lists.newArrayList(3, 1));
+ result.put(4, Lists.newArrayList(0, 1));
+ result.put(5, Lists.newArrayList(0, 1));
+ while(res.next()) {
+ assertTrue(result.get(res.getInt(1)).get(0) == res.getInt(2));
+ assertTrue(result.get(res.getInt(1)).get(1) == res.getInt(3));
+ }
+ } finally {
+ res.close();
+ }
+ }
+}
http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/a90895a7/tajo-core/tajo-core-backend/src/test/queries/tajo415_case.sql
----------------------------------------------------------------------
diff --git a/tajo-core/tajo-core-backend/src/test/queries/tajo415_case.sql b/tajo-core/tajo-core-backend/src/test/queries/tajo415_case.sql
new file mode 100644
index 0000000..4a73b04
--- /dev/null
+++ b/tajo-core/tajo-core-backend/src/test/queries/tajo415_case.sql
@@ -0,0 +1,33 @@
+select
+ c_custkey,
+ o_orderkey,
+ a.cnt
+
+from (
+
+ select
+ c_custkey,
+ count(*) as cnt
+
+ from
+ customer
+
+ group by
+ c_custkey
+
+) a left outer join (
+
+ select
+ o_orderkey,
+ count(*) as cnt
+
+ from
+ orders
+
+ where
+ o_orderkey is not null
+
+ group by
+ o_orderkey
+
+) b on (a.c_custkey = b.o_orderkey);
\ No newline at end of file