Skip to content
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

RFC: Client Controlled Nullability #895

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from 20 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 7 additions & 2 deletions spec/Appendix B -- Grammar Summary.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ Token ::
- FloatValue
- StringValue

Punctuator :: one of ! $ & ( ) ... : = @ [ ] { | }
Punctuator :: one of ! ? $ & ( ) ... : = @ [ ] { | }

Name ::

Expand Down Expand Up @@ -156,10 +156,15 @@ Selection :
- FragmentSpread
- InlineFragment

Field : Alias? Name Arguments? Directives? SelectionSet?
Field : Alias? Name Arguments? Nullability? Directives? SelectionSet?
twof marked this conversation as resolved.
Show resolved Hide resolved

Alias : Name :

Nullability :

- !
- ?

twof marked this conversation as resolved.
Show resolved Hide resolved
Arguments[Const] : ( Argument[?Const]+ )

Argument[Const] : Name : Value[?Const]
Expand Down
113 changes: 111 additions & 2 deletions spec/Section 2 -- Language.md
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ characters are permitted between the characters defining a {FloatValue}.

### Punctuators

Punctuator :: one of ! $ & ( ) ... : = @ [ ] { | }
Punctuator :: one of ! ? $ & ( ) ... : = @ [ ] { | }

GraphQL documents include punctuation in order to describe structure. GraphQL is
a data description language and not a programming language, therefore GraphQL
Expand Down Expand Up @@ -351,7 +351,7 @@ selection set. Selection sets may also contain fragment references.

## Fields

Field : Alias? Name Arguments? Directives? SelectionSet?
Field : Alias? Name Arguments? Nullability? Directives? SelectionSet?

A selection set is primarily composed of fields. A field describes one discrete
piece of information available to request within a selection set.
Expand Down Expand Up @@ -515,6 +515,115 @@ which returns the result:
}
```

## Nullability

Nullability :

- ListNullability NullabilityModifier?
- NullabilityModifier

ListNullability : `[` Nullability? `]`

NullabilityModifier :

- `!`
- `?`

Fields can have their nullability designated with either a `!` to indicate that
a field should be `Non-Nullable` or a `?` to indicate that a field should be
`Nullable`. These designators override the nullability set on a field by the
schema for the operation where they're being used. In addition to being
`Non-Nullable`, if a field marked with `!` resolves to `null`, it propagates to
the nearest parent field marked with a `?` or to `data` if one does not exist.
twof marked this conversation as resolved.
Show resolved Hide resolved
An error is added to the `errors` array identical to if the field had been
`Non-Nullable` in the schema.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update this once we land on error behavior.


In this example, we can indicate that a `user`'s `name` that could possibly be
`null`, should not be `null` and that `null` propagation should halt at the
`user` field:

```graphql example
{
user(id: 4)? {
id
name!
}
}
```

If `name` comes back non-`null`, then the return value is the same as if the
twof marked this conversation as resolved.
Show resolved Hide resolved
nullability designator was not used:
twof marked this conversation as resolved.
Show resolved Hide resolved

```json example
{
"user": {
"id": 4,
"name": "Mark Zuckerberg"
}
}
```

In the event that `name` is `null`, the field's parent selection set becomes
twof marked this conversation as resolved.
Show resolved Hide resolved
`null` in the result and an error is returned, just as if `name` was marked
`Non-Nullable` in the schema:

```json example
{
"data": {
"user": null
},
"errors": [
{
"locations": [{ "column": 13, "line": 4 }],
"message": "Cannot return null for non-nullable field User.name.",
"path": ["user", "name"]
}
]
}
```

If `user` was `Non-Nullable` in the schema, but we don't want `null`s
propagating past that point, then we can use `?` to create null propagation
boundary. `User` will be treated as `Nullable` for this operation:

```graphql example
{
user(id: 4)? {
id
name!
}
}
```
twof marked this conversation as resolved.
Show resolved Hide resolved

Nullability designators can also be applied to list elements like so.

```graphql example
{
user(id: 4)? {
id
petsNames[!]?
}
}
```

In the above example, the query author is saying that each individual pet name
should be `Non-Nullable`, but the list as a whole should be `Nullable`. The same
syntax can be applied to multidimensional lists.

```graphql example
{
threeDimensionalMatrix[[[?]!]]!
}
```

Any element without a nullability designator will inherit its nullability from
the schema definition, exactly the same as non-list fields do. When designating
twof marked this conversation as resolved.
Show resolved Hide resolved
nullability for list fields, query authors can either use a single designator
(`!` or `?`) to designate the nullability of the entire field, or they can use
the list element nullability syntax displayed above. The number of dimensions
indicated by list element nullability syntax is required to match the number of
dimensions of the field. Anything else results in a query validation error.

## Fragments

FragmentSpread : ... FragmentName Directives?
Expand Down
39 changes: 39 additions & 0 deletions spec/Section 5 -- Validation.md
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,45 @@ fragment conflictingDifferingResponses on Pet {
}
```

The same is true if a field is designated `Non-Nullable` in an operation. In
this case, `someValue` could be either a `String` or a `String!` which are two
different types and therefore can not be merged:

```graphql counter-example
fragment conflictingDifferingResponses on Pet {
... on Dog {
someValue: nickname
}
... on Cat {
someValue: nickname!
}
}
twof marked this conversation as resolved.
Show resolved Hide resolved
```

### Client Controlled Nullability Designator List Dimensions
twof marked this conversation as resolved.
Show resolved Hide resolved

**Formal Specification**

- For each {field} in the document
- Let {fieldDef} be the definition of {field}
- Let {fieldType} be the type of {fieldDef}
- Let {requiredStatus} be the required status of {field}
- Let {designatorDepth} be the number of square bracket pairs in
{requiredStatus}
twof marked this conversation as resolved.
Show resolved Hide resolved
- Let {typeDepth} be the number of list dimensions in {fieldType}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question for the community: Is it obvious what's meant by "list dimensions"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

more common term 'rank'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

number of dimensions is more like matrixes: M[i, j, k] - dimension 3; Graphq does not have these; what we have is rank (I believe)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PostgreSQL uses dimensions (and does not mention "rank"):

array_ndims ( anyarray )integer
Returns the number of dimensions of the array.
array_ndims(ARRAY[[1,2,3], [4,5,6]])2

C (and thus probably all C-style languages) uses dimensions; the manual page for arrays doesn't mention the term "rank": https://www.gnu.org/software/gnu-c-manual/gnu-c-manual.html#Multidimensional-Arrays

Haskell seems to use dimensions (this page on arrays doesn't mention "rank"): https://www.haskell.org/tutorial/arrays.html

Fortran uses rank, but defines rank as the "number of dimensions": https://www.ibm.com/docs/en/xffbg/121.141?topic=basics-rank-shape-size-array

.NET uses rank, but quickly defines it as "number of dimensions": https://docs.microsoft.com/en-us/dotnet/api/system.array.rank?view=net-6.0

I think "number of dimensions" is the most universal term, and "rank" is a shorthand used in some languages.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the .NET link above, note that at least in some languages, jagged arrays, i.e. arrays of arrays that need not be the same length, have rank/dimension of 1 rather than being truly multidimensional. Off the cuff, this is not of major concern, as the rank/dimension referred to seems to be most important there in terms of memory management. Although, I agree that the term "depth" more accurately describes what we have in GraphQL. Perhaps we can define depth even without the use of "dimension" at all.

- If {typeDepth} equals {designatorDepth} or {designatorDepth} equals 0 return
true
Comment on lines +595 to +596
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about this:

Suggested change
- If {typeDepth} equals {designatorDepth} or {designatorDepth} equals 0 return
true
- If {typeDepth} is greater or equal to {designatorDepth} return true

?

This way, 0 is just a particular case of a more generic rule that allows "skipping over the list items you want unchanged"

With this schema:

type Query {
  field: [[String]]
}

This would all be valid:

{
  # make the list required
  a: field! 
  # make the items of the list required
  a: field[!] 
  # make the items of the items of the list required
  a: field[[!]] 
}

I don't think it's going to be used that much but it's a more generic rule and avoids "0" as an outlier: everything that's not mentioned explicitely is untouched

Copy link
Contributor Author

@twof twof Jul 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Someone will need to go back through the notes, but IIRC the concern with this was that it's not clear how to read a list operator with fewer dimensions than the list type it's being applied to. ie

Does [!] == [[!]] or does [!] == [[]!]? @calvincestari will be leading a CCN discussion at the next working group meeting on August 3rd, and we'll also be having a CCN working group meeting on the 26th if you'd like to join that to discuss more: https://github.com/graphql/client-controlled-nullability-wg/blob/main/agendas/2023/2023-07-26.md

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does [!] == [[!]] or does [!] == [[]!] ?

I think it would work like for a single dimension list.

For this field definition list1: [String], list1! is the equivalent of list1[]!, not list1[!] so it's just a generalisation of the "single list" case. Just like ! == []!, I would expect [!] == [[]!] (modulo the caveat from above about [] but that's something else).

@calvincestari will be leading a CCN discussion at the next working group meeting on August 3rd, and we'll also be having a CCN working group meeting on the 26th if you'd like to join that to discuss more

I won't be able to join unfortunately but I'm working with Calvin so I'll discuss this with him :)!

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also think [!] == [[]!] makes sense.

But If {typeDepth} equals {designatorDepth} or {designatorDepth} equals 0 return allows future relaxing the rule to be If {typeDepth} is greater or equal to {designatorDepth} return true without breaking queries. So would it be possible to advance as is and wait for the feedback from the community?

- Otherwise return false

**Explanatory Text**

List fields can be marked with nullability designators that look like `[?]!` to
indicate the nullability of the list's elements and the nullability of the list
itself. For multi-dimensional lists, the designator would look something like
`[[[!]?]]!`. If the designator is not a simple `!` or `?`, then the number of
dimensions of the designator are required to match the number of dimensions of
the field's type. If the two do not match then a validation error is thrown.
twof marked this conversation as resolved.
Show resolved Hide resolved

### Leaf Field Selections

**Formal Specification**
Expand Down
68 changes: 62 additions & 6 deletions spec/Section 6 -- Execution.md
Original file line number Diff line number Diff line change
Expand Up @@ -570,13 +570,69 @@ ExecuteField(objectType, objectValue, fieldType, fields, variableValues):

- Let {field} be the first entry in {fields}.
- Let {fieldName} be the field name of {field}.
- Let {requiredStatus} be the required status of {field}.
- Let {argumentValues} be the result of {CoerceArgumentValues(objectType, field,
variableValues)}
- Let {resolvedValue} be {ResolveFieldValue(objectType, objectValue, fieldName,
argumentValues)}.
- Return the result of {CompleteValue(fieldType, fields, resolvedValue,
- Let {modifiedFieldType} be {ModifiedOutputType(fieldType, requiredStatus)}.
- Return the result of {CompleteValue(modifiedFieldType, fields, resolvedValue,
variableValues)}.
twof marked this conversation as resolved.
Show resolved Hide resolved

## Accounting For Client Controlled Nullability Designators

A field can have its nullability status set either in its service's schema, or a
nullability designator (! or ?) can override it for the duration of an
twof marked this conversation as resolved.
Show resolved Hide resolved
execution. In order to determine a field's true nullability, both are taken into
account and a final type is produced.

ModifiedOutputType(outputType, requiredStatus):
twof marked this conversation as resolved.
Show resolved Hide resolved

- Create a {stack} initially containing {type}.
twof marked this conversation as resolved.
Show resolved Hide resolved
- As long as the top of {stack} is a list:
- Let {currentType} be the top item of {stack}.
- Push the {elementType} of {currentType} to the {stack}.
- If {requiredStatus} exists:
- Start visiting {node}s in {requiredStatus} and building up a
{resultingType}:
- For each {node} that is a RequiredDesignator:
- If {resultingType} exists:
- Let {nullableResult} be the nullable type of {resultingType}.
- Set {resultingType} to the Non-Nullable type of {nullableResult}.
- Continue onto the next node.
- Pop the top of {stack} and let {nextType} be the result.
- Let {nullableType} be the nullable type of {nextType}.
- Set {resultingType} to the Non-Nullable type of {nullableType}.
- Continue onto the next node.
- For each {node} that is a OptionalDesignator:
- If {resultingType} exists:
- Set {resultingType} to the nullableType type of {resultingType}.
- Continue onto the next node.
- Pop the top of {stack} and let {nextType} be the result.
- Set {resultingType} to the nullable type of {resultingType}
- Continue onto the next node.
- For each {node} that is a ListNullabilityDesignator:
- Pop the top of {stack} and let {listType} be the result
- If the nullable type of {listType} is not a list
- Pop the top of {stack} and set {listType} to the result
- If {listType} does not exist:
- Throw an error because {requiredStatus} had more list dimensions than
{outputType} and is invalid.
- If {resultingType} exist:
- If {listType} is Non-Nullable:
- Set {resultingType} to a Non-Nullable list where the element is
{resultingType}.
- Otherwise:
- Set {resultingType} to a list where the element is {resultingType}.
- Continue onto the next node.
- Set {resultingType} to {listType}
- If {stack} is not empty:
- Throw an error because {requiredStatus} had fewer list dimensions than
{outputType} and is invalid.
- Return {resultingType}.
- Otherwise:
- Return {outputType}.

### Coercing Field Arguments

Fields may include arguments which are provided to the underlying runtime in
Expand Down Expand Up @@ -778,9 +834,9 @@ field returned {null}, and the error must be added to the {"errors"} list in the
response.

If the result of resolving a field is {null} (either because the function to
resolve the field returned {null} or because a field error was raised), and that
field is of a `Non-Null` type, then a field error is raised. The error must be
added to the {"errors"} list in the response.
resolve the field returned {null} or because a field error was raised), and the
{ModifiedOutputType} of that field is of a `Non-Null` type, then a field error
is raised. The error must be added to the {"errors"} list in the response.

If the field returns {null} because of a field error which has already been
added to the {"errors"} list in the response, the {"errors"} list must not be
Expand All @@ -789,8 +845,8 @@ field.

Since `Non-Null` type fields cannot be {null}, field errors are propagated to be
handled by the parent field. If the parent field may be {null} then it resolves
to {null}, otherwise if it is a `Non-Null` type, the field error is further
propagated to its parent field.
to {null}, otherwise if its {ModifiedOutputType} is a `Non-Null` type, the field
twof marked this conversation as resolved.
Show resolved Hide resolved
error is further propagated to its parent field.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If folks are cool with it, I think we should be more precise here and say that nulls are propagated rather than errors. I think the language about error propagation likely stems from the GraphQL-JS implementation which centers its logic for propagation decisions around errors, but the errors themselves technically have no impact on any fields beyond causing the originating field to become null, which may in turn violate the spec and cause propagation. The only way parent fields can "handle" an error propagated from a child field is by being Nullable.

If the intent is to provide fields other means of error handling in a future version of the spec, then I could see a reason to keep the current language.


If a `List` type wraps a `Non-Null` type, and one of the elements of that list
resolves to {null}, then the entire list must resolve to {null}. If the `List`
Expand Down
9 changes: 5 additions & 4 deletions spec/Section 7 -- Response.md
Original file line number Diff line number Diff line change
Expand Up @@ -160,10 +160,11 @@ The response might look like:
}
```

If the field which experienced an error was declared as `Non-Null`, the `null`
result will bubble up to the next nullable field. In that case, the `path` for
the error should include the full path to the result field where the error was
raised, even if that field is not present in the response.
If the field which experienced an error was declared as `Non-Null` or designated
`Non-Null` in the query document, the `null` result will propagate to the next
twof marked this conversation as resolved.
Show resolved Hide resolved
nullable field. In that case, the `path` for the error should include the full
path to the result field where the error was raised, even if that field is not
present in the response.

For example, if the `name` field from above had declared a `Non-Null` return
type in the schema, the result would look different but the error reported would
Expand Down