You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@age.apache.org by GitBox <gi...@apache.org> on 2022/06/23 04:49:35 UTC
[GitHub] [age] bravius opened a new issue, #236: Using pg-age Node.js driver, graphid values exceed JavaScript's Number.MAX_SAFE_INTEGER after 29 labels
bravius opened a new issue, #236:
URL: https://github.com/apache/age/issues/236
Ran into a problematic limitation of JavaScript when using the bundled pg-age Node.js driver, which appears to convert the 64-bit graphid to JavaScript's floating Number type.
According to the Mozilla JavaScript reference:
`The Number.MAX_SAFE_INTEGER constant represents the maximum safe integer in JavaScript (2^53 - 1).`
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER
From the source (graphid.h/graphid.c) it appears graphid is composed of a 48 bit entry id and a 16 bit label id:
```
ENTRY_ID_BITS = (32 + 16)
ENTRY_ID_MASK = 0x0000ffffffffffff
graphid = (((uint64)label_id) << ENTRY_ID_BITS) | (((uint64)entry_id) & ENTRY_ID_MASK)
```
Given that entry ids are scaled to 48 bits, and JavaScript integer precision is limited to 2^53-1, this essentially leaves just 5 bits for label ids, or a maximum of 32 labels. Any graphid exceeding JavaScript's maximum guaranteed integer precision would have to be considered unreliable.
Upon testing this, in fact, graphids already start exceeding Number.MAX_SAFE_INTEGER after creating just 30 labels.
Some thoughts on solving this for graphs with more than 29 labels:
1) Change ENTRY_ID_BITS and ENTRY_ID_MASK to 40 bits and rebuild. Would still leave room for over a trillion entries per label.
2) Have the pg-age Node.js driver parse graphids as strings, or possibly as BigInt, though using BigInt would not be serializable using the standard JSON parse / stringify.
Any thoughts on the best way to deal with this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@age.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [age] bravius closed issue #236: Using pg-age Node.js driver, graphid values exceed JavaScript's Number.MAX_SAFE_INTEGER after 29 labels
Posted by GitBox <gi...@apache.org>.
bravius closed issue #236: Using pg-age Node.js driver, graphid values exceed JavaScript's Number.MAX_SAFE_INTEGER after 29 labels
URL: https://github.com/apache/age/issues/236
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [age] jasperblues commented on issue #236: Using pg-age Node.js driver, graphid values exceed JavaScript's Number.MAX_SAFE_INTEGER after 29 labels
Posted by GitBox <gi...@apache.org>.
jasperblues commented on issue #236:
URL: https://github.com/apache/age/issues/236#issuecomment-1164814160
Since changing the scaling of entry Ids is non-trival major change, and using strings would impact CYPHER compatibility, it looks like using BigInt is the best solution, but:
* When is serialization an issue?
* What envs might need a polyfil?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [age] jasperblues commented on issue #236: Using pg-age Node.js driver, graphid values exceed JavaScript's Number.MAX_SAFE_INTEGER after 29 labels
Posted by GitBox <gi...@apache.org>.
jasperblues commented on issue #236:
URL: https://github.com/apache/age/issues/236#issuecomment-1165014412
So you will transform to/from annotated strings. Nice solution 👍
This solves `driver << database` side. I guess later there will be similar issues on the `database >> driver` side when querying for large numbers, eg `MATCH (v:VIP) where v.bankBalance > 18014398509481986`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [age] bravius commented on issue #236: Using pg-age Node.js driver, graphid values exceed JavaScript's Number.MAX_SAFE_INTEGER after 29 labels
Posted by GitBox <gi...@apache.org>.
bravius commented on issue #236:
URL: https://github.com/apache/age/issues/236#issuecomment-1164686196
Posting the results of some experiments, for when others run into the same issue.
First, I tried a quick experiment to change the scaling of entry ids to 40 bits instead of 48. No luck, server just crashes on any graph operation (after complete re-initialization of AGE extension via DROP/CREATE extension), so I think changing the scaling of the graphid type is more involved than just altering graphid.h definitions and recompiling. I don't think the solution is ideal anyway, since the issue is with the JavaScript client, not AGE.
Second, I tried having the node-pg-age driver convert integers to BigInt instead of Number. This works great, except any de/serialization code, e.g. json, has to be re-written to handle BigInt.
To change the node driver to use BigInt instead of Number involves changing just one line in CustomAgTypeListener.ts:
```
exitIntegerValue (ctx: IntegerValueContext): void {
- const value = parseInt(ctx.text)
+ const value = BigInt(ctx.text)
if (!this.pushIfArray(value)) {
this.lastValue = value
}
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [age] bravius commented on issue #236: Using pg-age Node.js driver, graphid values exceed JavaScript's Number.MAX_SAFE_INTEGER after 29 labels
Posted by GitBox <gi...@apache.org>.
bravius commented on issue #236:
URL: https://github.com/apache/age/issues/236#issuecomment-1164973950
@jasperblues
_"When is serialization an issue?"_
I believe this demonstrates the problem.
In pgsql generate two graphids with a label id of 64 and entry ids of 1 and 2:
```
SELECT _graphid(64,1), _graphid(64,2); // 18014398509481985, 18014398509481986
```
Then in JavaScript compare the two values:
```
console.log( 18014398509481985 == 18014398509481986 ); // true
```
Gasp... JavaScript thinks the two graphids are equal. This is because JavaScript Number type starts losing precision after 2^53-1.
Now stringify/parse the two values and compare:
```
let array = JSON.parse(JSON.stringify([18014398509481985, 18014398509481986]));
console.log(array[0] == array[1]); // true
```
Same issue, JavaScript thinks the two distinct graphids are equal.
BigInt solves the problem of precision loss, but the standard JSON.stringify/JSON.parse are not compatible with BigInt and require the use of custom replacer/reviver functions on both the client and server.
_"What envs might need a polyfil?"_
About 90% of browsers are supported.
https://caniuse.com/bigint
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [age] bravius commented on issue #236: Using pg-age Node.js driver, graphid values exceed JavaScript's Number.MAX_SAFE_INTEGER after 29 labels
Posted by GitBox <gi...@apache.org>.
bravius commented on issue #236:
URL: https://github.com/apache/age/issues/236#issuecomment-1165005781
@jasperblues
_I meant at what point(s) in the pipleline from (driver)-[:EXECUTES_STATEMENTS)->(apachAge) are we dependent on parsing/serialization, and thus need to plug in custom replacer and reviver ?_
Pretty much any case that depends upon communicating JSON data over the wire, such as between client and server.
I've solved the serialization issue using these custom replacer/reviver functions for JSON.parse and JSON.stringify.
```
const NULL = 'n';
const BIGINT = 'i';
const STRING = 's';
const BOOLEAN = 'b';
const NUMBER = 'f';
/**
* BigInt compatible JSON.stringify replacer function
*/
export function json_replacer(key, value) {
const type = typeof value;
if (type === "string") {
return `${STRING}:${value}`;
} else if (type === "number") {
return `${NUMBER}:${value}`;
} else if (type === "bigint") {
return `${BIGINT}:${value}`;
} else if (type === "boolean") {
return `${BOOLEAN}:${value}`;
} else if (value === null) {
return `${NULL}:${value}`;
} else if (type === "object") {
return value;
} else {
throw new TypeError();
}
}
/**
* BitInt compatible JSON.parse reviver function
*/
export function json_reviver(key, value) {
const type = typeof value;
if (type === "string") {
const t = String(value).substring(0, 1);
const v = String(value).substring(2);
if (t === STRING) {
return String(v);
} else if (t === NUMBER) {
return Number(v);
} else if (t === BIGINT) {
return BigInt(v);
} else if (t === BOOLEAN) {
return Boolean(v);
} else if (t === NULL) {
return null;
} else {
throw new TypeError();
}
} else if (type === "object") {
return value;
} else {
throw new TypeError();
}
};
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [age] jasperblues commented on issue #236: Using pg-age Node.js driver, graphid values exceed JavaScript's Number.MAX_SAFE_INTEGER after 29 labels
Posted by GitBox <gi...@apache.org>.
jasperblues commented on issue #236:
URL: https://github.com/apache/age/issues/236#issuecomment-1164983384
Oh sorry, I didn't pose my question clearly. The part about what happens with precision is clear, although the additional examples are great.
I meant at what point(s) in the pipleline from `(driver)-[:EXECUTES_STATEMENTS)->(apachAge)` are we dependent on parsing/seralization, and thus need to plug in custom replacer and reviver ? Is it a manageable amount? Maybe I could help?
> About 90% of browsers are supported.
I think that's reasonable. This number will only go up. For myself, I'll be incorporating the driver into [drivine](drivine.org), which is a NodeJS/TypeScript lib providing a higher level of abstraction - allowing to switch between different graph databases seamlessly. (For age, it can be used for testing compatibility with a reference set of cases, and comparative performance, etc).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org