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/01/30 18:01:17 UTC
git commit: TAJO-570: InvalidOperationException in outer join with
constant values.
Updated Branches:
refs/heads/master 567183fc2 -> 8d6e3785d
TAJO-570: InvalidOperationException in outer join with constant values.
Project: http://git-wip-us.apache.org/repos/asf/incubator-tajo/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-tajo/commit/8d6e3785
Tree: http://git-wip-us.apache.org/repos/asf/incubator-tajo/tree/8d6e3785
Diff: http://git-wip-us.apache.org/repos/asf/incubator-tajo/diff/8d6e3785
Branch: refs/heads/master
Commit: 8d6e3785d1b151456463961109387dd71657a8c6
Parents: 567183f
Author: Hyunsik Choi <hy...@apache.org>
Authored: Fri Jan 31 02:00:25 2014 +0900
Committer: Hyunsik Choi <hy...@apache.org>
Committed: Fri Jan 31 02:00:57 2014 +0900
----------------------------------------------------------------------
CHANGES.txt | 3 +
.../org/apache/tajo/datum/DatumFactory.java | 32 +++++++++++
.../org/apache/tajo/engine/eval/CastEval.java | 30 +---------
.../tajo/engine/planner/ExprAnnotator.java | 8 ++-
.../tajo/engine/planner/LogicalPlanner.java | 60 +++++++++-----------
.../planner/rewrite/ProjectionPushDownRule.java | 2 +-
.../apache/tajo/engine/query/TestJoinQuery.java | 12 ++++
.../testLeftOuterJoinWithConstantExpr3.sql | 15 +++++
.../testLeftOuterJoinWithConstantExpr3.result | 7 +++
9 files changed, 105 insertions(+), 64 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/8d6e3785/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 50b3695..69b4deb 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -236,6 +236,9 @@ Release 0.8.0 - unreleased
BUG FIXES
+ TAJO-570: InvalidOperationException in outer join with constant values.
+ (hyunsik)
+
TAJO-506: RawFile cannot support DATE type. (jinho)
TAJO-566: BIN/TAJO_DUMP makes wrong ddl script. (hyoungjunkim via hyunsik)
http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/8d6e3785/tajo-common/src/main/java/org/apache/tajo/datum/DatumFactory.java
----------------------------------------------------------------------
diff --git a/tajo-common/src/main/java/org/apache/tajo/datum/DatumFactory.java b/tajo-common/src/main/java/org/apache/tajo/datum/DatumFactory.java
index 784c039..c4f2b6c 100644
--- a/tajo-common/src/main/java/org/apache/tajo/datum/DatumFactory.java
+++ b/tajo-common/src/main/java/org/apache/tajo/datum/DatumFactory.java
@@ -344,4 +344,36 @@ public class DatumFactory {
public static Inet4Datum createInet4(String val) {
return new Inet4Datum(val);
}
+
+ public static Datum cast(Datum operandDatum, DataType target) {
+ switch (target.getType()) {
+ case BOOLEAN:
+ return DatumFactory.createBool(operandDatum.asBool());
+ case CHAR:
+ return DatumFactory.createChar(operandDatum.asChar());
+ case INT1:
+ case INT2:
+ return DatumFactory.createInt2(operandDatum.asInt2());
+ case INT4:
+ return DatumFactory.createInt4(operandDatum.asInt4());
+ case INT8:
+ return DatumFactory.createInt8(operandDatum.asInt8());
+ case FLOAT4:
+ return DatumFactory.createFloat4(operandDatum.asFloat4());
+ case FLOAT8:
+ return DatumFactory.createFloat8(operandDatum.asFloat8());
+ case TEXT:
+ return DatumFactory.createText(operandDatum.asTextBytes());
+ case DATE:
+ return DatumFactory.createDate(operandDatum);
+ case TIME:
+ return DatumFactory.createTime(operandDatum);
+ case TIMESTAMP:
+ return DatumFactory.createTimestamp(operandDatum);
+ case BLOB:
+ return DatumFactory.createBlob(operandDatum.asByteArray());
+ default:
+ throw new InvalidCastException("Cannot cast " + operandDatum.type() + " to " + target.getType());
+ }
+ }
}
http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/8d6e3785/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/eval/CastEval.java
----------------------------------------------------------------------
diff --git a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/eval/CastEval.java b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/eval/CastEval.java
index 9ff3df1..a024b01 100644
--- a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/eval/CastEval.java
+++ b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/eval/CastEval.java
@@ -56,35 +56,7 @@ public class CastEval extends EvalNode {
return operandDatum;
}
- switch (target.getType()) {
- case BOOLEAN:
- return DatumFactory.createBool(operandDatum.asBool());
- case CHAR:
- return DatumFactory.createChar(operandDatum.asChar());
- case INT1:
- case INT2:
- return DatumFactory.createInt2(operandDatum.asInt2());
- case INT4:
- return DatumFactory.createInt4(operandDatum.asInt4());
- case INT8:
- return DatumFactory.createInt8(operandDatum.asInt8());
- case FLOAT4:
- return DatumFactory.createFloat4(operandDatum.asFloat4());
- case FLOAT8:
- return DatumFactory.createFloat8(operandDatum.asFloat8());
- case TEXT:
- return DatumFactory.createText(operandDatum.asTextBytes());
- case DATE:
- return DatumFactory.createDate(operandDatum);
- case TIME:
- return DatumFactory.createTime(operandDatum);
- case TIMESTAMP:
- return DatumFactory.createTimestamp(operandDatum);
- case BLOB:
- return DatumFactory.createBlob(operandDatum.asByteArray());
- default:
- throw new InvalidCastException("Cannot cast " + operand.getValueType().getType() + " to " + target.getType());
- }
+ return DatumFactory.cast(operandDatum, target);
}
public String toString() {
http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/8d6e3785/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/ExprAnnotator.java
----------------------------------------------------------------------
diff --git a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/ExprAnnotator.java b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/ExprAnnotator.java
index c90325a..f25064f 100644
--- a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/ExprAnnotator.java
+++ b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/ExprAnnotator.java
@@ -487,7 +487,13 @@ public class ExprAnnotator extends BaseAlgebraVisitor<ExprAnnotator.Context, Eva
@Override
public EvalNode visitCastExpr(Context ctx, Stack<Expr> stack, CastExpr expr) throws PlanningException {
EvalNode child = super.visitCastExpr(ctx, stack, expr);
- return new CastEval(child, LogicalPlanner.convertDataType(expr.getTarget()));
+
+ if (child.getType() == EvalType.CONST) { // if it is a casting operation for a constant value
+ ConstEval constEval = (ConstEval) child; // it will be pre-computed and casted to a constant value
+ return new ConstEval(DatumFactory.cast(constEval.getValue(), LogicalPlanner.convertDataType(expr.getTarget())));
+ } else {
+ return new CastEval(child, LogicalPlanner.convertDataType(expr.getTarget()));
+ }
}
@Override
http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/8d6e3785/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 5ee3c69..3b4ec89 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
@@ -20,7 +20,6 @@ package org.apache.tajo.engine.planner;
import com.google.common.base.Joiner;
import com.google.common.collect.Lists;
-import com.google.common.collect.Sets;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.hadoop.conf.Configuration;
@@ -1423,29 +1422,17 @@ public class LogicalPlanner extends BaseAlgebraVisitor<LogicalPlanner.PlanContex
Util SECTION
===============================================================================================*/
- public static boolean checkIfBeEvaluatedAtGroupBy(EvalNode evalNode, GroupbyNode groupbyNode) {
+ public static boolean checkIfBeEvaluatedAtGroupBy(EvalNode evalNode, GroupbyNode node) {
Set<Column> columnRefs = EvalTreeUtil.findDistinctRefColumns(evalNode);
- if (!groupbyNode.getInSchema().containsAll(columnRefs)) {
- return false;
- }
-
- Set<String> tableIds = Sets.newHashSet();
- // getting distinct table references
- for (Column col : columnRefs) {
- if (!tableIds.contains(col.getQualifier())) {
- tableIds.add(col.getQualifier());
- }
- }
-
- if (tableIds.size() > 1) {
+ if (columnRefs.size() > 0 && !node.getInSchema().containsAll(columnRefs)) {
return false;
}
return true;
}
- public static boolean checkIfBeEvaluatedAtJoin(QueryBlock block, EvalNode evalNode, JoinNode joinNode,
+ public static boolean checkIfBeEvaluatedAtJoin(QueryBlock block, EvalNode evalNode, JoinNode node,
boolean isTopMostJoin) {
Set<Column> columnRefs = EvalTreeUtil.findDistinctRefColumns(evalNode);
@@ -1453,7 +1440,7 @@ public class LogicalPlanner extends BaseAlgebraVisitor<LogicalPlanner.PlanContex
return false;
}
- if (!joinNode.getInSchema().containsAll(columnRefs)) {
+ if (columnRefs.size() > 0 && !node.getInSchema().containsAll(columnRefs)) {
return false;
}
@@ -1461,14 +1448,15 @@ public class LogicalPlanner extends BaseAlgebraVisitor<LogicalPlanner.PlanContex
// at the topmost join operator.
// TODO - It's also valid that case-when is evalauted at the topmost outer operator.
// But, how can we know there is no further outer join operator after this node?
- if (checkCaseWhenWithOuterJoin(block, evalNode, isTopMostJoin)) {
- return true;
- } else {
+ if (!checkIfCaseWhenWithOuterJoinBeEvaluated(block, evalNode, isTopMostJoin)) {
return false;
}
+
+ return true;
}
- private static boolean checkCaseWhenWithOuterJoin(QueryBlock block, EvalNode evalNode, boolean isTopMostJoin) {
+ private static boolean checkIfCaseWhenWithOuterJoinBeEvaluated(QueryBlock block, EvalNode evalNode,
+ boolean isTopMostJoin) {
if (block.containsJoinType(JoinType.LEFT_OUTER) || block.containsJoinType(JoinType.RIGHT_OUTER)) {
Collection<CaseWhenEval> caseWhenEvals = EvalTreeUtil.findEvalsByType(evalNode, EvalType.CASE);
if (caseWhenEvals.size() > 0 && !isTopMostJoin) {
@@ -1478,32 +1466,38 @@ public class LogicalPlanner extends BaseAlgebraVisitor<LogicalPlanner.PlanContex
return true;
}
+ /**
+ * It checks if evalNode can be evaluated at this @{link RelationNode}.
+ */
public static boolean checkIfBeEvaluatedAtRelation(QueryBlock block, EvalNode evalNode, RelationNode node) {
Set<Column> columnRefs = EvalTreeUtil.findDistinctRefColumns(evalNode);
+ // aggregation functions cannot be evaluated in scan node
if (EvalTreeUtil.findDistinctAggFunction(evalNode).size() > 0) {
return false;
}
- if (node.getTableSchema().containsAll(columnRefs)) {
- // Why? - When a {case when} is used with outer join, case when must be evaluated at topmost outer join.
- if (block.containsJoinType(JoinType.LEFT_OUTER) || block.containsJoinType(JoinType.RIGHT_OUTER)) {
- Collection<CaseWhenEval> found = EvalTreeUtil.findEvalsByType(evalNode, EvalType.CASE);
- if (found.size() > 0) {
- return false;
- }
- }
- return true;
- } else {
+ if (columnRefs.size() > 0 && !node.getTableSchema().containsAll(columnRefs)) {
return false;
}
+
+ // Why? - When a {case when} is used with outer join, case when must be evaluated at topmost outer join.
+ if (block.containsJoinType(JoinType.LEFT_OUTER) || block.containsJoinType(JoinType.RIGHT_OUTER)) {
+ Collection<CaseWhenEval> found = EvalTreeUtil.findEvalsByType(evalNode, EvalType.CASE);
+ if (found.size() > 0) {
+ return false;
+ }
+ }
+
+ return true;
}
- public static boolean checkIfBeEvaluateAtThis(EvalNode evalNode, LogicalNode node) {
+ public static boolean checkIfBeEvaluatedAtThis(EvalNode evalNode, LogicalNode node) {
Set<Column> columnRefs = EvalTreeUtil.findDistinctRefColumns(evalNode);
- if (!node.getInSchema().containsAll(columnRefs)) {
+ if (columnRefs.size() > 0 && !node.getInSchema().containsAll(columnRefs)) {
return false;
}
+
return true;
}
}
http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/8d6e3785/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 c589910..391d4d1 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
@@ -295,7 +295,7 @@ public class ProjectionPushDownRule extends
if (context.targetListMgr.isResolved(referenceName)) {
finalTargets.add(new Target(new FieldEval(target.getNamedColumn())));
- } else if (LogicalPlanner.checkIfBeEvaluateAtThis(target.getEvalTree(), node)) {
+ } else if (LogicalPlanner.checkIfBeEvaluatedAtThis(target.getEvalTree(), node)) {
finalTargets.add(target);
context.targetListMgr.resolve(target);
resolvingCount++;
http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/8d6e3785/tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/query/TestJoinQuery.java
----------------------------------------------------------------------
diff --git a/tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/query/TestJoinQuery.java b/tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/query/TestJoinQuery.java
index fbb7fba..d7879cf 100644
--- a/tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/query/TestJoinQuery.java
+++ b/tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/query/TestJoinQuery.java
@@ -113,6 +113,18 @@ public class TestJoinQuery extends QueryTestCaseBase {
}
@Test
+ public final void testLeftOuterJoinWithConstantExpr3() throws Exception {
+ // outer join with constant projections
+ //
+ // select a.c_custkey, 123::INT8 as const_val, b.min_name from customer a
+ // left outer join ( select c_custkey, min(c_name) as min_name from customer group by c_custkey) b
+ // on a.c_custkey = b.c_custkey;
+ ResultSet res = executeQuery();
+ assertResultSet(res);
+ cleanupQuery(res);
+ }
+
+ @Test
public final void testRightOuterJoin1() throws Exception {
ResultSet res = executeQuery();
assertResultSet(res);
http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/8d6e3785/tajo-core/tajo-core-backend/src/test/resources/queries/TestJoinQuery/testLeftOuterJoinWithConstantExpr3.sql
----------------------------------------------------------------------
diff --git a/tajo-core/tajo-core-backend/src/test/resources/queries/TestJoinQuery/testLeftOuterJoinWithConstantExpr3.sql b/tajo-core/tajo-core-backend/src/test/resources/queries/TestJoinQuery/testLeftOuterJoinWithConstantExpr3.sql
new file mode 100644
index 0000000..03cdae2
--- /dev/null
+++ b/tajo-core/tajo-core-backend/src/test/resources/queries/TestJoinQuery/testLeftOuterJoinWithConstantExpr3.sql
@@ -0,0 +1,15 @@
+select
+ a.c_custkey,
+ 123::INT8 as const_val,
+ b.min_name
+from
+ customer a
+left outer join (
+ select
+ c_custkey,
+ min(c_name) as min_name
+ from customer
+ group by
+ c_custkey)
+ b
+on a.c_custkey = b.c_custkey;
\ No newline at end of file
http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/8d6e3785/tajo-core/tajo-core-backend/src/test/resources/results/TestJoinQuery/testLeftOuterJoinWithConstantExpr3.result
----------------------------------------------------------------------
diff --git a/tajo-core/tajo-core-backend/src/test/resources/results/TestJoinQuery/testLeftOuterJoinWithConstantExpr3.result b/tajo-core/tajo-core-backend/src/test/resources/results/TestJoinQuery/testLeftOuterJoinWithConstantExpr3.result
new file mode 100644
index 0000000..955cf44
--- /dev/null
+++ b/tajo-core/tajo-core-backend/src/test/resources/results/TestJoinQuery/testLeftOuterJoinWithConstantExpr3.result
@@ -0,0 +1,7 @@
+c_custkey,const_val,min_name
+-------------------------------
+1,123,Customer#000000001
+2,123,Customer#000000002
+3,123,Customer#000000003
+4,123,Customer#000000004
+5,123,Customer#000000005
\ No newline at end of file