edgedb-js icon indicating copy to clipboard operation
edgedb-js copied to clipboard

int64 incorrectly parsed

Open Tricked-dev opened this issue 2 years ago • 2 comments

Error

error: Uncaught (in promise) Error: a number was expected, got "336465356304678913"
      throw new Error(`a number was expected, got "${object}"`);
            ^
    at Int64Codec.encode (file:///home/tricked/coding/aethor/vendor/deno.land/x/[email protected]/_src/codecs/numbers.ts:26:13)
    at ObjectCodec._encodeNamedArgs (file:///home/tricked/coding/aethor/vendor/deno.land/x/[email protected]/_src/codecs/object.ts:156:15)

Reproduce

module default {
  type User {
      required property user_id -> int64 {
        constraint exclusive;
   }
  }
}
import * as edgedb from "https://deno.land/x/edgedb/mod.ts";

const client = edgedb.createClient();
console.log(await client.querySingle(`
insert User {
  user_id := <int64>$id,
};
`, { id: "336465356304678913" }));
await client.close();

Solution

to "safely" insert int64 you have to use a number instead of a bigint this can cause rounding issues.

https://github.com/edgedb/edgedb-js/blob/master/src/codecs/numbers.ts#L23-L45=

Tricked-dev avatar Jun 19 '22 20:06 Tricked-dev

got helped in the discord and found this to be the solutions

client.query(`
insert User {
  user_id := <int64><str>$id
}
`, { id: "336465356304678913" })

Tricked-dev avatar Jun 20 '22 05:06 Tricked-dev

actually i feel like this should be reopened selected int64's are turned into numbers and loose precision so using bigints which is pretty easy to do or using strings sounds like a much better solution

https://github.com/edgedb/edgedb-js/blob/f4a166408d9365f2a1467aaac321ca3c3f456c99/src/primitives/buffer.ts#L852-L868=

Tricked-dev avatar Jul 06 '22 10:07 Tricked-dev

Agreed that we need some way of allowing large integers to be inserted into int64 without requiring consumers to use the std::bigint type to address the difference between JavaScript's Number type and int64.

I think accepting number | string | BigInt is a fine solution since we can throw a helpful runtime error for the string case. We could maybe provide a nice branded type for NumericString that at least can provide a bit of speed bump to just passing "hi" to the function.

scotttrinh avatar May 02 '23 16:05 scotttrinh