-
Notifications
You must be signed in to change notification settings - Fork 28.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SPARK-20231] [SQL] Refactor star schema code for the subsequent star join detection in CBO #17544
Conversation
@gatorsmile I did a small refactoring for star schema. Would you please review. Thank you. |
add to whitelist |
ok to test |
*/ | ||
def findStarJoins( | ||
input: Seq[LogicalPlan], | ||
conditions: Seq[Expression]): Seq[LogicalPlan] = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To the other reviewers, this is to address the comment by @cloud-fan in the previous PR.
#15363 (comment)
// dimension table is a FK-PK join. Heuristically, a selective dimension may reduce | ||
// the result of a join. | ||
// Also, conservatively assume that a fact table is joined with more than one dimension. | ||
if (dimTables.size >= 2 && isSelectiveStarJoin(dimTables, conditions)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dimTables.size >= 2
is removed in the new code? Could you please explain it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gatorsmile I moved the condition from reorderStarJoin
to findStarJoin
to be used by both ReorderJoin and CostBasedReorder.
Test build #75557 has finished for PR 17544 at commit
|
} | ||
} | ||
|
||
if (eligibleDimPlans.isEmpty || eligibleDimPlans.size < 2) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uh, you move it to here.
LGTM. Look forward to your next PR for merging start join detection with CBO. |
Thanks! Merging to master. |
@gatorsmile Thank you! |
What changes were proposed in this pull request?
This commit moves star schema code from
join.scala
toStarSchemaDetection.scala
. It also applies some minor fixes inStarJoinReorderSuite.scala
.How was this patch tested?
Run existing
StarJoinReorderSuite.scala
.