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.
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
Support relational operators for decimal type #1173
Support relational operators for decimal type #1173
Changes from 10 commits
d2db40f
6e3228c
f5fb54b
863ed80
baa9fe1
46933a0
8df24d7
817cfe9
3267d7f
63a16c0
65f15d9
ed505fc
d63eb80
7f9ca91
b2069cc
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
nit: This is going to cause problems for add/etc.
We will fall have to fall back to the CPU in all of those cases. I am OK with this for now, but I think we need to work on begin sure that we select the proper set of precision and scale for a given test.
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.
Agreed. Selecting correct precision and scale for each test would be good while adding other tests.
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.
Even -1 and 1 do not work all of the time. a precision of 1 and scale of -3 would be an example where this would not work. It can only hold multiples of 100 up to 900.
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.
Sorry, I don't understand above statement. precision of 1 and scale of -3 is
1000
and I am not getting why it can hold multiples of 100. Could you please explain. I tried setting precision and scale as mentioned above and it passed. So I am missing something.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.
The details of -2 vs -3 scale don't matter, what matters is -1 becomes 0 for any scale that is negative. For a Decimal where the precision is larger than the scale this actually becomes an error'
We cannot blindly add in -1 as a special case.
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.
Thanks for the examples. I have removed
-1
and1
from special cases. Added another generator where scale and precision are same.