Skip to content

fix: Consolidate string-to-decimal parsing into a single code path - #10850

Open
neilconway wants to merge 1 commit into
apache:mainfrom
neilconway:neilc/consolidate-decimal-parsing
Open

fix: Consolidate string-to-decimal parsing into a single code path#10850
neilconway wants to merge 1 commit into
apache:mainfrom
neilconway:neilc/consolidate-decimal-parsing

Conversation

@neilconway

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

arrow-cast had two different string-to-decimal parsers: parse_string_to_decimal_native, used by cast, and parse_decimal, used by the CSV and JSON readers. Aside from redundancy, these code paths behaved differently (e.g., truncating vs rounding for digits beyond the target type's scale, whitespace trimming, support for e-notation, etc.), so decimal conversion behaved differently depending on how the decimal value arrived into arrow-rs.

This PR replaces these parsers with a single unified parser; parse_decimal(s, precision_scale) is now the public entry point and parse_string_to_decimal_native is a thin, deprecated wrapper over it. The new parser is based on the one-pass, u64-chunked parser in #10668, extended with support for e-notation and negative scales. This fixes a lot of bugs and ensures consistent behavior, but it does result in some behavior changes and a small performance regression for the CSV/JSON path; more details below.

Bugs fixed: (all in the JSON/CSV path)

Behaviour changes:

Performance:

  • cast string-to-decimal: ~unchanged. The cast path already used the fast single-pass parser from perf(arrow-cast): optimize parsing of decimals from strings #10668; the unified parser benchmarks within Criterion noise (~4%) of it.

  • CSV/JSON reader path: short inputs cost 1-2 ns more per value, while long Decimal256 inputs are ~35% faster. End-to-end, CSV reads of decimal columns are ~8% slower. I suspect there is room for further optimization here (which will now benefit both code paths!) to reach or exceed the previous performance, but I'd like to land the unified parser first before we tackle further optimizations.

What changes are included in this PR?

See above.

Are these changes tested?

New tests added to cover rounding, exponents, negative scale, whitespace, long inputs and the four widths, a seeded differential test checks 20k random inputs against a BigInt reference (num-bigint was added as a dev-dependency), and the CSV/JSON readers gain end-to-end tests for the new behavior and bugfixes listed above.

Are there any user-facing changes?

Yes; a deprecated public API, and user-visible behavioral changes in decimal parsing.

AI usage

Iterated primarily with CC Fable 5; code reviewed by Codex GPT 5.6. I read, understand, and revised the resulting code.

arrow-cast previously contained two different string-to-decimal parsers:
`parse_string_to_decimal_native`, used by `cast`, and `parse_decimal`,
used by the CSV and JSON readers.  Aside from redundancy, these code
paths behaved differently (e.g., truncating vs rounding for digits
beyond the target type's scale, whitespace trimming, support for
e-notation, etc.), so decimal conversion behaved differently depending
on how the decimal value arrived into arrow-rs.

This branch replaces these two parsers with a single unified parser;
`parse_decimal(s, precision_scale)` is now the public entry point and
`parse_string_to_decimal_native` is a thin, deprecated wrapper over it.

The new parser is based on the one-pass, u64-chunked parser in apache#10668,
extended with support for e-notation and negative scales. This fixes a
lot of bugs and ensures consistent behavior, but it does result in some
behavior changes and a small performance regression for the CSV/JSON
path; more details below.

Bugs fixed: (all in the JSON/CSV path)

- divide-by-zero panic or wrong values for some inputs in exponent
  notation (apache#10788, apache#5762)
- overflow on inputs with >= 256 digits or long exponents (apache#10787)
- negative scales were ignored (apache#10791)
- `0e0`/`-0e0` were rejected (apache#10789) while `e5` and `-.` parsed as 0
  (apache#10790)

Behaviour changes:

- CSV and JSON readers now round half away from zero instead of
  truncating digits beyond the scale (apache#9410, apache#9422, apache#7355)
- `cast` from strings accepts exponent notation (apache#5068) and negative
  scales, which `can_cast_types` already advertised (apache#10792), and
  validates the target precision and scale before parsing any values
- CSV and JSON readers now trim whitespace (apache#10793). Only ASCII
  whitespace characters are trimmed, which matches the behavior of the
  CSV float/int parsers; previously, the `cast` path trimmed Unicode
  whitespace as well, but it will no longer do so.
- parse errors use `ArrowError::ParseError` with unified messages
- `variant_get` validates the target precision for string inputs
  (apache#10794)

Tests: new tests cover rounding, exponents, negative scale, whitespace,
long inputs and the four widths, a seeded differential test checks 20k
random inputs against a BigInt reference (num-bigint was added as a
dev-dependency), and the CSV/JSON readers gain end-to-end tests for the
new behavior and bugfixes listed above.

Performance:

- `cast` string-to-decimal: ~unchanged. The cast path already used the
  fast single-pass parser from apache#10668; the unified parser benchmarks
  within Criterion noise (~4%) of it.

- CSV/JSON reader path: short inputs cost 1-2 ns more per value, while
  long Decimal256 inputs are ~35% faster. End-to-end, CSV reads of
  decimal columns are ~8% slower. I suspect there is room for further
  optimization here (which will now benefit both code paths!) to reach
  or exceed the previous performance, but I'd like to land the unified
  parser first before we tackle further optimizations.

Closes apache#10787, closes apache#10788, closes apache#10789, closes apache#10790,
closes apache#10791, closes apache#10792, closes apache#10793, closes apache#10794
@github-actions github-actions Bot added arrow Changes to the arrow crate parquet-variant parquet-variant* crates arrow-json arrow-csv arrow-cast labels Aug 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment