[CORE] Drop redundant partial sort which has pre-project when offload sortAgg #6294
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 |
1 similar comment
|
Run Gluten Clickhouse CI |
| @@ -23,11 +23,27 @@ import org.apache.spark.sql.execution.{ProjectExec, SortExec, SparkPlan} | |||
| object SortUtils { | |||
| def dropPartialSort(plan: SparkPlan): SparkPlan = plan match { | |||
| case RewrittenNodeWall(p) => RewrittenNodeWall(dropPartialSort(p)) | |||
| case sort: SortExec if !sort.global => sort.child | |||
| case PartialSortChild(p) => p | |||
There was a problem hiding this comment.
Why we only drop partial sorts? I know it's previous code but I am still curious about that.
Also I'd suggest renaming:
PartialSortChild(p) -> PartialSortLike(child)
ProjectChild(p) -> ProjectLike(child)
There was a problem hiding this comment.
|
Run Gluten Clickhouse CI |
|
Run Gluten Clickhouse CI |
1 similar comment
|
Run Gluten Clickhouse CI |
| @@ -50,6 +50,32 @@ abstract class VeloxAggregateFunctionsSuite extends VeloxWholeStageTransformerSu | |||
| .set("spark.sql.sources.useV1SourceList", "avro") | |||
| } | |||
|
|
|||
| test("drop redundant partial sort which has pre-project when offload sortAgg") { | |||
There was a problem hiding this comment.
Would you like to move this test case down, to the bottom of this file, probably? Thanks.
|
Run Gluten Clickhouse CI |
What changes were proposed in this pull request?
As titile.
How was this patch tested?
UT