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

Improved support (in both directions) for numbers, dates, and booleans #22

Merged
merged 3 commits into from May 31, 2012

Conversation

Projects
None yet
3 participants
@dfoody

dfoody commented Feb 22, 2012

This commit makes two primary changes:

  • Properly handle CQL binding for data types other than string and Buffer
  • Support better precision decoding for numbers when not using BigInteger

For the first of these, with a CQL statement like update a set b=? where key='c', parameters were always quoted. So, for numeric columns, passing 12 for the parameter would result in the invalid CQL update a set b='12' where key='c'. These changes detect the data type and, if it is boolean, number, or date, properly bind it without quotes.

In the decoder, when not using BigInteger, the decoding was limited to 32 bit numbers. However, javascript can represent integral numbers with 52 bits of precision (big enough to handle timestamps through the year 10k with no loss of precision). This commit updates the bytesToNum logic so that it can properly support numbers with no loss up to 52 bits of precision. Above 52 bits the number still works but gets approximated. As a result of making 52-bit numbers work without loss, this also allowed the decoding of Date data types to be streamlined. Boolean data types are now also properly decoded.

@gdusbabek

This comment has been minimized.

Show comment
Hide comment
@gdusbabek

gdusbabek Feb 23, 2012

Contributor

ACK. I'll try to look at this this week.

Contributor

gdusbabek commented Feb 23, 2012

ACK. I'll try to look at this this week.

Show outdated Hide outdated lib/driver.js Outdated
Show outdated Hide outdated lib/driver.js Outdated
@Kami

This comment has been minimized.

Show comment
Hide comment
@Kami

Kami Feb 23, 2012

Contributor

Besides the stuff I have pointed out above the changes look OK, but you definitely need to add tests for the new functionality (including tests for the edge cases).

Contributor

Kami commented Feb 23, 2012

Besides the stuff I have pointed out above the changes look OK, but you definitely need to add tests for the new functionality (including tests for the edge cases).

Dan Foody
- Follow coding style in driver.js encodeParam
- Fixed handling of negative numbers in decoder.js bytesToNum
- Added tests for encoding and deciding dates, boolean, and numbers (including numbers > 32 bits)
@dfoody

This comment has been minimized.

Show comment
Hide comment
@dfoody

dfoody Feb 26, 2012

Thanks Kami - I've added tests for all the changes, and addressed all the issues you pointed out.

dfoody commented Feb 26, 2012

Thanks Kami - I've added tests for all the changes, and addressed all the issues you pointed out.

@Kami

This comment has been minimized.

Show comment
Hide comment
@Kami

Kami May 16, 2012

Contributor

@dfoody Changes and tests look good. Can you please fix the indentation issues - it looks like you are using tabs instead of two spaces for a tab.

Let me know when you are done so I can merge it. Thanks.

Contributor

Kami commented May 16, 2012

@dfoody Changes and tests look good. Can you please fix the indentation issues - it looks like you are using tabs instead of two spaces for a tab.

Let me know when you are done so I can merge it. Thanks.

Dan Foody
Fixed indentation issues
As per feedback from @Kami
@dfoody

This comment has been minimized.

Show comment
Hide comment
@dfoody

dfoody May 16, 2012

@Kami I've fixed the indentation issues in the commit I just did. Thanks.

dfoody commented May 16, 2012

@Kami I've fixed the indentation issues in the commit I just did. Thanks.

Kami added a commit that referenced this pull request May 31, 2012

Merge pull request #22 from dfoody/datatypes
Improved support (in both directions) for numbers, dates, and booleans

@Kami Kami merged commit 6bb5f8d into racker:master May 31, 2012

@Kami

This comment has been minimized.

Show comment
Hide comment
@Kami

Kami May 31, 2012

Contributor

I just merged the branch. Thanks and sorry for the delay.

Contributor

Kami commented May 31, 2012

I just merged the branch. Thanks and sorry for the delay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment