alamb commented on code in PR #6034:
URL: https://github.com/apache/arrow-datafusion/pull/6034#discussion_r1175324013
##########
datafusion-examples/examples/custom_datasource.rs:
##########
@@ -217,7 +217,7 @@ impl ExecutionPlan for CustomExec {
datafusion::physical_plan::Partitioning::UnknownPartitioning(1)
}
- fn output_ordering(&self) -> Option<&[PhysicalSortExpr]> {
+ fn output_ordering(&self) -> Option<Vec<PhysicalSortExpr>> {
Review Comment:
~Why does this signature need to change? The new signature requires an
allocation / `clone` of a Vec where the previous one didn't and thus this seems
to change the API for the worse.~
Update: this is explained in the PR description:
> This functionality requires us to calculate output_ordering dynamically.
For this reason, this PR accompanies the api change from fn
output_ordering(&self) -> Option<&[PhysicalSortExpr]> to fn
output_ordering(&self) -> Option<Vec<PhysicalSortExpr>>. To support dynamic
calculation of the output ordering. See the corresponding
https://github.com/synnada-ai/arrow-datafusion/pull/78 for more information.
The linked PR
https://github.com/synnada-ai/arrow-datafusion/pull/78#issue-1657633081 says
that the issue is related to the AggregateExec changing during optimization,
which I don't understand -- if the input changes then a new AggregateExec is
also created
--
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]
