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

Auto convert to JS `BigInt` when numbers are not safe integer

Open unknown1337 opened this issue 2 years ago • 6 comments

Bug report

Describe the bug

A clear and concise description of what the bug is. type overflow bigint, seems that the client lib cannot handle bigint which causes some unexpected behaviour (overflow?)

To Reproduce

Steps to reproduce the behavior, please provide code snippets or a repository:

  1. SQL create table test ( myfield int8 ); insert into test (myfield) values (23849208278400758)
  2. JS
import { createClient } from '@supabase/supabase-js'
const supabaseUrl: string = process.env.SUPABASE_URL ?? ''
const supabaseKey: string = process.env.SUPABASE_KEY ?? '' 
supabase = createClient(supabaseUrl, supabaseKey)
  console.log(
    await supabase
      .from('test')
      .select('myfield')
      .eq('myfield', '23849208278400758') // quotes->string as the number wont fit in 32bit mem);
  ); 

Expected behavior

expected result to be 23849208278400758 or "23849208278400758" (automatic type conversion to prevent overflows observed result 23849208278400760

Screenshots

image

System information

  • OS: [Windows]
  • Browser (if applies chrome
  • Version of supabase-js: 1.35.6"

Additional context

note: causes difficult to find issues note: root cause of issue unknown (dep?). quasar v2

unknown1337 avatar Sep 05 '22 12:09 unknown1337

See also https://github.com/supabase/postgrest-js/issues/273:

This is because numbers in JS are float64, so if your number is more than Number.MAX_SAFE_INTEGER === 9007199254740991 it won't be represented correctly. An alternative is to cast it to text: supabase.from(table_name).select('id, num::text') and manually convert them to BigInt.

soedirgo avatar Sep 06 '22 06:09 soedirgo

Repurposing this issue to do auto-conversion to JS BigInt when numbers are !Number.isSafeInteger(x)

soedirgo avatar Sep 27 '22 08:09 soedirgo

Postgrest is returning the number correctly in the fetch response:

[{"id":349506210296369153}]

But it loses precision when the response.text() goes through JSON.parse in this library:

[{ id: 349506210296369150 }]

This is the offending line: https://github.com/supabase/postgrest-js/blob/878034b686bcde9abd8fe9be34e393ef90c22f64/src/PostgrestBuilder.ts#L95

Would you be open to adopting a different JSON parser that can transform ints bigger than MAX_SAFE_INTEGER or smaller than MIN_SAFE_INTEGER into BigInts or strings? For example, json-bigint?

It could be an opt-in feature per-request with a flag on PostgrestBuilder, though it seems safer out the box for the default behaviour to be to return correct values rather than truncated (wrong!) ones.

(Note, eventually this should become possible in the web platform itself without having to use a whole external JSON parser, instead using just reviver! see https://github.com/tc39/proposal-json-parse-with-source)

More conversation here: https://github.com/PostgREST/postgrest/issues/498

I imagine the supabase gen types command would need to be updated to change the generated TypeScript types to number | string or number | BigInt also.

Hope that helps!

bensalilijames avatar May 16 '23 18:05 bensalilijames

Would you be open to adopting a different JSON parser that can transform ints bigger than MAX_SAFE_INTEGER or smaller than MIN_SAFE_INTEGER into BigInts or strings?

Yes, but we consider this a breaking change so this will have to wait until the next major version.

The TC39 proposal looks interesting - thanks for sharing!

soedirgo avatar May 17 '23 08:05 soedirgo

Ideally you would decide if you want to get bigint back always or number | bigint if you are going to offer both.

bradennapier avatar May 21 '23 17:05 bradennapier

Just want to point out that if you cast a bigint to text as recommended in https://github.com/supabase/postgrest-js/issues/273#issuecomment-1142480261 you end up breaking TS as seen in https://github.com/supabase/postgrest-js/issues/370

afk-mario avatar Oct 25 '23 00:10 afk-mario