Skip to content
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

Add FAQ on how ints are returned as strings in JavaScript #1928

Merged
merged 1 commit into from
Oct 3, 2017

Conversation

jseldess
Copy link
Contributor

@jseldess jseldess requested a review from benesch September 26, 2017 02:32
@cockroach-teamcity
Copy link
Member

This change is Reviewable

v1.0/sql-faqs.md Outdated
~~~ sql
> SELECT CAST (id AS string) FROM my_table;
~~~

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, there's no need to explicitly cast to a string. That happens automatically. Here's how I would explain it.

Why are my INT columns returned as strings in JavaScript?

In CockroachDB, all INTs are represented with 64 bits of precision, but JavaScript numbers only have 53 bits of precision. This means that large integers stored in CockroachDB are not exactly representable as JavaScript numbers. For example, JavaScript will round the integer 235191684988928001 to the nearest representable value, 235191684988928000. Notice that the last digit is different. This is particularly problematic when using the unique_rowid() function, since unique_rowid() nearly always returns integers that require more than 53 bits of precision to represent.

To avoid this loss of precision, Node's pg driver will, by default, return all CockroachDB INTs as strings.

// Schema: CREATE TABLE users (id INT DEFAULT unique_rowid(), name STRING);
pgClient.query('SELECT id FROM users WHERE name = 'Roach' LIMIT 1', function(err, res) {
  var idString = res.rows[0].id; 
  // idString == '235191684988928001'
  // typeof idString == 'string'
});

To perform another query using the value of idString, you can simply use idString directly, even where an INT type is expected. Either the Postgres driver or CockroachDB will automatically convert the string to a CockroachDB INT.

pgClient.query('UPDATE users SET name = 'Ms. Roach' WHERE id = $1', [idString], function(err, res) {
  // All should be well!
});

If you instead need to perform arithmetic on INTs in JavaScript, you will need to use a big integer library like Long.js. Do not use the built-in parseInt function.

parseInt(idString, 10) + 1; // WRONG: returns 235191684988928000
require('long').fromString(idString).add(1).toString(); // GOOD: returns '235191684988928002'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Thanks very much for the revision, @benesch!

@jseldess jseldess changed the title Add FAQ on using unique_rowid in JavaScript Add FAQ on how ints are returned as strings in JavaScript Sep 27, 2017
Copy link
Contributor

@benesch benesch left a comment

Choose a reason for hiding this comment

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

Thanks @jseldess! cc @justinj to get someone else's eyes on this JS

v1.0/sql-faqs.md Outdated

~~~ javascript
// Schema: CREATE TABLE users (id INT DEFAULT unique_rowid(), name STRING);
pgClient.query('SELECT id FROM users WHERE name = 'Roach' LIMIT 1', function(err, res) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shoot, this isn't valid JavaScript as written. Needs to use double quotes to allow single quotes within.

"SELECT id FROM users WHERE name = 'Roach' LIMIT 1"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Done.

v1.0/sql-faqs.md Outdated
To perform another query using the value of `idString`, you can simply use `idString` directly, even where an `INT` type is expected. Either the Postgres driver or CockroachDB will automatically convert the string to a CockroachDB `INT`.

~~~ javascript
pgClient.query('UPDATE users SET name = 'Ms. Roach' WHERE id = $1', [idString], function(err, res) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto here.

"UPDATE users SET name = 'Ms. Roach' WHERE id = $1"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

v1.0/sql-faqs.md Outdated
// Schema: CREATE TABLE users (id INT DEFAULT unique_rowid(), name STRING);
pgClient.query("SELECT id FROM users WHERE name = 'Roach' LIMIT 1", function(err, res) {
var idString = res.rows[0].id;
// idString == '235191684988928001'
Copy link
Contributor

Choose a reason for hiding this comment

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

doesn't really matter since it's a comment, but my javascript head alarm goes off whenever it sees ==, since === should generally be used instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

v1.0/sql-faqs.md Outdated
});
~~~

To perform another query using the value of `idString`, you can simply use `idString` directly, even where an `INT` type is expected. Either the Postgres driver or CockroachDB will automatically convert the string to a CockroachDB `INT`.
Copy link
Contributor

Choose a reason for hiding this comment

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

what is meant by "Either the Postgres driver or CockroachDB"? shouldn't it always be Cockroach doing this conversion? or does the driver somehow check if the string is within the int bounds?

Copy link
Contributor

@benesch benesch Sep 27, 2017

Choose a reason for hiding this comment

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

Node's pg driver does its own type conversion provided you use prepared queries. So in a query like

query("UPDATE foo SET int_col = $1", ["1234"])

the pg driver sees an integer type hint on $1 and will send the integer 1234 over the wire, instead of the string "1234".

The reason we needed string -> number literal coercion in CockroachDB itself was to cope with Sequelize, which eschews prepared queries in favor of string mashing, e.g.:

query("UPDATE foo SET int_col = '" + escape("1234") + "'")

That's too detailed an explanation, of course, and I don't think we want a string mashing example in our docs. Too easy for people to think that we're encouraging string mashing.

Perhaps we can replace the sentence with "The string will automatically be coerced into a CockroachDB INT", and leave out mentioning exactly which layer is doing the conversion?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@jseldess jseldess merged commit 926c705 into master Oct 3, 2017
@jseldess jseldess deleted the javascript-gotcha branch October 3, 2017 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants