Use type aliases ExprOrdering and OrderingRequirements to simplify ordering-related type signatures

[ozankabak]

Which issue does this PR close?

Takes a first step at addressing this concern.

Rationale for this change

Function signatures involving types related to expression ordering are starting to get overly verbose/complex. We would like to have simpler types/function signatures for readability and maintainability reasons.

What changes are included in this PR?

We are introducing the following aliases:

pub type ExprOrdering = Vec<PhysicalSortExpr>;
pub type ExprOrderingRef<'a> = &'a [PhysicalSortExpr];
pub type OrderingRequirement = Vec<PhysicalSortRequirement>;

to simplify function signatures. If these aliases don't suffice in the future and we decide to use full-fledged types for this purpose, this PR will serve as a first step in that refactoring too.

Are these changes tested?

Yes, all existing tests pass.

Are there any user-facing changes?

Yes and no. Type-wise, nothing has changed (as these are type aliases). But a cursory look at traits and function signatures may give the impression that they have changed.

[alamb]

While these do simplify the signatures, I am a little worried about just adding a type signature without additional supporting documentation.

If we are going to change the signatures again what do you think about going all the way and actually making structures

struct ExprOrdering {
  exprs: Vec<PhysicalSortExpr>;
}

And then make functions like (with documentation):

impl ExprOrdering {
  fn new (exprs: Vec<PhysicalSortExpr>) -> Self {.._

  fn from_requirements(requirements: OrderingRequirement -> Self {..}
}

For example, here is a PR that tries to encapsulate more of the PhysicalSortRequirements along with a bunch of docs: #5863

[ozankabak]

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 Encapsulate PhysicalSortRequrement and add more doc comments #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.

[alamb]

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.

This makes sense -- thank you for trying. It definitely took me far more time to make #5863 than I expected -- mostly to come up with coherent description of what the structures did.

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.

I think this is my preference for the moment. Let's wait to see if anyone else has comments

[ozankabak]

@mingmwang, what do you think? Any preference on your side?

[ozankabak]

I will close this for now, we can revisit this issue in the future if we like.

[alamb]

Thank you @ozankabak -- I appreciate your attention to trying to make the code easier to work with