[GLUTEN-10559] Simplify StrictRule and remove unnecessary DummyLeafExec by beliefer · Pull Request #10553 · apache/gluten

beliefer · 2025-08-27T09:06:58Z

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 · 2025-08-27T09:07:33Z

Run Gluten Clickhouse CI on x86

github-actions · 2025-08-27T12:13:31Z

Run Gluten Clickhouse CI on x86

github-actions · 2025-08-27T12:14:02Z

#10559

beliefer · 2025-08-28T02:53:20Z

ping @zhztheplayer @zml1206

zhztheplayer

Hiding the children helps simplifying the offloading process of one single node. For consecutive matching of different nodes, we normally add an individual "transform rule" for that. You can view VeloxRuleApi for more usage patterns.

May I know what led you to this attempt?

beliefer · 2025-08-29T07:08:02Z

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.

zhztheplayer · 2025-08-29T15:38:09Z

DummyLeafExec does not work well due to the transform up.

I may lose the contexts... Would you elaborate on this? Thanks.

beliefer · 2025-08-31T04:42:19Z

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.

zhztheplayer · 2025-09-10T07:20:19Z

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?

beliefer · 2025-09-10T09:33:07Z

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.

zml1206 · 2025-09-10T10:29:33Z

OffloadOthers(), OffloadExchange(), and OffloadJoin() do not traverse the plan internally, so I also think DummyLeafExec is not necessary.

zhztheplayer · 2025-09-10T11:24:34Z

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.

zml1206 · 2025-09-10T12:04:19Z

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?

github-actions · 2025-10-26T02:09:09Z