Skip to content
This repository has been archived by the owner on Jan 2, 2023. It is now read-only.

Fixing encoding of decimals ending in .0 or 0.0 #115

Closed
wants to merge 3 commits into from
Closed

Fixing encoding of decimals ending in .0 or 0.0 #115

wants to merge 3 commits into from

Conversation

ahisbrook
Copy link
Contributor

Found an error where storing a BigDecimal ending in .0 or 00.0 would not store/retrieve correctly.
Traced this to the append_decimal method of CqlByteBuffer

Example:
Encoding a BigDecimal of 1042342234234.0 would result in a decoded decimal of -57169393542.0
Encoding a BigDecimal of 12000.0 was unable to be decoded

@iconara
Copy link
Owner

iconara commented Jul 3, 2014

Sorry, because you closed the first PR I assumed this was the same as #112, but it doesn't look like that.

@@ -2,7 +2,6 @@

require 'bigdecimal'


Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please avoid committing unrelated whitespace changes, it makes PRs so much harder to read. This is just a general comment, obviously a single line isn't a big deal.

@iconara
Copy link
Owner

iconara commented Jul 3, 2014

Thanks again for the PR and for finding this bug. Good catch. If you fix the small comments I added to the diff I'll merge this as soon as I get the time.

Do you need a new gem release soon?

@ahisbrook
Copy link
Contributor Author

Yes, I'll fix those. Thanks for the quick reply!

A new gem release would be great when you get a chance.

On Jul 3, 2014, at 1:50 PM, Theo Hultberg notifications@github.com wrote:

Thanks again for the PR and for finding this bug. Good catch. If you fix the small comments I added to the diff I'll merge this as soon as I get the time.

Do you need a new gem release soon?


Reply to this email directly or view it on GitHub.

@iconara
Copy link
Owner

iconara commented Jul 3, 2014

I can get one out tomorrow tonight or tomorrow morning.

@ahisbrook
Copy link
Contributor Author

Awesome. Tomorrow whenever is great. I fixed those issues, let me know if you have any other comments.

On Jul 3, 2014, at 1:55 PM, Theo Hultberg notifications@github.com wrote:

I can get one out tomorrow tonight or tomorrow morning.


Reply to this email directly or view it on GitHub.

@iconara
Copy link
Owner

iconara commented Jul 3, 2014

Looking at this code now I realize how much of a hack it is. Can you help me come up with a better way of doing this without going through #to_s -> #gsub -> #to_i? I've been looking at the BigDecimal docs for a while now and can't see anything obvious, but it feels like there's a way to use BigDecial#to_r.

I'll merge this when I can, regardless, but it would be great to have something better for the future.

@ahisbrook
Copy link
Contributor Author

It is pretty hacky. I'll look at the docs and try to come up with some alternate solutions.

On Jul 3, 2014, at 2:48 PM, Theo Hultberg notifications@github.com wrote:

Looking at this code now I realize how much of a hack it is. Can you help me come up with a better way of doing this without going through #to_s -> #gsub -> #to_i? I've been looking at the BigDecimal docs for a while now and can't see anything obvious, but it feels like there's a way to use BigDecial#to_r.

I'll merge this when I can, regardless, but it would be great to have something better for the future.


Reply to this email directly or view it on GitHub.

@iconara
Copy link
Owner

iconara commented Jul 4, 2014

Merged in 8d6790b. v2.0.2 is now on RubyGems.

@iconara iconara closed this Jul 4, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants