apache
[GLUTEN-10559] Simplify StrictRule and remove unnecessary DummyLeafExec
What changes are proposed in this pull request?
This PR proposes to simplify StrictRule and remove unnecessary DummyLeafExec.
Fixes: #10559
This improvement can advance the perf of Spark driver. I paste the perf below.
Before this improvement, the benchmark of TPCDS SF1000 show below.
| First | Second | Third. | Avg | Median |
|---|---|---|---|---|
| 3091.7 s | 3012.2 s | 3100.7 s | 3068.2 s | 3034.5 s |
Before this improvement, the benchmark of TPCDS SF1000 show below.
| First | Second | Third. | Avg | Median |
|---|---|---|---|---|
| 2996.5 s | 2938.4 s | 2945.9 s | 2960.267 s | 2948.6 s |
The average perf advances 3.52% and the median perf advances 2.83%.
There is an example. The original plan:
Project [3 AS e#8L, 1 AS c#10, 2 AS d#9L]
+- Scan OneRowRelation[]
Before this PR, the transform process is as follows:
Because the mechanism of transfomUp, treat Scan OneRowRelation[] first. Scan OneRowRelation[] do not have any children, so nothing could be hidden.
Scan OneRowRelation[] cannot be offloaded, so the plan will not change. The current plan show below.
Project [3 AS e#8L, 1 AS c#10, 2 AS d#9L]
+- Scan OneRowRelation[]
Because Scan OneRowRelation[] do not have any children, so nothing need restore from hidden.
So the plan will not change. The current plan show below.
Project [3 AS e#8L, 1 AS c#10, 2 AS d#9L]
+- Scan OneRowRelation[]
Because the mechanism of transfomUp, treat Project [3 AS e#8L, 1 AS c#10, 2 AS d#9L] now. The child Scan OneRowRelation[] would be hide.
So the plan changed. The current plan show below.
Project [3 AS e#8L, 1 AS c#10, 2 AS d#9L]
+- DummyLeaf Scan OneRowRelation[]
+- Scan OneRowRelation[]
After offload Project [3 AS e#8L, 1 AS c#10, 2 AS d#9L], the plan would be ...
ProjectExecTransformer [3 AS e#8L, 1 AS c#10, 2 AS d#9L]
+- DummyLeaf Scan OneRowRelation[]
+- Scan OneRowRelation[]
Now restore the child from hidden. The plan would be ...
ProjectExecTransformer [3 AS e#8L, 1 AS c#10, 2 AS d#9L]
+- Scan OneRowRelation[]
After this PR, the transform process is as follows:
Because the mechanism of transfomUp, treat Scan OneRowRelation[] first.
Scan OneRowRelation[] cannot be offloaded, so the plan will not change. The current plan show below.
Project [3 AS e#8L, 1 AS c#10, 2 AS d#9L]
+- Scan OneRowRelation[]
Because the mechanism of transfomUp, treat Project [3 AS e#8L, 1 AS c#10, 2 AS d#9L] now.
After offload Project [3 AS e#8L, 1 AS c#10, 2 AS d#9L], the plan would be ...
ProjectExecTransformer [3 AS e#8L, 1 AS c#10, 2 AS d#9L]
+- Scan OneRowRelation[]
How was this patch tested?
GA tests.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
May I know what led you to this attempt?
Thank you! I know that. In fact, DummyLeafExec does not work well due to the transform up.
Please see LegacyOffload.
DummyLeafExec does not work well due to the transform up.
I may lose the contexts... Would you elaborate on this? Thanks.
I may lose the contexts... Would you elaborate on this? Thanks.
Take an example, a ProjectExec has a child FileSourceScanExec. The StrictRule will transform FileSourceScanExec to FileSourceScanExecTransformer and then transform ProjectExec to ProjectExecTransformer due to transformUp.
When hiding the children of ProjectExec, the child of ProjectExec already be the ProjectExecTransformer, so StrictRule can't hide anything.
When hiding the children of ProjectExec, the child of ProjectExec already be the ProjectExecTransformer
I still don't understand the example. Are you able to share the plans before and after the strict rule?
I still don't understand the example. Are you able to share the plans before and after the strict rule?
I added an example into PR's description.
OffloadOthers(), OffloadExchange(), and OffloadJoin() do not traverse the plan internally, so I also think DummyLeafExec is not necessary.
OffloadOthers(), OffloadExchange(), and OffloadJoin() do not traverse the plan internally
Can we keep the hiding children logic in testing only? We should make sure the single node rules doesn't touch the children nodes to keep the logic atomic. If a rule matches and offloads a sub-tree, it should be placed in post-transform instead.
Can we keep the hiding children logic in testing only? We should make sure the single node rules doesn't touch the children nodes to keep the logic atomic. If a rule matches and offloads a sub-tree, it should be placed in post-transform instead.
I have no idea how to implement this test. I see that the trait OffloadSingleNode has added a note, is it sufficient?
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.