Skip to content

Move remaining decimal adapters into the decimal submodule - #11078

Open
anandghegde wants to merge 1 commit into
apache:mainfrom
anandghegde:extract-decimal-cast-adapters
Open

anandghegde wants to merge 1 commit into
apache:mainfrom
anandghegde:extract-decimal-cast-adapters

Conversation

@anandghegde

Copy link
Copy Markdown

Which issue does this PR close?

Part of #11032 — the remaining decimal adapters group only.

Sibling PRs against the same issue: #11075 (temporal), #11076 (numeric and
boolean), #11077 (binary and byte-view). With this one, the four groups I took
cover everything in the issue's list except the struct helpers, which
@amitvijapur mentioned taking. All four are independent and move disjoint
functions, but each touches the imports at the top of mod.rs, so whichever
lands later will want a trivial rebase.

Rationale for this change

arrow-cast/src/cast/mod.rs is ~14k lines. #5125 moved list, decimal,
dictionary, string, map, run-array and union support into private submodules,
and #11032 lists the groups still inline. The integer-to-decimal and generic
decimal routing helpers are the last of them.

What changes are included in this PR?

arrow-cast/src/cast/decimal.rs gains:

  • single_decimal_to_float_lossy
  • integer_to_decimal_native
  • cast_integer_to_decimal
  • cast_from_decimal
  • cast_to_decimal

cast_with_options stays the top-level dispatcher in mod.rs.

Worth flagging for review:

  • These go into the existing decimal module, not a new one. The issue
    calls them "remaining decimal adapters" and cast/decimal.rs already exists,
    so a second decimal module seemed like the wrong shape.
  • single_decimal_to_float_lossy needed an explicit re-export, and this is
    the one thing here that is not a pure move.
    It is pub and reachable today
    as arrow_cast::cast::single_decimal_to_float_lossy. decimal is a private
    module that mod.rs pulls in with a plain use, so moving the function there
    silently made it private — a breaking change that compiles and passes the
    whole test suite. mod.rs now re-exports that one name explicitly, which
    restores the path without exposing anything else from the module.
  • Visibility is minimal. cast_from_decimal and cast_to_decimal are called
    by the dispatcher and became pub(crate). integer_to_decimal_native and
    cast_integer_to_decimal have no callers outside the moved code and stay
    private.
  • The insertion point is above decimal.rs's own #[cfg(test)] mod tests, since
    clippy rejects items after a test module.

Are these changes tested?

Covered by the existing arrow-cast tests — this is a code move, so no new
tests are warranted and no existing test was modified.

cargo test -p arrow-cast     379 passed + 12 passed, 0 failed (same as base)
cargo clippy -p arrow-cast --all-targets -- -D warnings   clean
cargo fmt -p arrow-cast -- --check                        clean
cargo build -p arrow                                      builds

Three mechanical checks rather than a reading, since a code move is only worth
anything if it is faithful:

  • The moved code is byte-identical to the three original ranges from HEAD
    once the added pub(crate) is normalised away.
  • decimal.rs's pre-existing content, and its test module, are unchanged
    checked by comparing the regions either side of the insertion point. The diff
    is a symmetric +259/-259 apart from the re-export.
  • A temporary integration test calls arrow_cast::cast::single_decimal_to_float_lossy
    through its public path and passes. This is what caught the privacy
    regression described above; it is not part of this diff.

Run on macOS 26.6.2, aarch64-apple-darwin, toolchain 1.98.1 from
rust-toolchain.toml.

Are there any user-facing changes?

No — but only because of the explicit re-export. Without it this would have been
a silent breaking change for anyone calling
arrow_cast::cast::single_decimal_to_float_lossy.

AI usage disclosure

Written with AI assistance (Claude). The AI performed the mechanical extraction
and ran the verification above; I reviewed the result and own the change.

The moved bodies are unedited, and the byte-identity check is the evidence for
that. The judgement calls are listed above: reusing the existing decimal
module, the explicit re-export, the visibility split, and the insertion point.
The re-export in particular is worth a reviewer's attention, because it is the
only line here that is not a move, and nothing in the test suite would have
failed without it.

Follow-up to apache#5125, completing the extractions in apache#5537 and friends.

Move the integer-to-decimal and generic decimal routing helpers out of
`arrow-cast/src/cast/mod.rs` into the existing `decimal` submodule rather
than a new one, leaving `cast_with_options` as the top-level dispatcher.

The bodies are moved verbatim. `cast_from_decimal` and `cast_to_decimal`
are called from the dispatcher and become `pub(crate)`;
`integer_to_decimal_native` and `cast_integer_to_decimal` have no callers
outside the moved code and stay private.

`single_decimal_to_float_lossy` is public API. `decimal` is a private
module imported with a plain `use`, so moving it there would have made it
unreachable; it is re-exported explicitly from `mod.rs` to keep
`arrow_cast::cast::single_decimal_to_float_lossy` resolving as before.

No behavioural change and no public API change.
@github-actions github-actions Bot added arrow Changes to the arrow crate arrow-cast labels Sep 13, 2026
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-cast

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant