You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@tajo.apache.org by ykrips <gi...@git.apache.org> on 2015/03/09 08:50:21 UTC
[GitHub] tajo pull request: TAJO-1368: Exceptions during processing nested ...
GitHub user ykrips opened a pull request:
https://github.com/apache/tajo/pull/402
TAJO-1368: Exceptions during processing nested union queries
Please feel free to leave any comments on this patch.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ykrips/tajo TAJO-1368
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/tajo/pull/402.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 #402
----
commit dd4d1bbcb7048ad9c2b50daddcf01eeb6b4f647b
Author: Jihun Kang <ji...@apache.org>
Date: 2015-03-09T07:32:09Z
Initial work
commit 20a2e6b7a0bc97c9be3568aef82cc23aa6d3ecb1
Author: Jihun Kang <ji...@apache.org>
Date: 2015-03-09T07:34:19Z
Fixed unused import
commit 182983e2af9a2d1c1381193432c0b0e880f8eefd
Author: Jihun Kang <ji...@apache.org>
Date: 2015-03-09T07:47:12Z
Fixed invalid code
----
---
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-1368: Exceptions during processing nested ...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/402#discussion_r26217412
--- Diff: tajo-core/src/main/java/org/apache/tajo/engine/planner/global/GlobalPlanner.java ---
@@ -1371,32 +1373,57 @@ public LogicalNode visitUnion(GlobalPlanContext context, LogicalPlan plan, Logic
LogicalPlan.QueryBlock rightQueryBlock = plan.getBlock(node.getRightChild());
LogicalNode rightChild = visit(context, plan, rightQueryBlock, rightQueryBlock.getRoot(), stack);
stack.pop();
+
+ MasterPlan masterPlan = context.getPlan();
List<ExecutionBlock> unionBlocks = Lists.newArrayList();
List<ExecutionBlock> queryBlockBlocks = Lists.newArrayList();
ExecutionBlock leftBlock = context.execBlockMap.remove(leftChild.getPID());
ExecutionBlock rightBlock = context.execBlockMap.remove(rightChild.getPID());
+
+ boolean leftUnion = (leftChild.getType() == NodeType.TABLE_SUBQUERY) &&
+ (((TableSubQueryNode)leftChild).getSubQuery().getType() == NodeType.UNION);
+ boolean rightUnion = (rightChild.getType() == NodeType.TABLE_SUBQUERY) &&
+ (((TableSubQueryNode)rightChild).getSubQuery().getType() == NodeType.UNION);
if (leftChild.getType() == NodeType.UNION) {
unionBlocks.add(leftBlock);
} else {
- queryBlockBlocks.add(leftBlock);
+ if (leftUnion) {
+ node.setLeftChild(
+ createScanNodeWithTableSubQuery(masterPlan,
--- End diff --
Please check whether I understand your patch well.
When an union has a child of subquery that has another union child, the subquery and its child union should be removed. Let me suppose an example union ```u1```. It has a child subquery ```s1``` which has another union child, ```u2```. Finally, ```u2``` has a scan child, ```scan1```. In this case, ```u1``` should be directly connected with ```scan```.
However, according to your patch, the example query forms (```scan1``` - ```u2``` - ```s1```) - (```scan2``` - ```u1```). Parentheses mean query blocks, and ```scan2``` is created at this line. In this case, I think that it is difficult to avoid unnecessary materialization of intermediate data between query blocks.
If I misunderstand something, please tell me.
---
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-1368: Exceptions during processing nested ...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on the pull request:
https://github.com/apache/tajo/pull/402#issuecomment-77857664
I'll review tomorrow.
---
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-1368: Exceptions during processing nested ...
Posted by ykrips <gi...@git.apache.org>.
Github user ykrips commented on a diff in the pull request:
https://github.com/apache/tajo/pull/402#discussion_r26313458
--- Diff: tajo-core/src/main/java/org/apache/tajo/engine/planner/global/GlobalPlanner.java ---
@@ -1371,32 +1373,57 @@ public LogicalNode visitUnion(GlobalPlanContext context, LogicalPlan plan, Logic
LogicalPlan.QueryBlock rightQueryBlock = plan.getBlock(node.getRightChild());
LogicalNode rightChild = visit(context, plan, rightQueryBlock, rightQueryBlock.getRoot(), stack);
stack.pop();
+
+ MasterPlan masterPlan = context.getPlan();
List<ExecutionBlock> unionBlocks = Lists.newArrayList();
List<ExecutionBlock> queryBlockBlocks = Lists.newArrayList();
ExecutionBlock leftBlock = context.execBlockMap.remove(leftChild.getPID());
ExecutionBlock rightBlock = context.execBlockMap.remove(rightChild.getPID());
+
+ boolean leftUnion = (leftChild.getType() == NodeType.TABLE_SUBQUERY) &&
+ (((TableSubQueryNode)leftChild).getSubQuery().getType() == NodeType.UNION);
+ boolean rightUnion = (rightChild.getType() == NodeType.TABLE_SUBQUERY) &&
+ (((TableSubQueryNode)rightChild).getSubQuery().getType() == NodeType.UNION);
if (leftChild.getType() == NodeType.UNION) {
unionBlocks.add(leftBlock);
} else {
- queryBlockBlocks.add(leftBlock);
+ if (leftUnion) {
+ node.setLeftChild(
+ createScanNodeWithTableSubQuery(masterPlan,
+ (TableSubQueryNode)leftChild, leftBlock, leftBlock,
+ rightUnion? null: rightBlock, true));
+ } else {
+ queryBlockBlocks.add(leftBlock);
+ }
}
if (rightChild.getType() == NodeType.UNION) {
unionBlocks.add(rightBlock);
} else {
- queryBlockBlocks.add(rightBlock);
+ if (rightUnion) {
+ node.setRightChild(
+ createScanNodeWithTableSubQuery(masterPlan,
+ (TableSubQueryNode)rightChild, rightBlock, leftUnion? leftBlock: rightBlock,
+ leftUnion? null: leftBlock, false));
+ } else {
+ queryBlockBlocks.add(rightBlock);
+ }
}
ExecutionBlock execBlock;
if (unionBlocks.size() == 0) {
- execBlock = context.plan.newExecutionBlock();
+ if (leftUnion || rightUnion) {
+ execBlock = (!leftUnion && rightUnion)? rightBlock: leftBlock;
--- End diff --
This line is no more necessary. I will upload changes for this 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-1368: Exceptions during processing nested ...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on the pull request:
https://github.com/apache/tajo/pull/402#issuecomment-81316946
Sorry for delayed review.
I'll review tonight.
---
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-1368: Exceptions during processing nested ...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on the pull request:
https://github.com/apache/tajo/pull/402#issuecomment-78277194
@ykrips thanks for your patch. I left a comment.
In addition, it is difficult to understand details of some changes.
Would you mind telling me a brief summarization of the changes?
Also, it would be great if you add some comments.
---
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-1368: Exceptions during processing nested ...
Posted by ykrips <gi...@git.apache.org>.
Github user ykrips commented on the pull request:
https://github.com/apache/tajo/pull/402#issuecomment-82218213
Good finding!
I also figure out this error.
---
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-1368: Exceptions during processing nested ...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on the pull request:
https://github.com/apache/tajo/pull/402#issuecomment-82199826
Hi @ykrips thanks for your nice patch.
It looks good, but I have another suggestion even though it is not directly related with this issue.
I think that ```union``` queries are popularly used for ETLs. So, I tested the following query. (It is a CTAS statement of the case 1 of TAJO-1368.)
```
create table test using parquet as
select * from
(select c_custkey, c_nationkey from customer where c_nationkey < 0
union all
select c_custkey, c_nationkey from customer where c_nationkey > 0
) a
union all
select * from
(select c_custkey, c_nationkey from customer where c_nationkey < 0
union all
select c_custkey, c_nationkey from customer where c_nationkey > 0
) b
```
And, I found the following error.
```
2015-03-17 17:29:42,257 ERROR: org.apache.tajo.querymaster.QueryMasterTask (startQuery(368)) -
java.lang.NullPointerException
at org.apache.tajo.engine.planner.global.GlobalPlanner.buildNoPartitionedStorePlan(GlobalPlanner.java:1113)
at org.apache.tajo.engine.planner.global.GlobalPlanner.buildStorePlan(GlobalPlanner.java:1037)
at org.apache.tajo.engine.planner.global.GlobalPlanner.access$600(GlobalPlanner.java:68)
at org.apache.tajo.engine.planner.global.GlobalPlanner$DistributedPlannerVisitor.visitCreateTable(GlobalPlanner.java:1570)
at org.apache.tajo.engine.planner.global.GlobalPlanner$DistributedPlannerVisitor.visitCreateTable(GlobalPlanner.java:1141)
at org.apache.tajo.plan.visitor.BasicLogicalPlanVisitor.visit(BasicLogicalPlanVisitor.java:126)
at org.apache.tajo.plan.visitor.BasicLogicalPlanVisitor.visitRoot(BasicLogicalPlanVisitor.java:151)
at org.apache.tajo.engine.planner.global.GlobalPlanner$DistributedPlannerVisitor.visitRoot(GlobalPlanner.java:1146)
at org.apache.tajo.engine.planner.global.GlobalPlanner$DistributedPlannerVisitor.visitRoot(GlobalPlanner.java:1141)
at org.apache.tajo.plan.visitor.BasicLogicalPlanVisitor.visit(BasicLogicalPlanVisitor.java:60)
at org.apache.tajo.engine.planner.global.GlobalPlanner.build(GlobalPlanner.java:147)
at org.apache.tajo.querymaster.QueryMasterTask.startQuery(QueryMasterTask.java:360)
at org.apache.tajo.querymaster.QueryMasterTask.start(QueryMasterTask.java:175)
at org.apache.tajo.querymaster.QueryMaster$QueryStartEventHandler.handle(QueryMaster.java:416)
at org.apache.tajo.querymaster.QueryMaster$QueryStartEventHandler.handle(QueryMaster.java:403)
at org.apache.hadoop.yarn.event.AsyncDispatcher.dispatch(AsyncDispatcher.java:173)
at org.apache.hadoop.yarn.event.AsyncDispatcher$1.run(AsyncDispatcher.java:106)
at java.lang.Thread.run(Thread.java:745)
```
This error seems to be caused by misfunctioning of ```GlobalPlanner.hasUnionChild()```.
I think it would be great if you fix this bug, too.
---
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-1368: Exceptions during processing nested ...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/402#discussion_r26296284
--- Diff: tajo-core/src/main/java/org/apache/tajo/engine/planner/global/GlobalPlanner.java ---
@@ -1371,32 +1373,57 @@ public LogicalNode visitUnion(GlobalPlanContext context, LogicalPlan plan, Logic
LogicalPlan.QueryBlock rightQueryBlock = plan.getBlock(node.getRightChild());
LogicalNode rightChild = visit(context, plan, rightQueryBlock, rightQueryBlock.getRoot(), stack);
stack.pop();
+
+ MasterPlan masterPlan = context.getPlan();
List<ExecutionBlock> unionBlocks = Lists.newArrayList();
List<ExecutionBlock> queryBlockBlocks = Lists.newArrayList();
ExecutionBlock leftBlock = context.execBlockMap.remove(leftChild.getPID());
ExecutionBlock rightBlock = context.execBlockMap.remove(rightChild.getPID());
+
+ boolean leftUnion = (leftChild.getType() == NodeType.TABLE_SUBQUERY) &&
+ (((TableSubQueryNode)leftChild).getSubQuery().getType() == NodeType.UNION);
+ boolean rightUnion = (rightChild.getType() == NodeType.TABLE_SUBQUERY) &&
+ (((TableSubQueryNode)rightChild).getSubQuery().getType() == NodeType.UNION);
if (leftChild.getType() == NodeType.UNION) {
unionBlocks.add(leftBlock);
} else {
- queryBlockBlocks.add(leftBlock);
+ if (leftUnion) {
+ node.setLeftChild(
+ createScanNodeWithTableSubQuery(masterPlan,
+ (TableSubQueryNode)leftChild, leftBlock, leftBlock,
+ rightUnion? null: rightBlock, true));
+ } else {
+ queryBlockBlocks.add(leftBlock);
+ }
}
if (rightChild.getType() == NodeType.UNION) {
unionBlocks.add(rightBlock);
} else {
- queryBlockBlocks.add(rightBlock);
+ if (rightUnion) {
+ node.setRightChild(
+ createScanNodeWithTableSubQuery(masterPlan,
+ (TableSubQueryNode)rightChild, rightBlock, leftUnion? leftBlock: rightBlock,
+ leftUnion? null: leftBlock, false));
+ } else {
+ queryBlockBlocks.add(rightBlock);
+ }
}
ExecutionBlock execBlock;
if (unionBlocks.size() == 0) {
- execBlock = context.plan.newExecutionBlock();
+ if (leftUnion || rightUnion) {
+ execBlock = (!leftUnion && rightUnion)? rightBlock: leftBlock;
--- End diff --
Would you add some descriptions for this line?
---
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-1368: Exceptions during processing nested ...
Posted by ykrips <gi...@git.apache.org>.
Github user ykrips commented on the pull request:
https://github.com/apache/tajo/pull/402#issuecomment-78410182
@jihoonson , Thank you for review on this patch. I will add some comments on this patch, and will upload patches in a short time.
---
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-1368: Exceptions during processing nested ...
Posted by ykrips <gi...@git.apache.org>.
Github user ykrips commented on a diff in the pull request:
https://github.com/apache/tajo/pull/402#discussion_r26272910
--- Diff: tajo-core/src/main/java/org/apache/tajo/engine/planner/global/GlobalPlanner.java ---
@@ -1371,32 +1373,57 @@ public LogicalNode visitUnion(GlobalPlanContext context, LogicalPlan plan, Logic
LogicalPlan.QueryBlock rightQueryBlock = plan.getBlock(node.getRightChild());
LogicalNode rightChild = visit(context, plan, rightQueryBlock, rightQueryBlock.getRoot(), stack);
stack.pop();
+
+ MasterPlan masterPlan = context.getPlan();
List<ExecutionBlock> unionBlocks = Lists.newArrayList();
List<ExecutionBlock> queryBlockBlocks = Lists.newArrayList();
ExecutionBlock leftBlock = context.execBlockMap.remove(leftChild.getPID());
ExecutionBlock rightBlock = context.execBlockMap.remove(rightChild.getPID());
+
+ boolean leftUnion = (leftChild.getType() == NodeType.TABLE_SUBQUERY) &&
+ (((TableSubQueryNode)leftChild).getSubQuery().getType() == NodeType.UNION);
+ boolean rightUnion = (rightChild.getType() == NodeType.TABLE_SUBQUERY) &&
+ (((TableSubQueryNode)rightChild).getSubQuery().getType() == NodeType.UNION);
if (leftChild.getType() == NodeType.UNION) {
unionBlocks.add(leftBlock);
} else {
- queryBlockBlocks.add(leftBlock);
+ if (leftUnion) {
+ node.setLeftChild(
+ createScanNodeWithTableSubQuery(masterPlan,
--- End diff --
You're right. First of all, I eliminated these child subqueries, but plan threw exceptions that plan does not have sufficient data channels. I feel that it would be better to change the implementation. therefore, I have created these even unnecessary data operations exist.
I agree with your thought, and I will try again for this optimization work.
---
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-1368: Exceptions during processing nested ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/tajo/pull/402
---
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-1368: Exceptions during processing nested ...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on the pull request:
https://github.com/apache/tajo/pull/402#issuecomment-82678777
Thanks a lot!
The patch looks great.
I'll give +1 after CI test is finished.
It will be good to put your latest patch on Jira because Travis is too slow.
---
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.
---