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

ARROW-14999: [C++] Optional field name equality checks for map and list type #14847

Merged
merged 4 commits into from
Dec 8, 2022

Conversation

wjones127
Copy link
Member

@wjones127 wjones127 commented Dec 5, 2022

BREAKING CHANGE

Two changes for "internal fields" (fields within ListTypes and MapTypes):

  • ListType and MapType internal field names only matter in comparisons when check_metadata is true.
  • Nullability of MapType internal fields now matters in comparison.

@github-actions
Copy link

github-actions bot commented Dec 5, 2022

@pitrou
Copy link
Member

pitrou commented Dec 6, 2022

Seems like the Java test changes won't work as the latest stable PyArrow is used?

@wjones127
Copy link
Member Author

Seems like the Java test changes won't work as the latest stable PyArrow is used?

I'm pretty sure it uses local PyArrow. I got the tests passing in the other PR. I think the error is just that I added check_metadata to DataType.equals not Array.equals.

@wjones127 wjones127 marked this pull request as ready for review December 6, 2022 17:53
@wjones127
Copy link
Member Author

CI failures are unrelated.

Copy link
Member

@paleolimbot paleolimbot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The R side looks good to me! Thanks!

Copy link
Member

@pitrou pitrou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot @wjones127 ! This looks good on the princople, a couple minor comments.

python/pyarrow/types.pxi Outdated Show resolved Hide resolved
java/c/src/test/python/integration_tests.py Show resolved Hide resolved
cpp/src/arrow/type.cc Show resolved Hide resolved
wjones127 and others added 2 commits December 7, 2022 12:09
Co-authored-by: Antoine Pitrou <pitrou@free.fr>
@wjones127 wjones127 requested a review from pitrou December 8, 2022 15:55
Copy link
Member

@pitrou pitrou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, nice work @wjones127 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants