Skip to content

GH-49415: [C++] Don't change map type key/item/value field names#49416

Merged
kou merged 1 commit intoapache:mainfrom
kou:cpp-map-name
Mar 2, 2026
Merged

GH-49415: [C++] Don't change map type key/item/value field names#49416
kou merged 1 commit intoapache:mainfrom
kou:cpp-map-name

Conversation

@kou
Copy link
Member

@kou kou commented Mar 1, 2026

Rationale for this change

The current implementation forces using key/value/entries as key/item/value field names and ignores field names in FlatBuffers.

What changes are included in this PR?

Use field names in FlatBuffers.

Are these changes tested?

Yes.

Are there any user-facing changes?

Yes.

This PR includes breaking changes to public APIs. If an application depends on key/value/entriesas map type's field names, an application may not work when incoming Apache Arrow data doesn't usekey/value/entries` as map type's field names.

@kou kou requested review from lidavidm, wgtmac and wjones127 March 1, 2026 06:11
@github-actions
Copy link

github-actions bot commented Mar 1, 2026

⚠️ GitHub issue #49415 has been automatically assigned in GitHub to PR creator.

@kou
Copy link
Member Author

kou commented Mar 1, 2026

@wgtmac @lidavidm @wjones127 What do you think about this? This code was changed by #35298 and you worked on the PR.

Copy link
Member

@lidavidm lidavidm left a comment

Choose a reason for hiding this comment

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

I wonder if any users may have inadvertently come to depend on this behavior. Should we highlight it as a potentially breaking change?

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting committer review Awaiting committer review labels Mar 1, 2026
@wgtmac
Copy link
Member

wgtmac commented Mar 1, 2026

I think I've recently encountered a similar issue where Parquet and Arrow uses different hardcoded field names for map type: #49218 (comment)

@kou
Copy link
Member Author

kou commented Mar 1, 2026

I wonder if any users may have inadvertently come to depend on this behavior. Should we highlight it as a potentially breaking change?

OK. I've updated the PR description for it.

FYI: @raulcd you may be a release manager for the next major release. We want to highlight this change in the release announce blog post.

@raulcd raulcd added the Breaking Change Includes a breaking change to the API label Mar 2, 2026
@kou
Copy link
Member Author

kou commented Mar 2, 2026

I'll merge this.

@kou kou merged commit 0bddf5d into apache:main Mar 2, 2026
72 of 76 checks passed
@kou kou deleted the cpp-map-name branch March 2, 2026 23:58
@kou kou removed the awaiting merge Awaiting merge label Mar 2, 2026
@conbench-apache-arrow
Copy link

After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit 0bddf5d.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 7 possible false positives for unstable benchmarks that are known to sometimes produce them.

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

Labels

Breaking Change Includes a breaking change to the API Component: C++

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants