zanmato1984 commented on code in PR #44184: URL: https://github.com/apache/arrow/pull/44184#discussion_r2181923252
########## docs/source/cpp/compute.rst: ########## @@ -276,7 +275,9 @@ the input to a single output value. is always -1, regardless of whether there are nulls in the input. * \(5) For decimal inputs, the resulting decimal will have the same - precision and scale. The result is rounded away from zero. + scale and the precision for its width. For instance, an array Review Comment: I believe this is a typo. It was initially modified in [`7c69607` (#44184)](https://github.com/apache/arrow/pull/44184/commits/7c696075bf83e34e7778394b9fadb5445d12bb3f) the same way as bullet `(9)`. Later on we agreed to NOT change `mean` behavior in this PR, thus it was subsequently reverted in [`41fafbf` (#44184)](https://github.com/apache/arrow/pull/44184/commits/41fafbf98e57dfb045d5e5d1d02f454cd541fc42#diff-ce5b94577014735990903d3d03bd4ea4b8c8e6d32f5227592e60b7dd6a912d59) however it was not fully done. Let's revert it back to the very baseline. ########## docs/source/cpp/compute.rst: ########## @@ -294,8 +295,10 @@ the input to a single output value. the values to be pivoted. The output is a Struct with one field for each key in :member:`PivotOptions::key_names`. -* \(9) Output is Int64, UInt64, Float64, or Decimal128/256, depending on the - input type. +* \(9) Output is Int64, UInt64, Float64, or Decimal128/256, depending + on the input type. For sums of decimals, the precision is increased to the maximum + precision for the type's width. For instance, an array of + ``decimal128(3, 2)`` will return a ``decimal128(38, 2)`` scalar. Review Comment: This actually made me notice another nit: bullet `(9)` applies to both `product` and `sum`, thus here we have to differentiate `sum` from `product` for the precision promotion. Do you think it's better to move the promotion description (for `sum`) to an individual bullet, say `(13)`, and append `(13)` to the "Notes" for `sum`, like: `sum, ..., (9) (13)`. ########## docs/source/cpp/compute.rst: ########## @@ -294,8 +295,10 @@ the input to a single output value. the values to be pivoted. The output is a Struct with one field for each key in :member:`PivotOptions::key_names`. -* \(9) Output is Int64, UInt64, Float64, or Decimal128/256, depending on the - input type. +* \(9) Output is Int64, UInt64, Float64, or Decimal128/256, depending + on the input type. For sums of decimals, the precision is increased to the maximum + precision for the type's width. For instance, an array of + ``decimal128(3, 2)`` will return a ``decimal128(38, 2)`` scalar. Review Comment: I'm not sure what you mean. The example shows `decimal128(3, 2)` promoted to `decimal128(38, 2)`. The scale remains though, if that is what you are confused about? -- 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]
