You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "osipovartem (via GitHub)" <gi...@apache.org> on 2026/09/22 20:30:21 UTC
[PR] [Variant] Preserve fixed-point JSON numbers as decimals [arrow-rs]
osipovartem opened a new pull request, #11177:
URL: https://github.com/apache/arrow-rs/pull/11177
## Which issue does this PR close?
- Closes #11169.
## What changes are included in this PR?
`JsonToVariant::append_json` now parses JSON directly into `VariantBuilderExt` while retaining exact numeric lexemes. The parser:
- preserves the existing smallest-width integer encoding for values fitting in `i64`;
- encodes fixed-point values and wider integers as the smallest fitting `Decimal4`, `Decimal8`, or `Decimal16`;
- keeps exponent notation and values outside the Variant decimal range as `Double`;
- applies the same rules recursively in arrays and objects;
- borrows unescaped strings directly from the input and only allocates when JSON escapes must be decoded;
- bounds nesting depth and does not commit a root value until the full input has been validated.
This deliberately avoids enabling serde_json's workspace-wide `arbitrary_precision` feature and removes the intermediate `serde_json::Value` tree from the string conversion path.
The first commit adds a defaulted fallible `VariantBuilderExt::try_append_value` method. This lets strict object builders report duplicate fields as `ArrowError` instead of panicking. It also fixes duplicate validation to check before mutating an object's field mapping, so the same builder remains usable after an error.
The existing `append_json(&serde_json::Value, ...)` API remains available. Its documentation now explains that non-integer numbers use `Double` because the original fixed-point lexeme is no longer available.
## Are these changes tested?
Yes. This enables the existing Decimal4/8/16 tests and adds coverage for nested decimals, precision/scale boundaries, malformed JSON, Unicode escapes and surrogate pairs, duplicate fields, recursion limits, error rollback, and the documented difference between the string and `Value` APIs.
Local validation:
- `cargo +1.95.0 test -p parquet-variant --lib` (175 passed)
- `cargo +1.95.0 test -p parquet-variant-json` (76 unit tests and 6 doctests passed)
- `cargo +1.95.0 test -p parquet-variant-compute --lib` (367 passed)
- `cargo +1.95.0 clippy -p parquet-variant-json --all-targets -- -D warnings`
- `cargo +1.89.0 check -p parquet-variant-json --all-targets`
A Criterion benchmark compares the direct parser with the previous Value-tree path. On this machine the direct path was faster for all covered inputs:
| Input | Direct | Value tree | Improvement |
|---|---:|---:|---:|
| Integers | 534 ns | 644 ns | 17% |
| Mixed object | 711 ns | 793 ns | 10% |
| Fixed-point decimals | 437 ns | 474 ns | 8% |
| Unescaped strings | 394 ns | 484 ns | 19% |
| Escaped strings | 527 ns | 561 ns | 6% |
| Nested object/list | 850 ns | 1,062 ns | 20% |
The decimal baseline encodes non-integer numbers as `Double`, while the direct path performs the new exact decimal conversion.
## Are there any user-facing changes?
Yes. Parsing fixed-point JSON text into Variant now preserves exact decimal semantics and precision instead of converting those values to `Double`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Variant] Preserve fixed-point JSON numbers as decimals [arrow-rs]
Posted by "osipovartem (via GitHub)" <gi...@apache.org>.
osipovartem commented on PR #11177:
URL: https://github.com/apache/arrow-rs/pull/11177#issuecomment-6012213232
Thanks, agreed. I split the fallible builder work into #11401, including regression coverage and an independent review. I will separate direct JSON parsing from fixed-point decimal conversion, move benchmarks to their own PR, and check the existing arrow-json decoder APIs before finalizing the parser change. I will update/close this combined PR as the replacements are ready.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Variant] Preserve fixed-point JSON numbers as decimals [arrow-rs]
Posted by "osipovartem (via GitHub)" <gi...@apache.org>.
osipovartem commented on PR #11177:
URL: https://github.com/apache/arrow-rs/pull/11177#issuecomment-5791792402
Independent read-only review completed: **APPROVED** (no blocking correctness, regression, performance, API-compatibility, or test findings).
The review follow-up is included in `d6db05a73`:
- `TopLevelRowSink` now forwards `try_append_value`, so fallible validation is preserved through the top-level wrapper.
- duplicate-key coverage includes escaped-equivalent and nested keys;
- an overwritten non-finite value remains rejected, confirming whole-input validation;
- Criterion now includes descending-key and duplicate-key objects.
Performance-first decision: the direct streaming parser remains unchanged. A buffered sort/compact approach was measured and rejected after causing material parsing regressions. Focused measurements for the new cases:
| Input | Direct | Value tree | Result |
|---|---:|---:|---:|
| Descending keys | 1.403 us | 1.621 us | direct ~13% faster |
| Duplicate keys | 795 ns | 678 ns | direct ~17% slower |
The duplicate-key case is an adversarial input where the direct parser observes every member while the Value-tree path collapses duplicates during deserialization. The common-path benchmark set remains at parity or faster, and no extra buffering, sorting, or per-row materialization was introduced.
Validation:
- `cargo +1.95.0 test -p parquet-variant-json` (76 unit tests + 6 doctests)
- focused `parquet-variant-compute` unshred tests (11 passed)
- `cargo +1.95.0 clippy -p parquet-variant-json --all-targets -- -D warnings`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Variant] Preserve fixed-point JSON numbers as decimals [arrow-rs]
Posted by "osipovartem (via GitHub)" <gi...@apache.org>.
osipovartem commented on PR #11177:
URL: https://github.com/apache/arrow-rs/pull/11177#issuecomment-6015666596
Quick correction to my earlier reply: #11401 overlapped with #11106, so I closed it. I extracted the distinct object-field rollback fix into #11403. I won't ask for review of this combined PR as-is; the remaining direct JSON parser, decimal conversion, and benchmarks still need separate PRs. I'll check the existing arrow-json decoder APIs while extracting those pieces, then close #11177 once its replacements are ready. Thanks for the clear split suggestion.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Variant] Preserve fixed-point JSON numbers as decimals [arrow-rs]
Posted by "sdf-jkl (via GitHub)" <gi...@apache.org>.
sdf-jkl commented on PR #11177:
URL: https://github.com/apache/arrow-rs/pull/11177#issuecomment-6003446036
Hi @osipovartem :wave: thanks for the PR
Could you please split this work into separate smaller PRs to make the review process easier?
I'd propose adding separate PRs for:
- benchmarks
- fallible builder
- direct json parsing ( for this, I'd take a closer look at the existing https://github.com/apache/arrow-rs/tree/main/arrow-json)
- decimal conversion
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Variant] Preserve fixed-point JSON numbers as decimals [arrow-rs]
Posted by "osipovartem (via GitHub)" <gi...@apache.org>.
osipovartem commented on PR #11177:
URL: https://github.com/apache/arrow-rs/pull/11177#issuecomment-6019611486
Split update: the direct parser is now draft #11409; the separate exact-decimal and benchmark patches are stacked draft PRs in Embucket/arrow-rs ([decimal #8](https://github.com/Embucket/arrow-rs/pull/8), [benchmark #9](https://github.com/Embucket/arrow-rs/pull/9)). I will submit those upstream once the parser dependencies land so each upstream diff stays reviewable. The distinct object rollback fix is #11403. I will keep this combined PR draft and close it after its replacements are upstream.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Variant] Preserve fixed-point JSON numbers as decimals [arrow-rs]
Posted by "osipovartem (via GitHub)" <gi...@apache.org>.
osipovartem closed pull request #11177: [Variant] Preserve fixed-point JSON numbers as decimals
URL: https://github.com/apache/arrow-rs/pull/11177
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org