You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@tajo.apache.org by Hyunsik Choi <hy...@apache.org> on 2014/02/20 09:36:52 UTC
Review Request 18305: TAJO-614: Explaning a logical node should use
ExplainLogicalPlanVisitor.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18305/
-----------------------------------------------------------
Review request for Tajo.
Bugs: TAJO-614
https://issues.apache.org/jira/browse/TAJO-614
Repository: tajo
Description
-------
Currently, many parts use LogicalNode::toString() for explaning plans. But, we already have ExplainLogicalPlanVisitor class to generate pretty print strings.
This patch improves logical planning related parts to use ExplainLogicalPlanVisitor instead of toString().
For this, I added PlannerUtil::buildExplainString for generating pretty print explain strings and simplified obsolete toString() methods of all LogicalNodes.
It much improves the readability of explain string. I expect that it would be helpful for debugging and users' understanding of query plans.
Diffs
-----
Diff: https://reviews.apache.org/r/18305/diff/
Testing
-------
mvn clean install -Phcatalog-0.12
Thanks,
Hyunsik Choi
Re: Review Request 18305: TAJO-614: Explaning a logical node should use
ExplainLogicalPlanVisitor.
Posted by Hyunsik Choi <hy...@apache.org>.
> On Feb. 21, 2014, 2:52 p.m., Jung JaeHwa wrote:
> > +1.
> > It looks nicely, push it. :)
Thank you for quick review. I'll commit it.
- Hyunsik
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18305/#review35133
-----------------------------------------------------------
On Feb. 20, 2014, 9:16 p.m., Hyunsik Choi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18305/
> -----------------------------------------------------------
>
> (Updated Feb. 20, 2014, 9:16 p.m.)
>
>
> Review request for Tajo.
>
>
> Bugs: TAJO-614
> https://issues.apache.org/jira/browse/TAJO-614
>
>
> Repository: tajo
>
>
> Description
> -------
>
> Currently, many parts use LogicalNode::toString() for explaning plans. But, we already have ExplainLogicalPlanVisitor class to generate pretty print strings. This patch improves logical planning related parts to use ExplainLogicalPlanVisitor instead of toString(). For this, I added PlannerUtil::buildExplainString for generating pretty print explain strings and simplified obsolete toString() methods of all LogicalNodes. It much improves the readability of explain string. I expect that it would be helpful for debugging and users' understanding of query plans.
>
>
> Diffs
> -----
>
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/ExplainLogicalPlanVisitor.java 4cd40f7
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/ExprsVerifier.java 87ced21
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/InsertNode.java bdc7837
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/LogicalPlan.java 5491fef
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/LogicalPlanPreprocessor.java 818e01c
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/LogicalPlanner.java 2a4e151
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/PhysicalPlannerImpl.java 7ba2889
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/PlannerUtil.java 4f1421b
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/global/GlobalPlanner.java 258a2e7
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/global/MasterPlan.java e72ba05
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/CreateTableNode.java 6dcca2d
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/DropTableNode.java 06d8185
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/EvalExprNode.java ed63bbc
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/ExceptNode.java 0ed39df
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/GroupbyNode.java 38ca29b
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/HavingNode.java 816c34e
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/IndexScanNode.java e82cc3e
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/InsertNode.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/IntersectNode.java 74aa9eb
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/JoinNode.java 8902932
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/LimitNode.java a3d1f4b
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/LogicalNode.java 4e9721f
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/NodeType.java 32b9bc6
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/PartitionedTableScanNode.java 868c493
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/PersistentStoreNode.java 65fd4f5
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/ProjectionNode.java acfaeb6
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/RelationNode.java 8ed247e
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/ScanNode.java 0ec8110
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/SelectionNode.java 7a03d12
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/ShuffleFileWriteNode.java e4ef49b
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/StoreIndexNode.java fb8ae3c
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/StoreTableNode.java ff08e36
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/TableSubQueryNode.java 3f251f8
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/UnionNode.java efba1ef
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/join/GreedyHeuristicJoinOrderAlgorithm.java 428152b
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/physical/ColPartitionStoreExec.java 2ec0315
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/physical/StoreTableExec.java 786148b
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/rewrite/PartitionedTableRewriter.java d701935
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/Query.java 6a4eb4b
> tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/planner/TestLogicalNode.java b540868
> tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/planner/physical/TestPhysicalPlanner.java 7e3bd2b
>
> Diff: https://reviews.apache.org/r/18305/diff/
>
>
> Testing
> -------
>
> mvn clean install -Phcatalog-0.12.0
>
>
> Thanks,
>
> Hyunsik Choi
>
>
Re: Review Request 18305: TAJO-614: Explaning a logical node should use
ExplainLogicalPlanVisitor.
Posted by Jung JaeHwa <jh...@gruter.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18305/#review35133
-----------------------------------------------------------
Ship it!
+1.
It looks nicely, push it. :)
- Jung JaeHwa
On Feb. 20, 2014, 12:16 p.m., Hyunsik Choi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18305/
> -----------------------------------------------------------
>
> (Updated Feb. 20, 2014, 12:16 p.m.)
>
>
> Review request for Tajo.
>
>
> Bugs: TAJO-614
> https://issues.apache.org/jira/browse/TAJO-614
>
>
> Repository: tajo
>
>
> Description
> -------
>
> Currently, many parts use LogicalNode::toString() for explaning plans. But, we already have ExplainLogicalPlanVisitor class to generate pretty print strings. This patch improves logical planning related parts to use ExplainLogicalPlanVisitor instead of toString(). For this, I added PlannerUtil::buildExplainString for generating pretty print explain strings and simplified obsolete toString() methods of all LogicalNodes. It much improves the readability of explain string. I expect that it would be helpful for debugging and users' understanding of query plans.
>
>
> Diffs
> -----
>
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/ExplainLogicalPlanVisitor.java 4cd40f7
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/ExprsVerifier.java 87ced21
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/InsertNode.java bdc7837
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/LogicalPlan.java 5491fef
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/LogicalPlanPreprocessor.java 818e01c
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/LogicalPlanner.java 2a4e151
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/PhysicalPlannerImpl.java 7ba2889
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/PlannerUtil.java 4f1421b
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/global/GlobalPlanner.java 258a2e7
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/global/MasterPlan.java e72ba05
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/CreateTableNode.java 6dcca2d
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/DropTableNode.java 06d8185
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/EvalExprNode.java ed63bbc
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/ExceptNode.java 0ed39df
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/GroupbyNode.java 38ca29b
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/HavingNode.java 816c34e
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/IndexScanNode.java e82cc3e
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/InsertNode.java PRE-CREATION
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/IntersectNode.java 74aa9eb
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/JoinNode.java 8902932
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/LimitNode.java a3d1f4b
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/LogicalNode.java 4e9721f
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/NodeType.java 32b9bc6
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/PartitionedTableScanNode.java 868c493
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/PersistentStoreNode.java 65fd4f5
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/ProjectionNode.java acfaeb6
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/RelationNode.java 8ed247e
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/ScanNode.java 0ec8110
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/SelectionNode.java 7a03d12
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/ShuffleFileWriteNode.java e4ef49b
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/StoreIndexNode.java fb8ae3c
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/StoreTableNode.java ff08e36
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/TableSubQueryNode.java 3f251f8
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/UnionNode.java efba1ef
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/join/GreedyHeuristicJoinOrderAlgorithm.java 428152b
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/physical/ColPartitionStoreExec.java 2ec0315
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/physical/StoreTableExec.java 786148b
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/rewrite/PartitionedTableRewriter.java d701935
> tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/Query.java 6a4eb4b
> tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/planner/TestLogicalNode.java b540868
> tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/planner/physical/TestPhysicalPlanner.java 7e3bd2b
>
> Diff: https://reviews.apache.org/r/18305/diff/
>
>
> Testing
> -------
>
> mvn clean install -Phcatalog-0.12.0
>
>
> Thanks,
>
> Hyunsik Choi
>
>
Re: Review Request 18305: TAJO-614: Explaning a logical node should use
ExplainLogicalPlanVisitor.
Posted by Hyunsik Choi <hy...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18305/
-----------------------------------------------------------
(Updated Feb. 20, 2014, 9:16 p.m.)
Review request for Tajo.
Changes
-------
Rebased the patch against the latest patch.
Bugs: TAJO-614
https://issues.apache.org/jira/browse/TAJO-614
Repository: tajo
Description
-------
Currently, many parts use LogicalNode::toString() for explaning plans. But, we already have ExplainLogicalPlanVisitor class to generate pretty print strings. This patch improves logical planning related parts to use ExplainLogicalPlanVisitor instead of toString(). For this, I added PlannerUtil::buildExplainString for generating pretty print explain strings and simplified obsolete toString() methods of all LogicalNodes. It much improves the readability of explain string. I expect that it would be helpful for debugging and users' understanding of query plans.
Diffs (updated)
-----
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/ExplainLogicalPlanVisitor.java 4cd40f7
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/ExprsVerifier.java 87ced21
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/InsertNode.java bdc7837
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/LogicalPlan.java 5491fef
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/LogicalPlanPreprocessor.java 818e01c
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/LogicalPlanner.java 2a4e151
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/PhysicalPlannerImpl.java 7ba2889
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/PlannerUtil.java 4f1421b
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/global/GlobalPlanner.java 258a2e7
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/global/MasterPlan.java e72ba05
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/CreateTableNode.java 6dcca2d
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/DropTableNode.java 06d8185
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/EvalExprNode.java ed63bbc
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/ExceptNode.java 0ed39df
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/GroupbyNode.java 38ca29b
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/HavingNode.java 816c34e
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/IndexScanNode.java e82cc3e
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/InsertNode.java PRE-CREATION
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/IntersectNode.java 74aa9eb
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/JoinNode.java 8902932
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/LimitNode.java a3d1f4b
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/LogicalNode.java 4e9721f
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/NodeType.java 32b9bc6
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/PartitionedTableScanNode.java 868c493
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/PersistentStoreNode.java 65fd4f5
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/ProjectionNode.java acfaeb6
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/RelationNode.java 8ed247e
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/ScanNode.java 0ec8110
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/SelectionNode.java 7a03d12
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/ShuffleFileWriteNode.java e4ef49b
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/StoreIndexNode.java fb8ae3c
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/StoreTableNode.java ff08e36
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/TableSubQueryNode.java 3f251f8
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/UnionNode.java efba1ef
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/logical/join/GreedyHeuristicJoinOrderAlgorithm.java 428152b
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/physical/ColPartitionStoreExec.java 2ec0315
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/physical/StoreTableExec.java 786148b
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/rewrite/PartitionedTableRewriter.java d701935
tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/Query.java 6a4eb4b
tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/planner/TestLogicalNode.java b540868
tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/planner/physical/TestPhysicalPlanner.java 7e3bd2b
Diff: https://reviews.apache.org/r/18305/diff/
Testing
-------
mvn clean install -Phcatalog-0.12.0
Thanks,
Hyunsik Choi
Re: Review Request 18305: TAJO-614: Explaning a logical node should use
ExplainLogicalPlanVisitor.
Posted by Hyunsik Choi <hy...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18305/
-----------------------------------------------------------
(Updated Feb. 20, 2014, 6:45 p.m.)
Review request for Tajo.
Bugs: TAJO-614
https://issues.apache.org/jira/browse/TAJO-614
Repository: tajo
Description (updated)
-------
Currently, many parts use LogicalNode::toString() for explaning plans. But, we already have ExplainLogicalPlanVisitor class to generate pretty print strings. This patch improves logical planning related parts to use ExplainLogicalPlanVisitor instead of toString(). For this, I added PlannerUtil::buildExplainString for generating pretty print explain strings and simplified obsolete toString() methods of all LogicalNodes. It much improves the readability of explain string. I expect that it would be helpful for debugging and users' understanding of query plans.
Diffs
-----
Diff: https://reviews.apache.org/r/18305/diff/
Testing
-------
mvn clean install -Phcatalog-0.12.0
Thanks,
Hyunsik Choi
Re: Review Request 18305: TAJO-614: Explaning a logical node should use
ExplainLogicalPlanVisitor.
Posted by Hyunsik Choi <hy...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18305/
-----------------------------------------------------------
(Updated Feb. 20, 2014, 5:37 p.m.)
Review request for Tajo.
Bugs: TAJO-614
https://issues.apache.org/jira/browse/TAJO-614
Repository: tajo
Description
-------
Currently, many parts use LogicalNode::toString() for explaning plans. But, we already have ExplainLogicalPlanVisitor class to generate pretty print strings.
This patch improves logical planning related parts to use ExplainLogicalPlanVisitor instead of toString().
For this, I added PlannerUtil::buildExplainString for generating pretty print explain strings and simplified obsolete toString() methods of all LogicalNodes.
It much improves the readability of explain string. I expect that it would be helpful for debugging and users' understanding of query plans.
Diffs
-----
Diff: https://reviews.apache.org/r/18305/diff/
Testing (updated)
-------
mvn clean install -Phcatalog-0.12.0
Thanks,
Hyunsik Choi