Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate LexOrderingRef and LexRequirementRef #13220

Closed
alamb opened this issue Nov 1, 2024 · 2 comments · Fixed by #13233
Closed

Deprecate LexOrderingRef and LexRequirementRef #13220

alamb opened this issue Nov 1, 2024 · 2 comments · Fixed by #13233
Assignees
Labels
enhancement New feature or request

Comments

@alamb
Copy link
Contributor

alamb commented Nov 1, 2024

Is your feature request related to a problem or challenge?

Follow on to #12591

Now that LexOrdering and LexRequirement are actual structs thanks to @ngli-me and @berkaysynnada on #13146, we can refer to their references as &LexOrdering and &LexRequirement directly rather than going through a type alias.

For example

///`LexOrderingRef` is an alias for the type &`[PhysicalSortExpr]`, which represents
/// a reference to a lexicographical ordering.
pub type LexOrderingRef<'a> = &'a [PhysicalSortExpr];

This is significantly less confusing than the current situation where we have both &LexOrdering and LexOrderingRef and &LexRequirement and LexRequirementRef

Describe the solution you'd like

I would like to

  1. Deprecate LexOrderingRef and LexRequirementRef
  2. Update all references to use &LexOrdering and &LexRequirement directly

Describe alternatives you've considered

Basically make this change:

///`LexOrderingRef` is an alias for the type &`[PhysicalSortExpr]`, which represents
/// a reference to a lexicographical ordering.
#[deprecated(since = "43.0.0", note = "use &LexOrdering instead")]
pub type LexOrderingRef<'a> = &'a [PhysicalSortExpr];

And the similar change to LexRequirementRef and then fix all the clippy errors :)

Additional context

I have a WIP here #13205 that someone could pull from

@alamb alamb added the enhancement New feature or request label Nov 1, 2024
@jatin510
Copy link
Contributor

jatin510 commented Nov 1, 2024

take

@alamb
Copy link
Contributor Author

alamb commented Nov 1, 2024

Thanks @jatin510 -- note you can't start on this until we merge #12591

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants