Skip to content

graph, graphql, store: Coerce Int8 values to ints#5034

Closed
lutter wants to merge 1 commit intomasterfrom
lutter/int8-coerce
Closed

graph, graphql, store: Coerce Int8 values to ints#5034
lutter wants to merge 1 commit intomasterfrom
lutter/int8-coerce

Conversation

@lutter
Copy link
Collaborator

@lutter lutter commented Nov 30, 2023

Previously, they were coerced to strings which ultimately leads to errors when lists of int8 are used in queries

@dotansimha Do you remember why Int8 was initially coerced into String in a lot of places? The whole topic of coercion is really confusing to me; part of it is because the Value of graphql-parser can't represent all the values we deal with internally (like bytes or int4 vs int8) and so you are forever trying to figure out if a string is really a string or whether it's a byte array. The other part is that coercion happens all over the place and it's really hard to draw a line between 'here we are dealing with values that have fairly random types and aren't checked yet' vs 'these values have been properly checked and express the proper type'. I wonder if we should expand the r::Value type to faithfully represent all value types, and take the conversion from q::Value to r::Value as the point where we go from unchecked to checked.

Previously, they were coerced to strings which ultimately leads to errors
when lists of int8 are used in queries
@dotansimha
Copy link
Contributor

dotansimha commented Dec 3, 2023

@lutter I think the max value of i64 is 9223372036854776000 and this number is too big to be represented in JavaScript runtimes, so we need to use BigInt values (that are de/serialized as String).
Also check wasm-bindgen/wasm-bindgen#35

@lutter lutter marked this pull request as draft December 5, 2023 20:34
@github-actions
Copy link

This pull request hasn't had any activity for the last 90 days. If there's no more activity over the course of the next 14 days, it will automatically be closed.

@github-actions github-actions bot added the Stale label Mar 21, 2024
@lutter
Copy link
Collaborator Author

lutter commented Apr 8, 2024

Closing this since it's not been active for a while. Feel free to reopen when you're ready to work on this again

@lutter lutter closed this Apr 8, 2024
@lutter lutter deleted the lutter/int8-coerce branch April 17, 2025 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants