[GLUTEN-6202][VL] No extra SortExec needed before WindowExec if window type is sort
#6203
Conversation
|
Run Gluten Clickhouse CI |
WangGuangxin
force-pushed
the
fix_window_sort
branch
from
84704f2 to
b4bea80
Compare
|
Run Gluten Clickhouse CI |
|
cc @JkSelf |
WangGuangxin
force-pushed
the
fix_window_sort
branch
from
b4bea80 to
a9a1fd1
Compare
|
Run Gluten Clickhouse CI |
| child | ||
| case p @ ProjectExec(_, SortExec(_, false, child, _)) | ||
| if outputOrderSatisfied(child, transformer.requiredChildOrdering) => | ||
| p.copy(child = child) |
There was a problem hiding this comment.
Any reason not using p.withNewChildren to keep tags?
| private def outputOrderSatisfied(child: SparkPlan, required: Seq[Seq[SortOrder]]): Boolean = { | ||
| Seq(child).zip(required).forall { | ||
| case (child, requiredOrdering) => | ||
| SortOrder.orderingSatisfies(child.outputOrdering, requiredOrdering) | ||
| } | ||
| } |
There was a problem hiding this comment.
Could just assume an unary node is returned from replace.doReplace(window) then we can avoid zip to simplify code.
|
Run Gluten Clickhouse CI |
| val transformer = replace.doReplace(window) | ||
| val newChild = transformer.children.head match { |
There was a problem hiding this comment.
Oops, let's either add an assertion like assert(transformer.children.size == 1), or cast it to a unary node.
|
And CH CI failed https://opencicd.kyligence.com/job/gluten/job/gluten-ci/10302/, please also take a look. Thanks. |
|
Run Gluten Clickhouse CI |
WangGuangxin
force-pushed
the
fix_window_sort
branch
from
5340e52 to
f645924
Compare
|
Run Gluten Clickhouse CI |
WangGuangxin
force-pushed
the
fix_window_sort
branch
from
f645924 to
5c62404
Compare
|
Run Gluten Clickhouse CI |
|
Run Gluten Clickhouse CI |
|
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. |
What changes were proposed in this pull request?
When
spark.gluten.sql.columnar.backend.velox.window.typeis set tosort, there should be no extraSortExecbeforeWindowExecin plan.(Fixes: #6202)
How was this patch tested?
Mannual Test and UT