[Breaking] fix(GRAPHQL): fix input coercing for integers and make them GraphQL spec complaint. #7584
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
disallow string --> Int32 variable coercing
JatinDev543
requested review from
manishrjain,
pawanrawal and
vvbalaji-dgraph
as code owners
github-actions
bot
added
the
area/graphql
abhimanyusinghgaur
approved these changes
Mar 16, 2021
There was a problem hiding this comment.
In the PR title and description, we should correct some typos:
- Graphql -> GraphQL
- specs -> spec
- apart from
Intto be -> apart from integer to be (basically,Intanythwere -> integer) - variable of type
Int32-> variable of typeInt(basically,Int32anywhere -> Int)
Reviewed 4 of 4 files at r1.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @manishrjain, @pawanrawal, and @vvbalaji-dgraph)
JatinDev543
changed the title
aman-bansal
pushed a commit
that referenced
this pull request
Mar 25, 2021
GraphQL spec doesn't allow any value apart from integer to be passed to a variable of type Int. So we are disallowing strings to be passed as Int variables. Int64 is our own scalar data type, so we continue to accept string and Integer values for it. This PR allows passing strings to Int64 values when given directly in query/mutations and disallows string to be passed to Int in variables. So we will have below final behavior: `Int32` will accept only `Integer` values, and otherwise will give a coercion error. `Int64` will accept `Integer` as well as `String` value and will return an error if any other value. And if we can only pass Integer in a string like "1234" otherwise it will give a coercion error. Int coercion rules in GraphQL spec: https://spec.graphql.org/June2018/#sec-Int
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
GraphQL spec doesn't allow any value apart from
integerto be passed to a variable of typeInt. So we are disallowing strings to be passed asIntvariables. Int64 is our own scalar data type, so we continue to acceptstringandIntegervalues for it.This PR allows passing
stringstoInt64values when given directly in query/mutations and disallowsstringto be passed toIntin variables.So we will have below final behavior:Int coercion rules in GraphQL spec:https://spec.graphql.org/June2018/#sec-Int
This change is