-
Notifications
You must be signed in to change notification settings - Fork 707
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Precision-Loss Decimal Arithmetic #4664
Labels
Comments
This was referenced Aug 7, 2023
Merged
Having spent a significant amount of time working on this I'm inclined to not move forward with this.
TLDR the current decimal support, whilst likely not perfect, covers most bases, and is at least as feature-full as many commercial DBMSs, and I therefore struggle to justify spending more time working on it |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Is your feature request related to a problem or challenge? Please describe what you are trying to do.
Currently decimal arithmetic may return an overflow error if an arithmetic operation produces an output that exceeds the maximum precision of the storage type.
Describe the solution you'd like
Spark and similar systems instead truncate the precision of the output, if the precision would exceed the maximum of the storage type - https://github.com/apache/arrow/blob/36ddbb531cac9b9e512dfa3776d1d64db588209f/java/gandiva/src/main/java/org/apache/arrow/gandiva/evaluator/DecimalTypeUtil.java#L83
To achieve this the arithmetic kernels would need to be updated to detect this case, and instead perform arithmetic on double-width decimal primitives, before then truncating this down to an appropriate scale/precision.
Describe alternatives you've considered
We could continue to return an error, but this is likely to be surprising for users.
Additional context
#4640 proposes increasing the decimal output scale, which in turn would make this error case more likely.
The fundamental primitive necessary for this is, N-digit division, which will likely be implemented as part of #4663
The text was updated successfully, but these errors were encountered: