[CORE] Optimize JoinSelectionOverrides when disable ras#6048
[CORE] Optimize JoinSelectionOverrides when disable ras#6048zml1206 wants to merge 3 commits intoapache:mainfrom
Conversation
|
Thanks for opening a pull request! Could you open an issue for this pull request on Github Issues? https://github.com/apache/incubator-gluten/issues Then could you also rename commit message and pull request title in the following format? See also: |
|
Run Gluten Clickhouse CI |
[CORE] Optimize JoinSelectionOverrides [CORE] Optimize JoinSelectionOverrides
edbc020 to
1852c95
Compare
|
Run Gluten Clickhouse CI |
|
Run Gluten Clickhouse CI |
|
Run Gluten Clickhouse CI |
|
Thank you for continuing working on this. @zml1206 And would you like to summarize a simple note for the responsibility of each of the following two parts of rules, after this refactor:
? It will help one understand the code change. Thanks. |
|
IIUC, is the only thing |
|
What is |
No, the buildSide generated by ShuffledHashJoinExecTemp is final, and the column rules correction is the buildside of ShuffledHashJoinExec when ras is enabled. |
|
According to the comments and code, the reason |
Yes. |
|
@zml1206 why not pass prefer build side to ShuffledHashJoinExecTransformer directly ? BTW, if it will eventually be eliminated, why the golden files changed ? |
What I hope is that both creates "more vanilla" plan when the join operators are falling back and builds buildsideide with small tables. Prefer build side to ShuffledHashJoinExecTransformer directly is not always possible to select a smaller table. For example, before AQE is SortMergeJoin, the sizeByte on the left is small. During AQE after reOptimize, it is still SortMergeJoin and the sizeByte on the right is smaller. However, because the plan has not changed, the plan will not be replaced. In the end, the left table is selected. |
Initial Plan will contain |
it's not sure, AQE will use the new plan if currentPhysicalPlan != newPhysicalPlan. So I think ShuffledHashJoin should always choose the small table. SortMergeJoin is a special case that Spark always build right side for inner join, etc. If your goal is to optimize the vanilla Spark SortMergeJoin. I think it's better to push to Spark community first. For gluten, we can just optimize SortMergeJoin when do transform. |
First of all, it is primary to ensure that the plan after fallback is consistent with vanilla spark, so we should not force the generation of shuffledHashJoinExec. We should convert ShuffledHashJoinExec/SortMergeJoinExec into ShuffledHashJoinExecTransformer in offload. |
|
You mean ensure plan of Join fallback is consistent with vanilla spark, abandon some scenarios for use smaller table? @ulysses-you |
|
IIUC vanilla Spark shuffle hash join should choose small table since 3.0, can you point out to me which pr in Spark3.5 work on this ? thank you. |
|
|
It seems that pr only allows more build side for shuffled hash join ? Vanilla Spark would choose a small table without gluten, right ? If we want to fallback a smj to vanilla Spark, I think pass originalJoin to ShuffledHashJoinExecTransformer is enough. Add a tag ? To be clear, I was asking if ShuffledHashJoinExecTemp is required. |
ShuffledHashJoinExecTemp allows ShuffledHashJoinExecTransformer to use smaller table in all versions of spark, including the original plan is SortMergeJoin. Use smaller table for buildSide is more memory safe. |
|
Discussed with @ulysses-you that adding new physical operators is risky, use custom cost evaluator to force update the physical plan is a good way. close it, new PR #6093 cc @zhztheplayer |
What changes were proposed in this pull request?
How was this patch tested?