Conversation
There was a problem hiding this comment.
Pull request overview
Adds support for passing large integer literals through to custom scalar unmarshaling by deserializing integer literals as int64 when they don’t fit in 32-bit, while preserving existing int32 behavior for 32-bit-range values.
Changes:
- Update integer literal deserialization to parse as 64-bit and return
int64when outside 32-bit range. - Extend input coercion for
int32targets to also acceptint64values. - Add a regression test covering custom
Int64scalar input alongside built-inIntrange validation behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
ast/value.go | Parses integer literals with 64-bit precision and returns int64 for out-of-32-bit-range values. |
internal/exec/packer/packer.go | Adds int64 → int32 coercion for packing into int32 Go targets. |
issue_305_test.go | Introduces test coverage for large integer literal behavior with a custom Int64 scalar and built-in Int validation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| value, ok := input.(int64) | ||
| if !ok { | ||
| return fmt.Errorf("Int64 expects int64 got %T", input) | ||
| } | ||
| u.Value = value | ||
| return nil |
There was a problem hiding this comment.
The test scalar’s UnmarshalGraphQL only accepts int64. With the current deserialization behavior, integer literals within 32-bit range are still provided as int32, so this scalar would reject valid Int64 inputs like custom(hash: 123). Consider accepting both int32 and int64 (casting int32 up) so the test scalar matches real-world usage and doesn’t encode an overly strict/unintuitive contract.
| value, ok := input.(int64) | |
| if !ok { | |
| return fmt.Errorf("Int64 expects int64 got %T", input) | |
| } | |
| u.Value = value | |
| return nil | |
| switch v := input.(type) { | |
| case int64: | |
| u.Value = v | |
| return nil | |
| case int32: | |
| u.Value = int64(v) | |
| return nil | |
| default: | |
| return fmt.Errorf("Int64 expects int64 got %T", input) | |
| } |
| case int64: | ||
| if input < math.MinInt32 || input > math.MaxInt32 { | ||
| return nil, fmt.Errorf("not a 32-bit integer") | ||
| } | ||
| return int32(input), nil |
There was a problem hiding this comment.
UnmarshalInput now explicitly accepts int64 for reflect.Int32, but integer literals can also deserialize to int64 when used in a GraphQL Float position (GraphQL allows Int literals for Float). The reflect.Float64 branch currently only coerces int32/int, so a large integer literal (or a variable value) arriving as int64 will fail with "incompatible type". Please add int64 handling in the reflect.Float64 coercion path (and consider tests for Float arguments with large integer literals).
Allow int64 custom scalars.
Closes #305