-
Notifications
You must be signed in to change notification settings - Fork 842
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for null literals #401
base: master
Are you sure you want to change the base?
Conversation
@evulse Glad to see someone working on support for this! One question though: Why is adding a new type to represent null values necessary? Couldn't we just do this using func main() {
m := map[string]interface{}{}
_, priceOk := m["foo"].(float64)
v, priceNull := m["foo"]
fmt.Println(priceOk, priceNull && v == nil)
m["foo"] = nil
_, priceOk = m["foo"].(float64)
v, priceNull = m["foo"]
fmt.Println(priceOk, priceNull && v == nil)
}
|
@ccbrown I guess thats completely possible, I guess it was more to avoid any possible situations of the value being nil. Instead of going through and confirming that nil is never possibly outputted from maybe a nil pointer or struct it was easier to just add a new type knowing that nothing could possibly be using the new type. |
Any news on this pull request ? |
Facing a problem and I think this pull request fixes that, |
@chris-ramon Do you have some time to review this PR? What can I do to help getting this patch merged? |
Are there any chances this pull request will be merged into master? Is there anything we can do to help? |
Choice of implementation was based on not breaking existing code. For instance the following will not break with this change
However you can now detect null by adding another type assertion
As you can see the value of
graphql.NullValue
is irrelevant, we just need to test for its existence.The same thing can be accomplished by using a type switch, which there is an example of in the new example
crud-null
The motivation for this change is to bring inline with spec and that forcing null values on database fields is rather common. Plus obviously needed this functionality for current application use.
Also addresses #178