ARROW-14999: [C++] Don't check field name in ListType Equals()

[wjones127]

BREAKING CHANGE

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

• ListType and MapType internal field names only matter in cases where metadata also matters (but can be explicitly changed with options).
• Nullability of MapType internal fields now matters in comparison.

Examples

import pyarrow as pa

lt1 = pa.list_(pa.field("item", pa.int32(), nullable=False))
lt2 = pa.list_(pa.field("item", pa.int32(), nullable=True))
lt3 = pa.list_(pa.field("element", pa.int32(), nullable=False))
lt4 = pa.list_(pa.field("item", pa.int32(), nullable=False, metadata={"hello": "world"}))

# Nullability matters always:
lt1 == lt2 # False, was False
# Field names don't matter:
lt1 == lt3 # True, but was previously False
# ...unless you explicitly ask:
lt1.equals(lt3, check_internal_field_names=True) # False
# Metadata also doesn't matter:
lt1 == lt4 # True, was True
# ...unless you explicitly ask:
lt1.equals(lt4, check_metadata=True) # False


mt1 = pa.map_(pa.utf8(), pa.int32())
mt2 = pa.map_(pa.utf8(), pa.field("value", pa.int32(), nullable=False))
mt3 = pa.map_(pa.utf8(), pa.field("other", pa.int32()))
mt4 = pa.map_(pa.utf8(), pa.field("value", pa.int32(), metadata={"hello": "world"}))

# Nullability always matters:
mt1 == mt2 # False, was previously True
# Field names don't matter
mt1 == mt3 # True, was True
# ... unless you explicitly ask:
mt1.equals(mt3, check_internal_field_names=True) # False
# Metadata also doesn't matter:
mt1 == mt4 # True, was True
# ...unless you explicitly ask:
mt1.equals(mt4, check_metadata=True) # False

[github-actions]

https://issues.apache.org/jira/browse/ARROW-14999

[wjones127]

nullability of internal field is now part of the fingerprint.

[jorgecarleitao]

I think this should be raised in the mailing list - this is a non-trivial change to the spec - afaik internal names are an integral part of the field and must be taken into account in equality.

[wjones127]

I think this should be raised in the mailing list - this is a non-trivial change to the spec - afaik internal names are an integral part of the field and must be taken into account in equality.

@jorgecarleitao I was thinking about doing that; thanks for the nudge. To be clear, this PR makes checking field names optional, and keeps the check on by default in code paths where strict equality (where we also check field metadata) are already on. So I don't think this breaks the spec, but happy to discuss more on the mailing list.

https://lists.apache.org/thread/p6y48qznd61zxc78g3930h4nddz7oo4z

[wjones127]

@pitrou @paleolimbot Would you be willing to review?

[paleolimbot]

Just a few notes! You should take anything I say about C++ with a grain of salt, but I do wonder if the signature Equals(DataType|Field|Schema other, TypeEqualsOptions options) might be more appropriate than accumulating arguments? I can see how in some future somebody might not care about nullability either and that approach would help avoid even more arguments.

[paleolimbot]

This seems out of place?

[paleolimbot]

I think this would be more effective as a reprex() in ARROW-18204 rather than commented-out code here.

[paleolimbot]

Again, I would put this in the Jira rather than commented out code here.

[pitrou]

Do we really want to add a new option for this or can we just reuse check_metadata? New options add cognitive overhead, and I'm not sure there's a legitimate reason to decouple those two settings.

[wjones127]

Do we really want to add a new option for this or can we just reuse check_metadata? New options add cognitive overhead, and I'm not sure there's a legitimate reason to decouple those two settings.

I implemented it this way in #14847. I was skeptical at first, but it came out rather clean. I just added the field names to the metadata fingerprint.

[wjones127]

Closing in favor of #14847