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

Added PartialOrd for FixedSizeListArray (and struct members) #6393

Conversation

ngli-me
Copy link
Contributor

@ngli-me ngli-me commented Sep 13, 2024

Which issue does this PR close?

Related apache/datafusion#8932.

Rationale for this change

Allows upstream users of FixedSizeListArray, such as DataFusion, to use derived PartialOrd.

What changes are included in this PR?

Added PartialOrd implementation for Array, ArrayData and Buffer.
Added derived PartialOrd for BooleanBuffer, NullBuffer, and FixedSizeListArray.

Are there any user-facing changes?

FixedSizeListArray and its subtypes will have PartialOrd available.

…d derived PartialOrd for BooleanBuffer, NullBuffer, and FixedSizeListArray.
@github-actions github-actions bot added the arrow Changes to the arrow crate label Sep 13, 2024
@ngli-me ngli-me changed the title Added PartialOrd for FixedSizeListArray Added PartialOrd for FixedSizeListArray (and struct members) Sep 13, 2024
@tustvold
Copy link
Contributor

tustvold commented Sep 13, 2024

This won't derive an implementation of PartialOrd that has any real meaning, for example nulls won't behave in a sensible manner.

I think datafusion should instead use the logic already in arrow-ord for this, I'm not sure PartialOrd on the arrays is the way to achieve this

@ngli-me
Copy link
Contributor Author

ngli-me commented Sep 13, 2024

Ah, I didn't even realize that existed, that's my bad. Sorry I'm newer to these projects, just been poking around. In that case, yeah this is not needed, I'll close it out. Thanks!

@ngli-me ngli-me closed this Sep 13, 2024
@ngli-me ngli-me deleted the feature/fixed-size-list-array-partial-ord branch September 13, 2024 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrow Changes to the arrow crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants