apache / apache/arrow-rs

[arrow-cast] consider simplifying `parse_decimal` and `parse_e_notation`

Open
#9,170 1 comment 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
Rust
Stars
3.6k
Forks
1.3k
Avg merge
2d 18h
Merged PRs (30d)
169

Description

**Is your feature request related to a problem or challenge? Please describe what you are trying to do.**
Currently there isn't a clear separation of concerns between `parse_decimal` and `parse_e_notation`, as a part of the former functions logic leaks into the latter one.

Namely, to paraphrase from [here](https://github.com/apache/arrow-rs/pull/8700#discussion_r2676341440)
> in [`parse_decimal`] we skip parsing any fractionals after we reach `scale` digits, not knowing ahead of time whether the decimal contains an e-notation or not. So once we do hit into an e-notation, and drop down into [`parse_e_notation`], we need to parse the remaining unprocessed fractionals too, since otherwise we might lose precision.

Besides making this cognitively complex, it also leads to some preventable edge cases, such as this one https://github.com/apache/arrow-rs/pull/8700#issuecomment-3696913488

**Describe the solution you'd like**
One option (as suggested in the previously linked comment) would be to do `s.split_once(['e', 'E'])` at the start of `parse_decimal`, calling `parse_e_notation` optionally when e-notation is detected, and then proceeding to parse fractionals from the input decimal, at this point knowing exactly how many digits we'll need to retain.

**Describe alternatives you've considered**
Alternatively, there probably exists a one-pass algorithm that instead of parsing forward parses the decimal backward.

**Additional context**

Contributor guide

Open the contributing guide

Research direction

Start by locating the parse_decimal and parse_e_notation entry points and read the linked discussion for the parsing concerns and edge case. Compare the proposed split-on-e-notation approach with the one-pass alternative, then verify that decimal precision and the reported edge case are preserved.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
data
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.