sql: unique_rowid() returns and javascript

[petermattis]

unique_rowid() returns values that cannot be represented exactly by a number in Javascript. For example, 235191684988928001 is a recent value returned by unique_rowid() but in Javascript that is represented as 235191684988928000 (notice the last digit is different). The precision loss can then foul up Javascript application which temporarily stores the ID and then queries using it.

Reported by @mattiasmak. This is arguably a bug in the NodeJS driver.

[tamird]

Is there any sample code to go with this? Specifically, I'm curious about how the return type gets reported to the Javascript application, and which Node driver is being used.

…

On Fri, May 5, 2017 at 5:20 PM, Peter Mattis ***@***.***> wrote: unique_rowid() returns values that cannot be represented exactly by a number in Javascript. For example, 235191684988928001 is a recent value returned by unique_rowid() but in Javascript that is represented as 235191684988928000 (notice the last digit is different). The precision loss can then foul up Javascript application which temporarily stores the ID and the queries using it. Reported by @mattiasmak <https://github.com/mattiasmak>. This is arguably a bug in the NodeJS driver. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#15721>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABdsPL7QlP-dJGKmNPn_2hrHjPcBhFJZks5r25KagaJpZM4NSb6a> .

[petermattis]

node-pg was mentioned. @mattiasmak do you have a snippet of code you can share with us demonstrating this problem?

[jseldess]

@petermattis, is there a workaround we should suggest here? Don't query based on a returned unique_rowid() value in a Javascript application?

[petermattis]

The workaround would be to cast the integer to a string when using a language that doesn't support 64-bit integer types. For example: SELECT CAST(id AS string) FROM my_table.

[benesch]

Since v3, which is ancient at this point, the node-postgres driver (the de facto standard driver for Node) will return bigints as strings unless you explicitly configure it otherwise (source).

We should probably document this necessity. My guess is you've somehow changed this default, @mattiasmak?

[tamird]

Heh, v3 only changed the handling of arrays of bigints. Solitary bigints were correctly handled since v2, released 2013-06-18 (see brianc/node-postgres#353).

…

On Mon, May 8, 2017 at 10:29 AM, Nikhil Benesch ***@***.***> wrote: Since v3, which is ancient at this point, the node-postgres driver (the de facto standard driver for Node) will return bigints as strings unless you explicitly configure it otherwise (source <https://github.com/brianc/node-postgres/blob/master/CHANGELOG.md#v300>) We should probably document this necessity. My guess is you've somehow changed this default, @mattiasmak <https://github.com/mattiasmak>? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#15721 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABdsPB1BlFVmEBYxYyvU5aV6lHH-nDQFks5r3ybkgaJpZM4NSb6a> .

[jseldess]

I don't understand the implications of the last two comments. Here's how I'm explaining the limitation currently. Is it still accurate? If not, how should I change it?

The unique_rowid() function returns values that cannot be represented exactly by numbers in Javascript. For example, a 235191684988928001 value returned by unique_rowid() would get represented as 235191684988928000 in Javascript. Notice that the last digit is different. In such cases, if the Javascript application temporarily stores the value 235191684988928001 and queries using it, it won't match the value that actually got stored.

As a workaround when using JavaScript or another language that doesn't support 64-bit integer types, cast the integer to a string server-side before inspecting or using it, for example:

SELECT CAST (id AS string) FROM my_table;

[tamird]

I believe we don't need to document any workarounds for this; this doesn't appear to be a CockroachDB issue, and it doesn't even appear to be a node-postgres issue. We're still waiting for some code to reproduce this problem, so we can decide if it's something we need to address.

…

On Mon, May 8, 2017 at 11:10 AM, jseldess ***@***.***> wrote: I don't understand the implications of the last two comments. Here's how I'm explaining the limitation currently. Is it still accurate? If not, how should I change it? The unique_rowid() function <http://functions-and-operators.html> returns values that cannot be represented exactly by numbers in Javascript. For example, a 235191684988928001 value returned by unique_rowid() would get represented as 235191684988928000 in Javascript. Notice that the last digit is different. In such cases, if the Javascript application temporarily stores the value 235191684988928001 and queries using it, it won't match the value that actually got stored. As a workaround when using JavaScript or another language that doesn't support 64-bit integer types, cast the integer to a string server-side before inspecting or using it, for example: SELECT CAST (id AS string) FROM my_table; — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#15721 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABdsPJ8fDtuFDUmRxDUhcW3lXvnUNgQJks5r3zB2gaJpZM4NSb6a> .

[tamird]

Ah, I see we already have this on known-limitations. We should remove it. It's also the case that this kind of problem will affect any 64-bit integer returned from the database to a Javascript application (but again, only with prehistoric versions of node-pg or with exotic configurations of node-pg), so if we do decide to document this, we should not confine the note to unique_rowid.

[jseldess]

Thanks, @tamird. Asked @sploiselle to remove it as part of cockroachdb/docs#1400.

[benesch]

Wait wait! I think this is worth documenting. I'll wager there are many node applications that choose to parse int8s as ints instead of strings. (See here for one such discussion.)

I do think we should expand the documentation to include all 64-bit integers, but calling out unique_rowid explicitly isn't so crazy. Auto-incrementing primary keys are exactly where most users will run into this first, if they run into it, because parsing a Postgres auto-incrementing int8 column as a number will work until you hit your 100-billionth insert or whatever, whereas it'll fall over basically immediately with Cockroach.

[jseldess]

Perhaps this just shouldn't be documented as a known limitation, which implies a limitation on the part of CockroachDB, but rather somewhere else, maybe in the SQL-specific FAQs section?

[benesch]

That's a good point! It's not a known limitation; it's a "gotcha" for folks who were already bending the rules.

[benesch]

It just occurred to me that our examples-orm repository literally gets this wrong at this very moment. https://reviewable.io/reviews/cockroachdb/examples-orms/20#-Kfiyp_3rHRaWNdsq433

We should document this more loudly, if anything.

[tamird]

Ouch.

…

On Fri, May 12, 2017 at 6:11 PM, Nikhil Benesch ***@***.***> wrote: It just occurred to me that our examples-orm repository literally gets this wrong at this very moment. https://reviewable.io/reviews/ cockroachdb/examples-orms/20#-Kfiyp_3rHRaWNdsq433 We should document this more loudly, if anything. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#15721 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABdsPDFar4b6Pcoz-TuySACvNZ_EoJ5Yks5r5NkegaJpZM4NSb6a> .

[petermattis]

@jseldess Have we done anything towards documenting this?

[petermattis]

@jseldess Is this done?

@benesch Was examples-orms fixed?

[jseldess]

It looks like we removed this from known limitations, but we never put it back anywhere else.

I've just opened this docs PR to add it back as a SQL FAQ: cockroachdb/docs#1928.

[jseldess]

Docs PR merged: cockroachdb/docs#1928

@petermattis, ok to close this?

[petermattis]

Looks great.