Json decoder factory - #10670
Draft
hareshkh wants to merge 2 commits into
Draft
Conversation
Preparatory work for allowing custom decoders in the arrow-json reader. A custom `ArrayDecoder` necessarily reads the tape, so `Tape` and `TapeElement` have to be public for such a hook to be usable. Rather than making `mod tape` public, this re-exports just the two types that a decoder needs. `TapeDecoder` stays private, so downstream code can read a `Tape` it is handed but cannot construct one, keeping the tape's production an implementation detail. `TapeElement` is marked `#[non_exhaustive]`. Its representation is an implementation detail — 64-bit values are split across two consecutive elements, and the type's own docs note that offsets may become a custom `u56` type — so downstream matches need a wildcard arm for that to remain a non-breaking change. Also documents two aspects of the tape that become public contract: string data is copied into the tape with escapes resolved, and numbers appear either as `Number` (parsing JSON text) or as the native `I32`/`I64`/`F32`/`F64` variants (serializing Rust values), so decoders must handle both.
hareshkh
force-pushed
the
json-decoder-factory
branch
from
August 12, 2026 23:37
91479ca to
63e65ba
Compare
The JSON writer has supported overriding how a type is encoded since apache#7015 via `EncoderFactory`. The reader has had no equivalent, so anything the built-in decoders don't do — a different binary encoding, an extension type — requires forking the crate. This adds the reader-side counterpart: * `ArrayDecoder` is now public, so callers can implement a decoder. * `DecoderFactory` is consulted for every data type before the reader's own dispatch, and returns `Ok(None)` to accept the default. * `DecoderContext::make_decoder` is now public, so a factory can delegate to the decoder the reader would otherwise have used. Without this, overriding anything nested would mean reimplementing all of its children. * `ReaderBuilder::with_decoder_factory` registers a factory. Prior art: apache#9021 and apache#9272, both of which went stale.
hareshkh
force-pushed
the
json-decoder-factory
branch
from
August 12, 2026 23:53
63e65ba to
d38a0dd
Compare
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
1 participant
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.
Which issue does this PR close?
Rationale for this change
EncoderFactory, plus a publicmake_encoderto delegate to defaults). The reader has no equivalent.DecoderContextalready sits onmain.What changes are included in this PR?
TapeElement#[non_exhaustive], documenting numbers asNumber(JSON text) or nativei32/i64/f32/f64(serde path), 64-bit spanning two elements.ArrayDecoderpublic;posholds one tape index per output row.DecoderFactory, consulted before the reader's dispatch;Ok(None)accepts the default.DecoderContext::make_decoderpublic, so a factory can delegate to the decoder the reader would otherwise use - without it, overriding a nested type means reimplementing its children (@scovich's point on Allow extensions to arrow-json decoder and include an extension for variant #9021).ReaderBuilder::with_decoder_factory.Are these changes tested?
Yes. Unit tests and a doctest decoding
Binaryfrom a JSON int array - the inverse of the existingEncoderFactorydoctest, so the two round-trip, no new dependency.Are there any user-facing changes?
New:
arrow_json::{Tape, TapeElement, ArrayDecoder, DecoderFactory},DecoderContext::{make_decoder, decoder_factory},ReaderBuilder::with_decoder_factory.