You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@tajo.apache.org by jihoonson <gi...@git.apache.org> on 2015/02/20 04:55:01 UTC
[GitHub] tajo pull request: TAJO-1350: Refactor FilterPushDownRule::visitJo...
GitHub user jihoonson opened a pull request:
https://github.com/apache/tajo/pull/384
TAJO-1350: Refactor FilterPushDownRule::visitJoin() into well-defined, small methods
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jihoonson/tajo-2 TAJO-1350
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/tajo/pull/384.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #384
----
commit cf960f99212ebd7c631071af7508036f70e0a210
Author: Jihoon Son <ji...@apache.org>
Date: 2015-02-19T12:57:04Z
TAJO-1350
commit 1bf85f60280dcaeb8b14323530bf3c9fd6b9aba6
Author: Jihoon Son <ji...@apache.org>
Date: 2015-02-20T03:13:22Z
TAJO-1350
commit 5a4e55531aa1c797eead8a940138f61d79ccc0be
Author: Jihoon Son <ji...@apache.org>
Date: 2015-02-20T03:23:39Z
TAJO-1350
commit 1bad40b88719da67adf3aa5c103879135f536aee
Author: Jihoon Son <ji...@apache.org>
Date: 2015-02-20T03:46:15Z
TAJO-1350
commit 8b1cd6441ebe6794a3675bd4a64b652af82f4356
Author: Jihoon Son <ji...@apache.org>
Date: 2015-02-20T03:52:21Z
TAJO-1350
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1350: Refactor FilterPushDownRule::visitJo...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/384#discussion_r26447163
--- Diff: tajo-plan/src/main/java/org/apache/tajo/plan/LogicalPlanner.java ---
@@ -1978,26 +1979,18 @@ public static boolean checkIfBeEvaluatedAtJoin(QueryBlock block, EvalNode evalNo
return false;
}
- // When a 'case-when' is used with outer join, the case-when expression must be evaluated
- // 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 (containsOuterJoin(block)) {
- if (!isTopMostJoin) {
- Collection<EvalNode> found = EvalTreeUtil.findOuterJoinSensitiveEvals(evalNode);
- if (found.size() > 0) {
- return false;
- }
- }
+ /*
+ * For outer joins, only predicates which are specified at the on clause can be evaluated during processing join.
+ * Other predicates from the where clause must be evaluated after the join.
+ * The below code will be modified after improving join operators to keep join filters by themselves (TAJO-1310).
+ */
+ if (PlannerUtil.isOuterJoin(node.getJoinType()) && !isOnPredicate) {
--- End diff --
I'm concerning that this change does not consider the original purpose. Please look over TAJO-428. This change is for workaround of TAJO-428 bug.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1350: Refactor FilterPushDownRule::visitJo...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:
https://github.com/apache/tajo/pull/384#issuecomment-86696028
+1
LGTM!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1350: Refactor FilterPushDownRule::visitJo...
Posted by blrunner <gi...@git.apache.org>.
Github user blrunner commented on a diff in the pull request:
https://github.com/apache/tajo/pull/384#discussion_r27196435
--- Diff: tajo-plan/src/main/java/org/apache/tajo/plan/LogicalPlanner.java ---
@@ -1972,6 +1976,52 @@ public static boolean checkIfBeEvaluatedAtGroupBy(EvalNode evalNode, GroupbyNode
return true;
}
+ public static boolean isEvaluatableJoinQual(QueryBlock block, EvalNode evalNode, JoinNode node,
+ boolean isOnPredicate, boolean isTopMostJoin) {
+
+ if (checkIfBeEvaluatedAtJoin(block, evalNode, node, isTopMostJoin)) {
+
+ if (isNonEquiThetaJoinQual(block, node, evalNode)) {
+ return false;
+ }
+
+ if (PlannerUtil.isOuterJoin(node.getJoinType())) {
+ /*
--- End diff --
It seems to exceed 120 columns.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1350: Refactor FilterPushDownRule::visitJo...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on the pull request:
https://github.com/apache/tajo/pull/384#issuecomment-84762423
Rebased.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1350: Refactor FilterPushDownRule::visitJo...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/384#discussion_r26092914
--- Diff: tajo-plan/src/main/java/org/apache/tajo/plan/LogicalPlanner.java ---
@@ -1978,26 +1979,18 @@ public static boolean checkIfBeEvaluatedAtJoin(QueryBlock block, EvalNode evalNo
return false;
}
- // When a 'case-when' is used with outer join, the case-when expression must be evaluated
- // 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 (containsOuterJoin(block)) {
- if (!isTopMostJoin) {
- Collection<EvalNode> found = EvalTreeUtil.findOuterJoinSensitiveEvals(evalNode);
- if (found.size() > 0) {
- return false;
- }
- }
+ /*
+ * For outer joins, only predicates which are specified at the on clause can be evaluated during processing join.
+ * Other predicates from the where clause must be evaluated after the join.
+ * The below code will be modified after improving join operators to keep join filters by themselves (TAJO-1310).
+ */
+ if (PlannerUtil.isOuterJoin(node.getJoinType()) && !isOnPredicate) {
--- End diff --
Predicates must be checked at every join node rather than the topmost one. Thus, I removed the topMostJoin flag.
The newly added line is for outer joins. As you know, in the case of the outer joins, only the predicates from the on clauses can be the join conditions. Other predicates from the where clauses are join filters.
Currently, we maintain join filters in separate selection nodes rather than the join nodes themselves. So, I prevent the filters being pushed down to outer join nodes. This will be improved when TAJO-1310 is resolved.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1350: Refactor FilterPushDownRule::visitJo...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/384#discussion_r25909557
--- Diff: tajo-plan/src/main/java/org/apache/tajo/plan/LogicalPlanner.java ---
@@ -1963,7 +1964,7 @@ public static boolean checkIfBeEvaluatedAtGroupBy(EvalNode evalNode, GroupbyNode
}
public static boolean checkIfBeEvaluatedAtJoin(QueryBlock block, EvalNode evalNode, JoinNode node,
- boolean isTopMostJoin) {
+ boolean isOnPredicate) {
--- End diff --
```isOnPredicate``` seems to be a flag to indicate if a predicate is ON clause?
But, it seems to be inconsistent with its usecases. In ProjectionPushDownRule, TRUE is given to this parameter. But, in the line 1153, this value is true even though they are not placed ON clause.
Could you let me know your exact intention about this change?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1350: Refactor FilterPushDownRule::visitJo...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/384#discussion_r26644814
--- Diff: tajo-plan/src/main/java/org/apache/tajo/plan/rewrite/rules/FilterPushDownRule.java ---
@@ -355,20 +230,144 @@ public LogicalNode visitJoin(FilterPushDownContext context, LogicalPlan plan, Lo
if (joinNode.getJoinType() == JoinType.CROSS) {
joinNode.setJoinType(JoinType.INNER);
}
- context.pushingDownFilters.removeAll(matched);
}
- context.pushingDownFilters.addAll(outerJoinFilterEvalsExcludePredication);
- context.pushingDownFilters.addAll(thetaJoinFilter);
+ context.pushingDownFilters.removeAll(matched);
return joinNode;
}
+ private static Set<EvalNode> extractNonPushableJoinQuals(final LogicalPlan plan,
+ final LogicalPlan.QueryBlock block,
+ final JoinNode joinNode,
+ final Set<EvalNode> onPredicates,
+ final Set<EvalNode> wherePredicates)
+ throws PlanningException {
+ Set<EvalNode> nonPushableQuals = TUtil.newHashSet();
+ // TODO: non-equi theta join quals must not be pushed until TAJO-742 is resolved.
+ nonPushableQuals.addAll(extractNonEquiThetaJoinQuals(wherePredicates, block, joinNode));
+
+ // for outer joins
+ if (PlannerUtil.isOuterJoin(joinNode.getJoinType())) {
+ nonPushableQuals.addAll(extractNonPushableOuterJoinQuals(plan, onPredicates, wherePredicates, joinNode));
+ }
+ return nonPushableQuals;
+ }
+
+ /**
+ * For outer joins, pushable predicates can be decided based on their locations in the SQL and types of referencing relations.
--- End diff --
It seems to exceed 120 columns.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1350: Refactor FilterPushDownRule::visitJo...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on the pull request:
https://github.com/apache/tajo/pull/384#issuecomment-81262357
@hyunsik thanks for your comment.
I've fixed it. Please review the patch.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1350: Refactor FilterPushDownRule::visitJo...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/384#discussion_r26089560
--- Diff: tajo-plan/src/main/java/org/apache/tajo/plan/LogicalPlanner.java ---
@@ -1978,26 +1979,18 @@ public static boolean checkIfBeEvaluatedAtJoin(QueryBlock block, EvalNode evalNo
return false;
}
- // When a 'case-when' is used with outer join, the case-when expression must be evaluated
- // 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 (containsOuterJoin(block)) {
- if (!isTopMostJoin) {
- Collection<EvalNode> found = EvalTreeUtil.findOuterJoinSensitiveEvals(evalNode);
- if (found.size() > 0) {
- return false;
- }
- }
+ /*
+ * For outer joins, only predicates which are specified at the on clause can be evaluated during processing join.
+ * Other predicates from the where clause must be evaluated after the join.
+ * The below code will be modified after improving join operators to keep join filters by themselves (TAJO-1310).
+ */
+ if (PlannerUtil.isOuterJoin(node.getJoinType()) && !isOnPredicate) {
--- End diff --
Could explain your intension about this change? It would be great if I can hear what the problem (or motivation) is and what your solution is.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1350: Refactor FilterPushDownRule::visitJo...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on the pull request:
https://github.com/apache/tajo/pull/384#issuecomment-86361068
Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1350: Refactor FilterPushDownRule::visitJo...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:
https://github.com/apache/tajo/pull/384#issuecomment-83848989
Could you trigger CI again?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1350: Refactor FilterPushDownRule::visitJo...
Posted by blrunner <gi...@git.apache.org>.
Github user blrunner commented on the pull request:
https://github.com/apache/tajo/pull/384#issuecomment-86415309
+1
Thanks for your efforts.
Your works will be helpful to other contributors.
And I leaved a trivial comment. Before you push it, you need to check 120 columns at LogicalPlanner::isEvaluatableJoinQual. :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1350: Refactor FilterPushDownRule::visitJo...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on the pull request:
https://github.com/apache/tajo/pull/384#issuecomment-86791563
Thanks for all your valuable reviews. I'll commit shortly.
@sirpkt, I also think that the selection should be pushed down to after the first join. This will improve the query performance.
To resolve this problem, I think that we need to add another filter pushdown optimization phase after join ordering. Please note that the filter pushdown phase is currently called only before join ordering.
After join ordering, the best position of each predicate will be differernt. So, it seems the problem of our query optimization rather than just a problem of filter pushdown. I'll create another issue.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1350: Refactor FilterPushDownRule::visitJo...
Posted by sirpkt <gi...@git.apache.org>.
Github user sirpkt commented on the pull request:
https://github.com/apache/tajo/pull/384#issuecomment-86771989
Oh above example may not be suitable.
Forget it :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1350: Refactor FilterPushDownRule::visitJo...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:
https://github.com/apache/tajo/pull/384#issuecomment-82821794
I leave some trivial comments on some java docs. Others are all good. Especially, extractNonPushableOuterJoinQuals looks intuitive.
After you reflect my comment, I'll finish the review.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1350: Refactor FilterPushDownRule::visitJo...
Posted by sirpkt <gi...@git.apache.org>.
Github user sirpkt commented on the pull request:
https://github.com/apache/tajo/pull/384#issuecomment-86769740
+1
By the way, during the test, I found that there still exists some more room to improve push filter down.
{code}
select
n1.n_nationkey,
n1.n_name,
n2.n_name
from nation n1, nation n2, nation n3 where (n1.n_nationkey in (1,2) or n2.n_nationkey in (2)) and n2.n_nationkey = n3.n_nationkey and n1.n_nationkey = n2.n_nationkey
order by n1.n_nationkey;
{code}
For above query, resulting optimized query plan is
{code}
PROJECTION(5)
=> Targets: default.n1.n_nationkey (INT4), default.n1.n_name (TEXT), default.n2.n_name (TEXT)
=> out schema: {(3) default.n1.n_nationkey (INT4), default.n1.n_name (TEXT), default.n2.n_name (TEXT)}
=> in schema: {(4) default.n1.n_nationkey (INT4), default.n1.n_name (TEXT), default.n2.n_name (TEXT), default.n2.n_nationkey (INT4)}
SORT(4)
=> Sort Keys: default.n1.n_nationkey (INT4) (asc)
SELECTION(3)
=> Search Cond: default.n1.n_nationkey (INT4) IN (1, 2) OR default.n2.n_nationkey (INT4) IN (2)
JOIN(10)(INNER)
=> Join Cond: default.n2.n_nationkey (INT4) = default.n3.n_nationkey (INT4)
=> target list: default.n1.n_nationkey (INT4), default.n1.n_name (TEXT), default.n2.n_name (TEXT), default.n2.n_nationkey (INT4)
=> out schema: {(4) default.n1.n_nationkey (INT4), default.n1.n_name (TEXT), default.n2.n_name (TEXT), default.n2.n_nationkey (INT4)}
=> in schema: {(5) default.n1.n_nationkey (INT4), default.n1.n_name (TEXT), default.n2.n_name (TEXT), default.n2.n_nationkey (INT4), default.n3.n_nationkey (INT4)}
SCAN(2) on default.nation as n3
=> target list: default.n3.n_nationkey (INT4)
=> out schema: {(1) default.n3.n_nationkey (INT4)}
=> in schema: {(4) default.n3.n_nationkey (INT4), default.n3.n_name (TEXT), default.n3.n_regionkey (INT4), default.n3.n_comment (TEXT)}
JOIN(9)(INNER)
=> Join Cond: default.n1.n_nationkey (INT4) = default.n2.n_nationkey (INT4)
=> target list: default.n1.n_nationkey (INT4), default.n1.n_name (TEXT), default.n2.n_name (TEXT), default.n2.n_nationkey (INT4)
=> out schema: {(4) default.n1.n_nationkey (INT4), default.n1.n_name (TEXT), default.n2.n_name (TEXT), default.n2.n_nationkey (INT4)}
=> in schema: {(4) default.n1.n_nationkey (INT4), default.n1.n_name (TEXT), default.n2.n_name (TEXT), default.n2.n_nationkey (INT4)}
SCAN(1) on default.nation as n2
=> target list: default.n2.n_name (TEXT), default.n2.n_nationkey (INT4)
=> out schema: {(2) default.n2.n_name (TEXT), default.n2.n_nationkey (INT4)}
=> in schema: {(4) default.n2.n_nationkey (INT4), default.n2.n_name (TEXT), default.n2.n_regionkey (INT4), default.n2.n_comment (TEXT)}
SCAN(0) on default.nation as n1
=> target list: default.n1.n_nationkey (INT4), default.n1.n_name (TEXT)
=> out schema: {(2) default.n1.n_nationkey (INT4), default.n1.n_name (TEXT)}
=> in schema: {(4) default.n1.n_nationkey (INT4), default.n1.n_name (TEXT), default.n1.n_regionkey (INT4), default.n1.n_comment (TEXT)}
{code}
It would be more nice if we can do SELECTION(3) just after JOIN(9).
I think, After TAJO-1310 is resolved, we can further enhance filter push down with new join filter feature.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1350: Refactor FilterPushDownRule::visitJo...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/384#discussion_r26644864
--- Diff: tajo-plan/src/main/java/org/apache/tajo/plan/rewrite/rules/FilterPushDownRule.java ---
@@ -355,20 +230,144 @@ public LogicalNode visitJoin(FilterPushDownContext context, LogicalPlan plan, Lo
if (joinNode.getJoinType() == JoinType.CROSS) {
joinNode.setJoinType(JoinType.INNER);
}
- context.pushingDownFilters.removeAll(matched);
}
- context.pushingDownFilters.addAll(outerJoinFilterEvalsExcludePredication);
- context.pushingDownFilters.addAll(thetaJoinFilter);
+ context.pushingDownFilters.removeAll(matched);
return joinNode;
}
+ private static Set<EvalNode> extractNonPushableJoinQuals(final LogicalPlan plan,
+ final LogicalPlan.QueryBlock block,
+ final JoinNode joinNode,
+ final Set<EvalNode> onPredicates,
+ final Set<EvalNode> wherePredicates)
+ throws PlanningException {
+ Set<EvalNode> nonPushableQuals = TUtil.newHashSet();
+ // TODO: non-equi theta join quals must not be pushed until TAJO-742 is resolved.
+ nonPushableQuals.addAll(extractNonEquiThetaJoinQuals(wherePredicates, block, joinNode));
+
+ // for outer joins
+ if (PlannerUtil.isOuterJoin(joinNode.getJoinType())) {
+ nonPushableQuals.addAll(extractNonPushableOuterJoinQuals(plan, onPredicates, wherePredicates, joinNode));
+ }
+ return nonPushableQuals;
+ }
+
+ /**
+ * For outer joins, pushable predicates can be decided based on their locations in the SQL and types of referencing relations.
+ *
+ * <h3>Table types</h3>
+ * <ul>
+ * <li>Preserved Row table : The table in an Outer Join that must return all rows.
+ * For left outer joins this is the Left table, for right outer joins it is the Right table.
+ * For full outer joins both tables are Preserved Row tables.</li>
+ * <li>Null Supplying table : This is the table that has nulls filled in for its columns in unmatched rows.
+ * In the non-full outer join case, this is the other table in the Join.
--- End diff --
Could you check this comment?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1350: Refactor FilterPushDownRule::visitJo...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/tajo/pull/384
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1350: Refactor FilterPushDownRule::visitJo...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/384#discussion_r26645305
--- Diff: tajo-plan/src/main/java/org/apache/tajo/plan/rewrite/rules/FilterPushDownRule.java ---
@@ -355,20 +230,144 @@ public LogicalNode visitJoin(FilterPushDownContext context, LogicalPlan plan, Lo
if (joinNode.getJoinType() == JoinType.CROSS) {
joinNode.setJoinType(JoinType.INNER);
}
- context.pushingDownFilters.removeAll(matched);
}
- context.pushingDownFilters.addAll(outerJoinFilterEvalsExcludePredication);
- context.pushingDownFilters.addAll(thetaJoinFilter);
+ context.pushingDownFilters.removeAll(matched);
return joinNode;
}
+ private static Set<EvalNode> extractNonPushableJoinQuals(final LogicalPlan plan,
+ final LogicalPlan.QueryBlock block,
+ final JoinNode joinNode,
+ final Set<EvalNode> onPredicates,
+ final Set<EvalNode> wherePredicates)
+ throws PlanningException {
+ Set<EvalNode> nonPushableQuals = TUtil.newHashSet();
+ // TODO: non-equi theta join quals must not be pushed until TAJO-742 is resolved.
+ nonPushableQuals.addAll(extractNonEquiThetaJoinQuals(wherePredicates, block, joinNode));
+
+ // for outer joins
+ if (PlannerUtil.isOuterJoin(joinNode.getJoinType())) {
+ nonPushableQuals.addAll(extractNonPushableOuterJoinQuals(plan, onPredicates, wherePredicates, joinNode));
+ }
+ return nonPushableQuals;
+ }
+
+ /**
+ * For outer joins, pushable predicates can be decided based on their locations in the SQL and types of referencing relations.
+ *
+ * <h3>Table types</h3>
+ * <ul>
+ * <li>Preserved Row table : The table in an Outer Join that must return all rows.
+ * For left outer joins this is the Left table, for right outer joins it is the Right table.
+ * For full outer joins both tables are Preserved Row tables.</li>
+ * <li>Null Supplying table : This is the table that has nulls filled in for its columns in unmatched rows.
+ * In the non-full outer join case, this is the other table in the Join.
+ * For full outer joins both tables are also Null Supplying tables.</li>
+ * </ul>
+ *
+ * <h3>Predicate types</h3>
+ * <ul>
+ * <li>During Join predicate : A predicate that is in the JOIN ON clause.</li>
+ * <li>After Join predicate : A predicate that is in the WHERE clause.</li>
+ * </ul>
+ *
+ * <h3>Predicate Pushdown Rules</h3>
+ * <ol>
+ * <li>During Join predicates cannot be pushed past Preserved Row tables.</li>
+ * <li>After Join predicates cannot be pushed past Null Supplying tables.</li>
+ * </ol>
+ */
+ private static Set<EvalNode> extractNonPushableOuterJoinQuals(final LogicalPlan plan,
+ final Set<EvalNode> onPredicates,
+ final Set<EvalNode> wherePredicates,
+ final JoinNode joinNode) throws PlanningException {
+ Set<String> nullSupplyingTableNameSet = TUtil.newHashSet();
--- End diff --
This method seems to be changed to be even more intuitive.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1350: Refactor FilterPushDownRule::visitJo...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on the pull request:
https://github.com/apache/tajo/pull/384#issuecomment-77494963
@hyunsik thanks for your comment.
That was my misunderstanding. I fixed it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1350: Refactor FilterPushDownRule::visitJo...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on the pull request:
https://github.com/apache/tajo/pull/384#issuecomment-82950907
Thanks. I improved java docs.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1350: Refactor FilterPushDownRule::visitJo...
Posted by blrunner <gi...@git.apache.org>.
Github user blrunner commented on the pull request:
https://github.com/apache/tajo/pull/384#issuecomment-86359111
@jihoonson
It seems that there was some problems on my laptop. I can't reproduce it again. :-)
And I'm still reviewing. I'll finish soon.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1350: Refactor FilterPushDownRule::visitJo...
Posted by blrunner <gi...@git.apache.org>.
Github user blrunner commented on the pull request:
https://github.com/apache/tajo/pull/384#issuecomment-86348625
@jihoonson
Thanks for your contribution.
But I failed to run an unit test for TestNetTypes as following:
```
-------------------------------------------------------------------------------
Test set: org.apache.tajo.engine.query.TestNetTypes
-------------------------------------------------------------------------------
Tests run: 6, Failures: 6, Errors: 0, Skipped: 0, Time elapsed: 0.012 sec <<< FAILURE! - in org.apache.tajo.engine.query.TestNetTypes
testSort2(org.apache.tajo.engine.query.TestNetTypes) Time elapsed: 0 sec <<< FAILURE!
java.lang.AssertionError: file:/Users/blrunner/Source/tajo/tajo-core/target/test-classes/dataset/TestNetTypes/table1 existence check
at org.junit.Assert.fail(Assert.java:88)
at org.junit.Assert.assertTrue(Assert.java:41)
at org.apache.tajo.QueryTestCaseBase.getDataSetFile(QueryTestCaseBase.java:605)
at org.apache.tajo.QueryTestCaseBase.executeDDL(QueryTestCaseBase.java:656)
at org.apache.tajo.QueryTestCaseBase.executeDDL(QueryTestCaseBase.java:643)
at org.apache.tajo.engine.query.TestNetTypes.setUp(TestNetTypes.java:32)
```
The strange thing is that it only is reproduced on console. I found that it had run successfully with IDE.
Could you check this problem on your development environment?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---