anandghegde opened a new pull request, #11078:
URL: https://github.com/apache/arrow-rs/pull/11078
# 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.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
