2010YOUY01 commented on PR #27393:
URL: https://github.com/apache/datafusion/pull/27393#issuecomment-6868985044
@tschwarzinger thanks for the review!
Regarding extra projection: it's a great point, we could do it in a
follow-up PR
> I think we have code that solves this problem in
`datafusion_physical_optimizer::output_requirements::require_top_ordering`. It
looks at `maintains_input_order` and keeps the sort requirement. Maybe we
should re-use that piece of code? Then we would also have consistency between
`OutputRequirements` and `DataSinkExec`.
I think the challenge is, for the below plan shape:
```
DataSinkExec
SubPlan
```
- Derived output ordering of subplan is expected to be wrong before
`EnsureRequirements` rule is finished
- If DataSinkExec tries to set `required_input_ordering` after
`EnsureRequirements`, the subplan might even get removed in an unintended way.
I suspect we need some deep plumbing into `EnsureRequirements`'s sorting
part, to solve it more elegantly.
> The only downside I see is that custom sort operators no longer work for
`COPY` (but they have a hard time in general considering the output
requirements case IIRC). So I think we should address that once a use case
comes along with another planner info `established_output_ordering` or
something like that.
Question: any case that still won't work with explicitly extra sort, like
```
# This initial plan shape keeps written file order, while avoids re-sorting
during later optimizations
DataSinkExec
SortExec // explicit order by 'k'
CustomExec // existing order by 'k'
```
----
So I think the future work should be implementing a general helper to detect
explicit sorts like
```
DataSinkExec
SortExec
DataSinkExec
Projection
SortExec
... and other possible shapes
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]