[CALCITE-3968] Disable JoinPushThroughJoinRule.right by default#1958
Closed
hsyuan wants to merge 3 commits intoapache:masterfrom
hsyuan:assiociate
Closed
[CALCITE-3968] Disable JoinPushThroughJoinRule.right by default#1958hsyuan wants to merge 3 commits intoapache:masterfrom hsyuan:assiociate
hsyuan wants to merge 3 commits intoapache:masterfrom
hsyuan:assiociate
Conversation
hsyuan
changed the title
hsyuan
added
the
LGTM-will-merge-soon
JoinPushThroughJoinRule.right does (RS)T -> (RT)S JoinPushThroughJoinRule.left does (RS)T -> (TS)R If JoinCommuteRule is enabled, only enabling JoinPushThroughJoinRule.left can achieve the same alternative with JoinPushThroughJoinRule.right, vice versa (suppose they are connected). So there is no need to enable left and right instances at the same time, which will slow down the optimization dramatically, e.g TPCH q05, q07 in TpchTest.java never finish. There is also the same bug report from [1]. Meanwhile, JoinPushThroughJoinRule matches RelNode.class, which is unnecessary. It should be just a group, or RelSet / RelSubset, as a place holder, because we don't care about what exactly the top join's right child is. But since the rule is designed to work with both HepPlanner and VolcanoPlanner, so just bear with the slowness. [1] https://lists.apache.org/thread.html/r195c267ef15f50aa21bbcefd7bf523f8bf2495b2345fd163e91e3c36%40%3Cdev.calcite.apache.org%3E
hsyuan
removed
the
LGTM-will-merge-soon
Member
Author
|
Closing this PR, since this has been superseded by #1976. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
JIRA: CALCITE-3968
JoinPushThroughJoinRule.right does
(RS)T -> (RT)S
JoinPushThroughJoinRule.left does
(RS)T -> (TS)R
If JoinCommuteRule is enabled, only enabling JoinPushThroughJoinRule.right can
achieve the same alternative with JoinPushThroughJoinRule.left, vice versa
(suppose they are connected). So there is no need to enable left and right
instances at the same time, which will slow down the optimization dramatically,
e.g TPCH q05, q07 in TpchTest.java never finish. There is also the same bug
report from [1].
Meanwhile, JoinPushThroughJoinRule matches RelNode.class, which is unnecessary.
It should be just a group, or RelSet / RelSubset, as a place holder, because we
don't care about what exactly the top join's right child is. But since the rule
is designed to work with both HepPlanner and VolcanoPlanner, so just bear with
the slowness.
[1] https://lists.apache.org/thread.html/r195c267ef15f50aa21bbcefd7bf523f8bf2495b2345fd163e91e3c36%40%3Cdev.calcite.apache.org%3E