Skip to content

pa.compute.sum result for decimal128 doesn't fit into precision/scale #35166

New issue
New issue
Closed
Closed
pa.compute.sum result for decimal128 doesn't fit into precision/scale#35166
Assignees
khwilson
Labels
Component: PythonType: bug
Milestone
21.0.0

Description

@wirable23
wirable23
opened on Apr 16, 2023

Describe the bug, including details regarding any error messages, version, and platform.

import pyarrow as pa
import decimal

>>> arr = pa.array([decimal.Decimal("9.999"), decimal.Decimal("1.234"), decimal.Decimal("1.234"), decimal.Decimal("1.234")], type=pa.decimal128(4,3))
>>> sum_scalar = pa.compute.sum(arr)
>>> sum_scalar
<pyarrow.Decimal128Scalar: Decimal('13.701')>
>>> sum_scalar.type
Decimal128Type(decimal128(4, 3))
>>>

The decimal "13.701" cannot fit into a decimal with scale/precision 4/3. An error is raised when trying to create the scalar directly:

>>> pa.scalar(decimal.Decimal("13.701"), type=pa.decimal128(4,3))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "pyarrow\scalar.pxi", line 1100, in pyarrow.lib.scalar
  File "pyarrow\error.pxi", line 144, in pyarrow.lib.pyarrow_internal_check_status
  File "pyarrow\error.pxi", line 100, in pyarrow.lib.check_status
pyarrow.lib.ArrowInvalid: Decimal type with precision 5 does not fit into precision inferred from first array element: 4
>>>

So it seems the type of the returned scalar should be Decimal128Type(decimal128(5, 3)), not Decimal128Type(decimal128(4, 3)). It seems sum tries to preserve original scale/precision of array, when likely the sum will not fit in those bounds.

Component(s)

Python

Activity

  1. wirable23
    added
    Type: bug
    on Apr 16, 2023
  2. github-actions
    added
    Component: Python
    on Apr 16, 2023
  3. wirable23
    changed the title [-]pa.compute.sum result for decimal doesn't fit into precision/scale[/-] [+]pa.compute.sum result for decimal128 doesn't fit into precision/scale[/+] on Apr 16, 2023

  4. westonpace commented on Apr 19, 2023

    @westonpace
    westonpace
    on Apr 19, 2023
    Member

    Yikes. That's definitely a bug. I think the return type of sum should probably maximize the P parameter.

    E.g. SUM(Decimal128<X,Y>) -> Decimal<38,Y> and SUM(Decimal256<X,Y>) -> Decimal<76,Y>. This matches the rules for Substrait as well as matches what SQL server does.

  5. rohanjain101 commented on Apr 19, 2023

    @rohanjain101
    rohanjain101
    on Apr 19, 2023

    @westonpace thanks for taking a look. Is there documentation for scale/precision rules for decimal128 in different API's? For example, I found addition follows these rules: https://docs.aws.amazon.com/redshift/latest/dg/r_numeric_computations201.html are used for binary expressions, but couldn't find scale/precision rules for other API's like sum, concat_table, publically documented. Does this information exist somewhere, or does it need to be found look at internal compute implementation?

  6. westonpace commented on Apr 20, 2023

    @westonpace
    westonpace
    on Apr 20, 2023
    Member

    but couldn't find scale/precision rules for other API's like sum, concat_table, publically documented. Does this information exist somewhere, or does it need to be found look at internal compute implementation?

    I'm not sure there is any particular place that we document these rules. If it isn't found in the function description then it is likely missing.

  7. wirable23 commented on Apr 20, 2023

    @wirable23
    wirable23
    on Apr 20, 2023
    Author

    Where would I go to find these rules? In the internal compute implementation? Should the decimal scale/precision semantics be publically documented as it seems fairly fundamental to the behavior of the API?

  8. westonpace commented on Apr 26, 2023

    @westonpace
    westonpace
    on Apr 26, 2023
    Member

    Where would I go to find these rules? In the internal compute implementation?

    Yes, unfortunately.

    Should the decimal scale/precision semantics be publically documented as it seems fairly fundamental to the behavior of the API?

    Yes, I'd definitely welcome more documentation on how this happens. There isn't a great spot for it today beyond the function doc string itself (e.g. what you get back from help(pc.sum)) and this page: https://arrow.apache.org/docs/cpp/compute.html#compute-function-list.

  9. khwilson commented on Sep 8, 2024

    @khwilson
    khwilson
    on Sep 8, 2024
    Contributor

    I know this is old, but I was curious about this. It seems that for binary operations, PyArrow follows the rules of Redshift (see #40123). However, for array aggregators (specifically, sum) it seems to retain the input type (which is definitely wrong). I was curious what some other databases do, and so attempted to put together a short list. The tl;dr is that @westonpace 's suggestion to simply promote the sum to decimal128(38, scale) or decimal256(76, scale) seems as good of a choice as any.

    Data

    In each case, I ran approximately the following query in the database's dialect (this query is for Clickhouse):

    with base_values as (
  select toDecimal128(1, 1) as foo
  union all
  select toDecimal128(1, 1) as foo
)
select toTypeName(sum(foo))
from base_values

    Clickhouse

    Like Arrow, the return type of sum on a column is the input type, but Clickhouse doesn't retain precision information in its decimal type. So this is basically what @westonpace suggests the solution for Arrow should be.

    Postgres

    The return type of sum appears to be an "unconstrained numeric" type. This was verified by running

    create table sum_columns as
select 
  sum(foo) as the_sum
from base_values;

select
  table_name,
  column_name,
  numeric_precision,
  numeric_scale
from information_schema.columns
where table_name in ('sum_columns');

    and noting that numeric_precision and numeric_scale are both NULL.

    MariaDB

    The return type of sum appears to be numeric(min(65, 22 + precision), scale). This was verified in the same way as postgres. N.B. 65 is the maximum width of a decimal in MariaDB

    duckdb

    The return type of sum appears to be decimal(38, s) no matter its internal representation.

  10. khwilson commented on Sep 19, 2024

    @khwilson
    khwilson
    on Sep 19, 2024
    Contributor

    @westonpace I have a PR that addresses this, but as I was working on it, I realized that Arrow also handles the product and mean in the same way that it does for sums. While the mean might make sense, the product definitely does not.

    ducksb handles this (for product and avg) by converting everything to doubles. Taking the same approach in Arrow would be quite a radical shift from the current behavior.

    Before investing in updating all the tests and whatnot, is there a good place to have a discussion on what these aggregate types should be?

  11. github-actions
    assigned
    khwilson
    on Sep 23, 2024
  12. zanmato1984
    added a commit that references this issue on Jul 3, 2025

    GH-35166: [C++][Compute] Increase precision of decimals in sum aggreg…

    bf56a95
  13. zanmato1984
    added this to the 21.0.0 milestone on Jul 3, 2025

  14. zanmato1984 commented on Jul 3, 2025

    @zanmato1984
    zanmato1984
    on Jul 3, 2025
    Contributor

    Issue resolved by pull request 44184
    #44184

  15. zanmato1984
    closed this as completedon Jul 3, 2025

  16. zanmato1984 commented on Jul 3, 2025

    @zanmato1984
    zanmato1984
    on Jul 3, 2025
    Contributor

    The merge script auto tagged MS 21.0.0. Not sure if this is correct (maint-21 not branched yet?).

    But I think we want to include it in v21.0.0 anyway. So @amoeba please help to make sure the inclusion happens. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Component: PythonType: bug

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions