ump45nose opened a new pull request, #1519:
URL: https://github.com/apache/arrow-go/pull/1519

   ### Rationale for this change
   
   `FromString` scaled a binary approximation of the input and then added one 
half before truncating. Exact decimal ties such as `0.35` and `0.15` are not 
representable in binary, so they landed just below or just above one half and 
rounded in different directions (`0.35` at scale 1 gave 3, `0.15` gave 2).
   
   ### What changes are included in this PR?
   
   A small internal helper, `arrow/internal/decimalstr`, rounds a plain decimal 
literal multiplied by `10^scale` directly from its digits, with ties away from 
zero, which is the rule the `+0.5` code was aiming for. 
`decimal.Decimal32FromString`, `decimal.Decimal64FromString`, 
`decimal128.FromString` and `decimal256.FromString` use it in the non-negative 
scale path after their existing magnitude checks, so the allocation bounds for 
very large and very small exponents still apply first. Inputs that are not 
plain decimal literals (for example a binary `p` exponent, which 
`big.ParseFloat` accepts) keep the previous float rounding. The negative scale 
path is unchanged.
   
   ### Are these changes tested?
   
   Yes. `TestFromStringRoundsExactTies` in `arrow/decimal`, `arrow/decimal128` 
and `arrow/decimal256`, plus unit tests for the helper (rounding, rejected 
inputs, digit bounds and the float fallback). The `arrow/decimal` and 
`arrow/decimal128` tie tests fail on main; the `decimal256` cases already 
passed there because of its wider float precision and are kept as regression 
coverage. `go test ./arrow/...` (with `arrow-testing`/`parquet-testing` data), 
`go test ./parquet/pqarrow/ ./parquet/schema/ ./parquet/variant/`, `go vet` and 
`gofmt` on the touched packages pass with Go 1.27.
   
   ### Are there any user-facing changes?
   
   Yes. Some in-range values that are exact decimal ties now round differently: 
they consistently round half away from zero, for example `0.35` at scale 1 now 
gives 4 instead of 3. No public API change.
   
   Note: this PR conflicts textually with #1515 near the same lines in 
`FromString`; I'm happy to rebase whichever lands second.
   
   Closes #1518.
   
   ---
   This change was prepared with AI assistance and reviewed before submission.
   


-- 
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]

Reply via email to