-
Notifications
You must be signed in to change notification settings - Fork 4k
ARROW-13966: [C++] Support decimals in comparisons #11129
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this came up in a different PR or discussion somewhere, but this makes it so that decimal-integer addition works 'properly' now if the decimal precision wasn't big enough to hold the integer (now it will promote both to a larger decimal type).
westonpace
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. One small nit on docs.
docs/source/cpp/compute.rst
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is Decimal not part of Numeric? I don't see any reason to call it out special here given that we don't do so for add/multiply/divide/subtract.
Although, to be fair, we don't currently support decimal for the rest of the math operations (trig, pow/ln, etc.) and that's more than just a casting issue so it may be sometime before we do offer support. So we might want to call out that fact.
Either way, I think we should keep this page consistent. Either decimal is part of "Numeric" everywhere or nowhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair, "Numeric" is defined up top as often including decimal. The blurb about promotion here should suffice, I think; I also added a blurb about decimal promotion to the arithmetic section.
be64fa6 to
16e7237
Compare
Closes apache#11129 from lidavidm/arrow-13966 Authored-by: David Li <li.davidm96@gmail.com> Signed-off-by: David Li <li.davidm96@gmail.com>
No description provided.