Repository navigation
feat(arrow/ipc): store dictionary value type metadata in DictionaryEncoding - #1381
Draft
rustyconover wants to merge 1 commit into
Draft
rustyconover wants to merge 1 commit into
rustyconover wants to merge 1 commit into
Conversation
…coding
In IPC a dictionary-encoded field is a single Field whose type is the
dictionary value type, and Field.custom_metadata was the only place to put
extension type keys. As a result dictionary<values=extension<X>> and
extension<X, storage=dictionary<...>> serialized identically, and a
dictionary of UUID values could not be read back ("invalid storage type
for UUIDType: dictionary").
Following the format proposal for apache/arrow#49704, DictionaryEncoding
gains an optional custom_metadata member holding the metadata of the
dictionary value type:
- Regenerate arrow/internal/flatbuf/DictionaryEncoding.go with flatc
25.12.19 from Schema.fbs with the new member.
- The IPC writer puts the ARROW:extension:name and
ARROW:extension:metadata keys of an extension dictionary value type in
DictionaryEncoding.custom_metadata instead of Field.custom_metadata. An
extension type with dictionary storage still writes its keys to
Field.custom_metadata. This also fixes a panic when writing the schema
of an extension type with dictionary storage.
- The IPC reader applies DictionaryEncoding.custom_metadata to the value
type before wrapping it in the dictionary type, then applies
Field.custom_metadata as before. Only registered extension types are
applied; other keys and unregistered extension names are dropped.
- Removing the extension keys from field metadata no longer modifies the
slices returned by Metadata.Keys and Metadata.Values.
- The integration JSON reader and writer use an optional "metadata" list
in a field's "dictionary" object for the value type metadata, and the
reader now honours "isOrdered".
- dictutils.Mapper looks through extension value types for nested
dictionaries.
- A new arrow/internal/integration package registers the canonical
extension types; the IPC integration programs and the Flight
integration client and server import it. The C Data integration
library does not, since the C Data importer does not use the registry.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This was referenced Oct 16, 2026
zeroshade
reviewed
Oct 23, 2026
zeroshade
left a comment
Member
There was a problem hiding this comment.
Thanks — reviewed while still a draft, since it's gated on the apache/arrow#54959 vote. The overall approach looks right to me:
- Writer/reader symmetry is clean: extension keys of a dictionary's value type go to
DictionaryEncoding.custom_metadata, and the reader applies them to the value type before wrapping inDictionaryType, sodict<ext>andext<dict>are no longer conflated.fieldVisitoris per-field, sodictMetacan't leak, and an emptydictMetawrites no member. extensionTypeFromMetadatanow always builds fresh key/value slices — good catch on the oldappend(mdkeys[:i], ...)aliasing the caller's metadata.- Tests pin the behaviour: disabling the reader's use of
DictionaryEncodingmetadata fails 13 tests inarrow/ipc.
One thing we'd like changed before this lands:
DictionaryEncoding.custom_metadatashould survive a Go read→write round trip. Today, at field level, an unregistered extension name (and any other key) stays inField.Metadata, so Go passes it through unchanged. In the new path, anythingextensionTypeFromMetadatadoesn't consume — unregistered extension names, their serialized metadata, and any non-extension keys — is dropped on read, so reading and re-writing adict<unregistered-ext>file silently strips the annotation. We'd rather not introduce that pass-through regression, even as a documented limitation. Moving the leftover keys intoField.Metadatawouldn't work, since that reintroduces thedict<ext>/ext<dict>ambiguity on the next write. The leftover metadata needs to be carried with the dictionary's value type so the writer can emit it back intoDictionaryEncoding.custom_metadata. How to do that is open — e.g. a field onarrow.DictionaryTypefor value-type metadata, or an internal placeholder value type that keeps the unregistered name and serialized metadata — happy to discuss whichever you think fits best. A round-trip test with an unregistered extension name plus an extra non-extension key would pin it.- Minor:
dictutils.Mapper.ImportFieldunwraps one level of extension value type; an extension whose storage is itself an extension wrapping a nested type wouldn't be descended. Likely unreachable today.
This review was drafted by an AI-assisted tool and
confirmed by an Apache Arrow maintainer. After you've
addressed the points above and pushed an update, an Apache Arrow
maintainer — a real person — will take the next look
at the PR. The findings cite the project's review criteria;
if you think one of them is mis-applied, please reply on the
PR and a maintainer will weigh in.More on how Apache Arrow handles maintainer review:
CONTRIBUTING.md.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale for this change
Closes #1380. Part of apache/arrow#49704 and apache/arrow#54958.
In IPC a dictionary-encoded field is one
Fieldwhose type is the value type, sodictionary<values=extension<X>>andextension<X, storage=dictionary<...>>serialize identically. In arrow-go:arrow.uuidvalues is written with the UUID keys on theField, and reading it back fails (invalid storage type for UUIDType: dictionary);The dev@ discussion (https://lists.apache.org/thread/4xj0yrl2vn0lbrv39p3dvd28okvkjxyz) reached lazy consensus on adding
custom_metadata: [KeyValue]toDictionaryEncodingfor the dictionary value type's metadata.Companion drafts: Rust apache/arrow-rs#11622, nanoarrow apache/arrow-nanoarrow#972.
This depends on the format change in apache/arrow#54959 (
Schema.fbsand the columnar, C Data Interface and integration JSON specs), which is pending a dev@ vote. The C++ implementation and thedictionary_extensionarchery integration case are in apache/arrow#53994, which is stacked on it. It is the second reference implementation for that vote, so it should not be merged before the format change is accepted.What changes are included in this PR?
arrow/internal/flatbuf/DictionaryEncoding.goregenerated (flatc 25.12.19) with the newDictionaryEncoding.custom_metadatamember.ARROW:extension:name/ARROW:extension:metadatakeys of a dictionary's extension value type are written toDictionaryEncoding.custom_metadatainstead ofField.custom_metadata.Field.custom_metadata, and its schema no longer panics on write.DictionaryEncoding.custom_metadatato the value type, wraps the result in the dictionary type, then appliesField.custom_metadataas before.DictionaryTypehas nowhere to keep other value-type metadata, so other keys and unregistered extension names are dropped on read. That is a known limitation of this implementation; the format gives the member the same semantics asField.custom_metadata.arrow.Metadata."metadata"list in a field's"dictionary"object. The reader also now honours"isOrdered".dictutils.Mapper: finds nested dictionaries inside extension value types.arrow/internal/integrationpackage: registers the canonical extension types. It is imported byarrow-json-integration-test,arrow-file-to-stream,arrow-stream-to-fileand the Flight integration client and server. The C Data integration library doesn't import it, because its importer does not use the extension registry.Are these changes tested?
Yes.
DictionaryEncodingand notField;dict<uuid>;opaque<dict<uuid>>,list<dict<uuid>>and ordered dictionaries.extension,dictionary_extension,dictionary,nested_dictionaryandextension_unioncases.Are there any user-facing changes?
Yes:
Field.custom_metadatato the newDictionaryEncoding.custom_metadata.dictionary<extension<...>>in Go and in other implementations that support the new member."metadata"key in the"dictionary"object.This PR's code and description were written by AI (Claude Code) and reviewed by a human maintainer and an AI reviewer.
🤖 Generated with Claude Code