Skip to content

chore: formalize the default map field names to match default arrow spec - #10297

Merged
Jefffrey merged 5 commits into
apache:mainfrom
rluvaton:update-default-map-name
Jul 11, 2026
Merged

chore: formalize the default map field names to match default arrow spec#10297
Jefffrey merged 5 commits into
apache:mainfrom
rluvaton:update-default-map-name

Conversation

@rluvaton

@rluvaton rluvaton commented Jul 7, 2026

Copy link
Copy Markdown
Member

Which issue does this PR close?

N/A

Rationale for this change

In some places we write keys and values and in some cases key and value (without the s)

so matching the names to match the default arrow spec names

this is also important as this is a source of common pitfalls when comparing data types from schema and actual data (where actual data might coming from another arrow impl like java which uses key and value and not keys and values - https://github.com/apache/arrow-java/blob/a9f0086090e649b83c89ea864c82ed7fcc84ab2f/vector/src/main/java/org/apache/arrow/vector/complex/MapVector.java#L47-L49)

This is similar to what was done for list:

What changes are included in this PR?

add constants for entries, key and value field name in Field, replace manually every place, left out places that test different names specifically or made it more explicit

I did it manually since I don't trust LLM that will do it correctly and might change tests that check that case specifically and did not want to use a script to change that

Are these changes tested?

Existing tests

Are there any user-facing changes?

the default MapFieldNames is key and not keys, value and not values

In some places we write `keys` and in some `key`
@github-actions github-actions Bot added parquet Changes to the parquet crate arrow Changes to the arrow crate arrow-flight Changes to the arrow-flight crate parquet-variant parquet-variant* crates arrow-avro arrow-avro crate labels Jul 7, 2026

@yonipeleg33 yonipeleg33 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice! Some cleanups of hard-coded strings, otherwise LGTM

Comment thread arrow-array/src/array/map_array.rs Outdated
Comment thread arrow-array/src/array/map_array.rs Outdated
Comment thread arrow-array/src/array/map_array.rs Outdated
Comment thread arrow-array/src/builder/map_builder.rs Outdated

@Jefffrey Jefffrey left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

makes sense, thanks

@rluvaton

rluvaton commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

@Jefffrey can you please merge?

@Jefffrey
Jefffrey merged commit b963ecf into apache:main Jul 11, 2026
35 checks passed
@Jefffrey

Copy link
Copy Markdown
Contributor

thanks @rluvaton

@rluvaton
rluvaton deleted the update-default-map-name branch July 12, 2026 17:04
@alamb alamb added the enhancement Any new improvement worthy of a entry in the changelog label Jul 31, 2026
@alamb

alamb commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

The actual Arrow spec uses key and value as well for the field ames

https://github.com/apache/arrow/blob/35c5ffd12173284406e4a2c86405415444e596d7/format/Schema.fbs#L120

/// A Map is a logical nested type that is represented as
///
/// List<entries: Struct<key: K, value: V>>

@alamb

alamb commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

I hit an error due to this change when testing the upgrade to DataFusion:

I was working on

Here are the errors

thread 'tokio-rt-worker' (41727) panicked at /usr/local/cargo/git/checkouts/arrow-rs-583cca34693b79b8/1359cb9/arrow-array/src/array/struct_array.rs:91:46:
called `Result::unwrap()` on an `Err` value: InvalidArgumentError("Incorrect datatype for StructArray field \"metadata\", expected Map(\"entries\": non-null Struct(\"keys\": non-null Utf8, \"values\": Utf8), unsorted) got Map(\"entries\": non-null Struct(\"key\": non-null Utf8, \"value\": Utf8), unsorted)")
stack backtrace:

I think this PR is a good one to align to the spec, but I think we should probably treat it as a "breaking API change" and hold it for the next breaking release to minimize downstream churn on a minor release

I will prepare a PR

alamb added a commit to alamb/datafusion that referenced this pull request Jul 31, 2026
arrow-rs formalized MapBuilder's default field names from keys/values
to key/value to match the Arrow spec (apache/arrow-rs#10297). Update
the hardcoded schema in arrow_field()/arrow_metadata() and the
nested_struct cast test to match, fixing a StructArray::new panic on
mismatched field names.
@alamb

alamb commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

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 arrow-avro arrow-avro crate arrow-flight Changes to the arrow-flight crate enhancement Any new improvement worthy of a entry in the changelog parquet Changes to the parquet crate parquet-variant parquet-variant* crates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants