Add a query flag to toggle the type of casting done to Decimals with no decimal places #60
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In #37 there was a change that updated the value of SUM to be Integer if what is being summed are all Integers. It used to be that it will _always_ return a Decimal type. When we pulled in that change, there were tests that failed because we were doing arthimitic operations on the result of queries that have SUM. Ruby will have different results if you divide intergers and decimals. In order to keep that behavior for us (GitHub) while allows other consumers to use SUM and expect the _right_ type we are introducing a new query flag that we can set in our adapter. This query flag will be what will determine if we should be casting the value of SUM to follow the _new_ behavior or roll back to the _old_ behavior
There was a problem hiding this comment.
The code looks good to me. I think this applies to more than just sum (I wasn't thinking about that as we were browsing sum queries earlier today), so we may need a more general name, but I don't have any good ideas at the moment. Maybe something will come to us over the weekend 😄
ah good to know, if that is the case we can generalize it and may use aggregate since that is what SQL buckets those functions in anyway. Maybe something like |
…CIMALS_TO_BIGDECIMALS
nickborromeo
force-pushed
the
add-query-flag-for-sum-casting
branch
from
f6908ac
to
a31cefb
Compare
nickborromeo
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
In #37 there was a change that updated the value of SUM to be Integer if what is being summed are all Integers. It used to be that it will always return a Decimal type.
When we pulled in that change, there were tests that failed because we were doing arthimitic operations on the result of queries that have SUM. Ruby will have different results if you divide intergers and decimals.
In order to keep that behavior for us (GitHub) while allowing other consumers to use SUM and expect the right type we are introducing a new query flag that we can set in our adapter. This query flag will be what will determine if we should be casting the value of SUM to follow the new behavior or roll back to the old behavior