ozankabak commented on code in PR #5867:
URL: https://github.com/apache/arrow-datafusion/pull/5867#discussion_r1157562976
##########
datafusion/physical-expr/src/sort_expr.rs:
##########
@@ -116,9 +116,11 @@ impl PhysicalSortRequirement {
}
}
-pub fn make_sort_requirements_from_exprs(
- ordering: &[PhysicalSortExpr],
-) -> Vec<PhysicalSortRequirement> {
+pub type ExprOrdering = Vec<PhysicalSortExpr>;
+pub type ExprOrderingRef<'a> = &'a [PhysicalSortExpr];
+pub type OrderingRequirement = Vec<PhysicalSortRequirement>;
Review Comment:
I tried a refactor using full-fledged types but the resulting changes were
more involved/time-consuming, so I opted to go with aliases until we know there
are concrete use cases requiring multiple attributes in types such as
`ExprOrdering`.
I see two options:
- We can just merge #5863 and defer/revisit this kind of a refactor when we
have a real use case for multiple fields in types such as `ExprOrdering`.
- We can merge both and do another PR when/if such use cases arise.
I am OK with both, up to you.
--
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]
