Repository: tajo Updated Branches: refs/heads/hbase_storage cc46aeacd -> 85627a520
TAJO-1126: Join condition including functions throws IllegalArgumentException. Closes #209 Project: http://git-wip-us.apache.org/repos/asf/tajo/repo Commit: http://git-wip-us.apache.org/repos/asf/tajo/commit/d9ba02bc Tree: http://git-wip-us.apache.org/repos/asf/tajo/tree/d9ba02bc Diff: http://git-wip-us.apache.org/repos/asf/tajo/diff/d9ba02bc Branch: refs/heads/hbase_storage Commit: d9ba02bc1adebf12660d0af192d3d3c5441e7cd1 Parents: 3c3bcce Author: Hyunsik Choi <[email protected]> Authored: Mon Oct 27 00:41:53 2014 -0700 Committer: Hyunsik Choi <[email protected]> Committed: Mon Oct 27 00:41:53 2014 -0700 ---------------------------------------------------------------------- CHANGES | 3 + .../java/org/apache/tajo/OverridableConf.java | 2 +- .../tajo/engine/query/QueryUnitRequest.java | 3 +- .../tajo/engine/query/QueryUnitRequestImpl.java | 4 +- .../main/java/org/apache/tajo/worker/Task.java | 2 +- .../org/apache/tajo/plan/expr/EvalTreeUtil.java | 97 +++++++++++++------- .../apache/tajo/plan/joinorder/JoinGraph.java | 5 +- .../plan/rewrite/rules/FilterPushDownRule.java | 10 +- .../org/apache/tajo/plan/util/PlannerUtil.java | 2 +- 9 files changed, 85 insertions(+), 43 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tajo/blob/d9ba02bc/CHANGES ---------------------------------------------------------------------- diff --git a/CHANGES b/CHANGES index a9a3127..f4df30d 100644 --- a/CHANGES +++ b/CHANGES @@ -21,6 +21,9 @@ Release 0.9.1 - unreleased BUG FIXES + TAJO-1126: Join condition including functions throws + IllegalArgumentException. (hyunsik) + TASKS http://git-wip-us.apache.org/repos/asf/tajo/blob/d9ba02bc/tajo-common/src/main/java/org/apache/tajo/OverridableConf.java ---------------------------------------------------------------------- diff --git a/tajo-common/src/main/java/org/apache/tajo/OverridableConf.java b/tajo-common/src/main/java/org/apache/tajo/OverridableConf.java index c9cf7fa..84be00e 100644 --- a/tajo-common/src/main/java/org/apache/tajo/OverridableConf.java +++ b/tajo-common/src/main/java/org/apache/tajo/OverridableConf.java @@ -51,7 +51,7 @@ import static org.apache.tajo.rpc.protocolrecords.PrimitiveProtos.KeyValueSetPro public class OverridableConf extends KeyValueSet { private static final Log LOG = LogFactory.getLog(OverridableConf.class); private ConfigType [] configTypes; - private TajoConf conf; + protected TajoConf conf; public OverridableConf(final TajoConf conf, ConfigType...configTypes) { this.conf = conf; http://git-wip-us.apache.org/repos/asf/tajo/blob/d9ba02bc/tajo-core/src/main/java/org/apache/tajo/engine/query/QueryUnitRequest.java ---------------------------------------------------------------------- diff --git a/tajo-core/src/main/java/org/apache/tajo/engine/query/QueryUnitRequest.java b/tajo-core/src/main/java/org/apache/tajo/engine/query/QueryUnitRequest.java index dc9a63d..3b0d60d 100644 --- a/tajo-core/src/main/java/org/apache/tajo/engine/query/QueryUnitRequest.java +++ b/tajo-core/src/main/java/org/apache/tajo/engine/query/QueryUnitRequest.java @@ -24,6 +24,7 @@ package org.apache.tajo.engine.query; import org.apache.tajo.QueryUnitAttemptId; import org.apache.tajo.catalog.proto.CatalogProtos; import org.apache.tajo.common.ProtoObject; +import org.apache.tajo.conf.TajoConf; import org.apache.tajo.engine.planner.enforce.Enforcer; import org.apache.tajo.engine.planner.global.DataChannel; import org.apache.tajo.ipc.TajoWorkerProtocol; @@ -44,7 +45,7 @@ public interface QueryUnitRequest extends ProtoObject<TajoWorkerProtocol.QueryUn public List<FetchImpl> getFetches(); public boolean shouldDie(); public void setShouldDie(); - public QueryContext getQueryContext(); + public QueryContext getQueryContext(TajoConf conf); public DataChannel getDataChannel(); public Enforcer getEnforcer(); } http://git-wip-us.apache.org/repos/asf/tajo/blob/d9ba02bc/tajo-core/src/main/java/org/apache/tajo/engine/query/QueryUnitRequestImpl.java ---------------------------------------------------------------------- diff --git a/tajo-core/src/main/java/org/apache/tajo/engine/query/QueryUnitRequestImpl.java b/tajo-core/src/main/java/org/apache/tajo/engine/query/QueryUnitRequestImpl.java index ef82427..1b89afd 100644 --- a/tajo-core/src/main/java/org/apache/tajo/engine/query/QueryUnitRequestImpl.java +++ b/tajo-core/src/main/java/org/apache/tajo/engine/query/QueryUnitRequestImpl.java @@ -186,7 +186,7 @@ public class QueryUnitRequestImpl implements QueryUnitRequest { fetches.add(fetch); } - public QueryContext getQueryContext() { + public QueryContext getQueryContext(TajoConf conf) { QueryUnitRequestProtoOrBuilder p = viaProto ? proto : builder; if (queryContext != null) { return queryContext; @@ -194,7 +194,7 @@ public class QueryUnitRequestImpl implements QueryUnitRequest { if (!p.hasQueryContext()) { return null; } - this.queryContext = new QueryContext(new TajoConf(), p.getQueryContext()); + this.queryContext = new QueryContext(conf, p.getQueryContext()); return this.queryContext; } http://git-wip-us.apache.org/repos/asf/tajo/blob/d9ba02bc/tajo-core/src/main/java/org/apache/tajo/worker/Task.java ---------------------------------------------------------------------- diff --git a/tajo-core/src/main/java/org/apache/tajo/worker/Task.java b/tajo-core/src/main/java/org/apache/tajo/worker/Task.java index 2632415..3858c96 100644 --- a/tajo-core/src/main/java/org/apache/tajo/worker/Task.java +++ b/tajo-core/src/main/java/org/apache/tajo/worker/Task.java @@ -149,7 +149,7 @@ public class Task { this.taskId = taskId; this.systemConf = executionBlockContext.getConf(); - this.queryContext = request.getQueryContext(); + this.queryContext = request.getQueryContext(systemConf); this.executionBlockContext = executionBlockContext; this.taskDir = StorageUtil.concatPath(baseDir, taskId.getQueryUnitId().getId() + "_" + taskId.getId()); http://git-wip-us.apache.org/repos/asf/tajo/blob/d9ba02bc/tajo-plan/src/main/java/org/apache/tajo/plan/expr/EvalTreeUtil.java ---------------------------------------------------------------------- diff --git a/tajo-plan/src/main/java/org/apache/tajo/plan/expr/EvalTreeUtil.java b/tajo-plan/src/main/java/org/apache/tajo/plan/expr/EvalTreeUtil.java index 1f3f2ab..f5c2cbd 100644 --- a/tajo-plan/src/main/java/org/apache/tajo/plan/expr/EvalTreeUtil.java +++ b/tajo-plan/src/main/java/org/apache/tajo/plan/expr/EvalTreeUtil.java @@ -264,7 +264,7 @@ public class EvalTreeUtil { * @return True if it is join condition. */ public static boolean isJoinQual(EvalNode expr, boolean includeThetaJoin) { - return isJoinQual(null, expr, includeThetaJoin); + return isJoinQual(null, null, null, expr, includeThetaJoin); } /** @@ -281,12 +281,16 @@ public class EvalTreeUtil { * from different two tables" instead of the first rule. * * @param block if block is not null, it tracks the lineage of aliased name derived from complex expressions. + * @param leftSchema Schema to be used to check if columns belong to different relations + * @param rightSchema Schema to be used to check if columns belong to different relations * @param expr EvalNode to be evaluated * @param includeThetaJoin If true, it will return equi as well as non-equi join conditions. * Otherwise, it only returns equi-join conditions. * @return True if it is join condition. */ - public static boolean isJoinQual(@Nullable LogicalPlan.QueryBlock block, EvalNode expr, boolean includeThetaJoin) { + public static boolean isJoinQual(@Nullable LogicalPlan.QueryBlock block, + @Nullable Schema leftSchema, @Nullable Schema rightSchema, + EvalNode expr, boolean includeThetaJoin) { if (expr instanceof BinaryEval) { boolean joinComparator; @@ -308,47 +312,74 @@ public class EvalTreeUtil { Column leftColumn = leftColumns.iterator().next(); Column rightColumn = rightColumns.iterator().next(); - String leftQualifier = CatalogUtil.extractQualifier(leftColumn.getQualifiedName()); - String rightQualifier = CatalogUtil.extractQualifier(rightColumn.getQualifiedName()); - - // if block is given, it will track an original expression of each term in order to decide whether - // this expression is a join condition, or not. + // ensure if both column belong to different tables if (block != null) { - boolean leftQualified = CatalogUtil.isFQColumnName(leftColumn.getQualifiedName()); - boolean rightQualified = CatalogUtil.isFQColumnName(rightColumn.getQualifiedName()); + ensureColumnsOfDifferentTables = isJoinQualWithOnlyColumns(block, leftColumn, rightColumn); + } else if (leftSchema != null && rightSchema != null) { + ensureColumnsOfDifferentTables = isJoinQualwithSchemas(leftSchema, rightSchema, leftColumn, rightColumn); + } else { + ensureColumnsOfDifferentTables = isJoinQualWithOnlyColumns(block, leftColumn, rightColumn); + } + } - if (!leftQualified) { // if left one is aliased name + return joinComparator && isBothTermFields && ensureColumnsOfDifferentTables; + } else { + return false; + } + } - // getting original expression of left term - NamedExpr rawExpr = block.getNamedExprsManager().getNamedExpr(leftColumn.getQualifiedName()); - Set<ColumnReferenceExpr> foundColumns = ExprFinder.finds(rawExpr.getExpr(), OpType.Column); + private static boolean isJoinQualwithSchemas(Schema leftSchema, Schema rightSchema, Column left, Column right) { - // ensure there is only one column of an original expression - if (foundColumns.size() == 1) { - leftQualifier = CatalogUtil.extractQualifier(foundColumns.iterator().next().getCanonicalName()); - } - } - if (!rightQualified) { // if right one is aliased name + boolean duplicated = leftSchema.contains(left) && rightSchema.contains(left); + duplicated |= leftSchema.contains(right) && rightSchema.contains(right); - // getting original expression of right term - NamedExpr rawExpr = block.getNamedExprsManager().getNamedExpr(rightColumn.getQualifiedName()); - Set<ColumnReferenceExpr> foundColumns = ExprFinder.finds(rawExpr.getExpr(), OpType.Column); + if (duplicated) { + return false; + } - // ensure there is only one column of an original expression - if (foundColumns.size() == 1) { - rightQualifier = CatalogUtil.extractQualifier(foundColumns.iterator().next().getCanonicalName()); - } - } - } + boolean isJoinQual = leftSchema.contains(left) && rightSchema.contains(right); + isJoinQual |= leftSchema.contains(right) && rightSchema.contains(left); - // if columns of both term is different to each other, it will be true. - ensureColumnsOfDifferentTables = !leftQualifier.equals(rightQualifier); + return isJoinQual; + } + + private static boolean isJoinQualWithOnlyColumns(@Nullable LogicalPlan.QueryBlock block, + Column left, Column right) { + String leftQualifier = CatalogUtil.extractQualifier(left.getQualifiedName()); + String rightQualifier = CatalogUtil.extractQualifier(right.getQualifiedName()); + + // if block is given, it will track an original expression of each term in order to decide whether + // this expression is a join condition, or not. + if (block != null) { + boolean leftQualified = CatalogUtil.isFQColumnName(left.getQualifiedName()); + boolean rightQualified = CatalogUtil.isFQColumnName(right.getQualifiedName()); + + if (!leftQualified) { // if left one is aliased name + + // getting original expression of left term + NamedExpr rawExpr = block.getNamedExprsManager().getNamedExpr(left.getQualifiedName()); + Set<ColumnReferenceExpr> foundColumns = ExprFinder.finds(rawExpr.getExpr(), OpType.Column); + + // ensure there is only one column of an original expression + if (foundColumns.size() == 1) { + leftQualifier = CatalogUtil.extractQualifier(foundColumns.iterator().next().getCanonicalName()); + } } + if (!rightQualified) { // if right one is aliased name - return joinComparator && isBothTermFields && ensureColumnsOfDifferentTables; - } else { - return false; + // getting original expression of right term + NamedExpr rawExpr = block.getNamedExprsManager().getNamedExpr(right.getQualifiedName()); + Set<ColumnReferenceExpr> foundColumns = ExprFinder.finds(rawExpr.getExpr(), OpType.Column); + + // ensure there is only one column of an original expression + if (foundColumns.size() == 1) { + rightQualifier = CatalogUtil.extractQualifier(foundColumns.iterator().next().getCanonicalName()); + } + } } + + // if columns of both term is different to each other, it will be true. + return !leftQualifier.equals(rightQualifier); } static boolean isSingleColumn(EvalNode evalNode) { http://git-wip-us.apache.org/repos/asf/tajo/blob/d9ba02bc/tajo-plan/src/main/java/org/apache/tajo/plan/joinorder/JoinGraph.java ---------------------------------------------------------------------- diff --git a/tajo-plan/src/main/java/org/apache/tajo/plan/joinorder/JoinGraph.java b/tajo-plan/src/main/java/org/apache/tajo/plan/joinorder/JoinGraph.java index f7fc30e..72e9b1d 100644 --- a/tajo-plan/src/main/java/org/apache/tajo/plan/joinorder/JoinGraph.java +++ b/tajo-plan/src/main/java/org/apache/tajo/plan/joinorder/JoinGraph.java @@ -123,7 +123,10 @@ public class JoinGraph extends SimpleUndirectedGraph<String, JoinEdge> { Set<EvalNode> cnf = Sets.newHashSet(AlgebraicUtil.toConjunctiveNormalFormArray(joinNode.getJoinQual())); for (EvalNode singleQual : cnf) { - if (EvalTreeUtil.isJoinQual(block, singleQual, true)) { + if (EvalTreeUtil.isJoinQual(block, + joinNode.getLeftChild().getOutSchema(), + joinNode.getRightChild().getOutSchema(), + singleQual, true)) { String[] relations = guessRelationsFromJoinQual(block, (BinaryEval) singleQual); String leftExprRelName = relations[0]; String rightExprRelName = relations[1]; http://git-wip-us.apache.org/repos/asf/tajo/blob/d9ba02bc/tajo-plan/src/main/java/org/apache/tajo/plan/rewrite/rules/FilterPushDownRule.java ---------------------------------------------------------------------- diff --git a/tajo-plan/src/main/java/org/apache/tajo/plan/rewrite/rules/FilterPushDownRule.java b/tajo-plan/src/main/java/org/apache/tajo/plan/rewrite/rules/FilterPushDownRule.java index 31063bf..ed410f9 100644 --- a/tajo-plan/src/main/java/org/apache/tajo/plan/rewrite/rules/FilterPushDownRule.java +++ b/tajo-plan/src/main/java/org/apache/tajo/plan/rewrite/rules/FilterPushDownRule.java @@ -154,7 +154,8 @@ public class FilterPushDownRule extends BasicLogicalPlanVisitor<FilterPushDownCo } @Override - public LogicalNode visitJoin(FilterPushDownContext context, LogicalPlan plan, LogicalPlan.QueryBlock block, JoinNode joinNode, + public LogicalNode visitJoin(FilterPushDownContext context, LogicalPlan plan, LogicalPlan.QueryBlock block, + JoinNode joinNode, Stack<LogicalNode> stack) throws PlanningException { // here we should stop selection pushdown on the null supplying side(s) of an outer join // get the two operands of the join operation as well as the join type @@ -238,7 +239,10 @@ public class FilterPushDownRule extends BasicLogicalPlanVisitor<FilterPushDownCo List<EvalNode> removedFromFilter = new ArrayList<EvalNode>(); for (EvalNode eachEval: context.pushingDownFilters) { - if (EvalTreeUtil.isJoinQual(block, eachEval, true)) { + if (EvalTreeUtil.isJoinQual(block, + joinNode.getLeftChild().getOutSchema(), + joinNode.getRightChild().getOutSchema(), + eachEval, true)) { outerJoinPredicationEvals.add(eachEval); removedFromFilter.add(eachEval); } else { @@ -584,7 +588,7 @@ public class FilterPushDownRule extends BasicLogicalPlanVisitor<FilterPushDownCo BiMap<EvalNode, EvalNode> matched = HashBiMap.create(); for (EvalNode eval : context.pushingDownFilters) { - if (ignoreJoin && EvalTreeUtil.isJoinQual(block, eval, true)) { + if (ignoreJoin && EvalTreeUtil.isJoinQual(block, null, null, eval, true)) { notMatched.add(eval); continue; } http://git-wip-us.apache.org/repos/asf/tajo/blob/d9ba02bc/tajo-plan/src/main/java/org/apache/tajo/plan/util/PlannerUtil.java ---------------------------------------------------------------------- diff --git a/tajo-plan/src/main/java/org/apache/tajo/plan/util/PlannerUtil.java b/tajo-plan/src/main/java/org/apache/tajo/plan/util/PlannerUtil.java index a7175a1..5fe1515 100644 --- a/tajo-plan/src/main/java/org/apache/tajo/plan/util/PlannerUtil.java +++ b/tajo-plan/src/main/java/org/apache/tajo/plan/util/PlannerUtil.java @@ -597,7 +597,7 @@ public class PlannerUtil { @Override public void visit(EvalNode node) { - if (EvalTreeUtil.isJoinQual(node, includeThetaJoin)) { + if (EvalTreeUtil.isJoinQual(null, schemas[0], schemas[1], node, includeThetaJoin)) { BinaryEval binaryEval = (BinaryEval) node; Column[] pair = new Column[2];
