fix(arrow/array): use scale-aware ValueStr in decimal array String()#849
Merged
Merged
Conversation
Decimal arrays printed the raw decimal.Num struct (e.g. {1999 0}) from String(), so printing an array or RecordBatch showed unscaled values instead of the logical decimal. Switch to ValueStr, which already formats using the type's scale (matching GetOneForMarshal and JSON output).
baseDecimal is generic, so this fixes Decimal32/64/128/256, and RecordBatch printing inherits the corrected output via the column String() path.
lidavidm
approved these changes
Jun 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale for this change
Decimal arrays stringify the raw
decimal.Numstruct instead of the scaled logical value.fmt.Printlnon a decimal array — or on aRecord/RecordBatchcontaining one — prints e.g.{1999 0}instead of19.99, even thoughValueStralready formats the value correctly using the type's scale. This is inconsistent withValueStr/JSON output and with other Arrow implementations (e.g. PyArrow prints19.99).What changes are included in this PR?
baseDecimal[T].String()now formats each non-null value viaValueStr(which applies the type's scale throughGetOneForMarshal) instead offmt.Sprintf("%v", Value(i)). BecausebaseDecimalis generic, this correctsDecimal32/64/128/256at once, andRecordBatchprinting inherits the fix through the columnString()path (record.go).decimal128_test.go/decimal256_test.gothat pinned the old raw-struct output (they were already inconsistent with the adjacentValueStrexpectations on the next lines).TestDecimal128StringScaled, reproducing the issue (decimal128(5, 2)=19.99) and asserting both the arrayString()and theRecordBatchoutput.Are these changes tested?
Yes.
go test ./arrow/array/passes, including the new regression test and the updated slice tests.go build ./...andgo vet/golangci-lint (via pre-commit) pass.Are there any user-facing changes?
Yes — the textual output of
String()on decimal arrays, and of anyRecord/RecordBatchcontaining decimal columns, now shows scaled decimal values (e.g.19.99) instead of the internal struct (e.g.{1999 0}). This only affects human-readable stringification; binary/IPC/JSON representations are unchanged.Closes #848