None's are converted to null, which Postgres doesn't like

[bkonkle]

I'm using postgraphile on the backend to speed up data development.

I'm creating a new object, and the type in the module generated from the %graphql expression results in option(Js.Json.t) for my id. I pass None for the id, and it's sent across in the graphql request in the variables as "id": null.

Postgraphile sees that "id": null and tries to set the id field in the database to null, which Postgres understandably complains about: null value in column "id" violates not-null constraint

Here's my module:

module CreateEvent = [%graphql
  {|
    mutation CreateEvent($input: CreateEventInput!) {
      createEvent(input: $input) {
        event {
          id
          createdAt
          updatedAt
          name
          date
          eventType
          location
          description
          owner: userByOwner {
            id
            name
            headline
          }
        }
      }
    }
  |}
];

And the resulting signature of the make function's arguments:

(
  ~input: Js.t(
    {
      ..
      clientMutationId : option(string),
      event : Js.t(
        {
          ..
          createdAt : option(Js.Json.t),
          date : Js.Json.t,
          description : option(string),
          eventType : [< `DEFAULT | `MEETUP ],
          id : option(Js.Json.t),
          location : option(string),
          name : string,
          owner : option(Js.Json.t),
          updatedAt : option(Js.Json.t)
        }
      )
    }
  ),
  unit
)

If I pass None for any of those optional values, they're sent across to the server as null. The server thinks I'm intentionally nullifying those fields. How can I work around?

Thanks for a great project!

[bkonkle]

Demo project here, with both sides of the interaction: https://github.com/ecliptic/reason-events

[mhallin]

Hello!

What would be the correct behavior for graphql_ppx here? To me, the problem seems to be the fact that the fields aren't really optional if you can't set them to null.

[bkonkle]

@mhallin It's the id, createdAt, and updatedAt fields - fields that are automatically set by Postgres and don't need to be provided initially.

[bkonkle]

I misspoke above - passing None for description and owner are fine because they are nullable.

[bkonkle]

^ My database table structure

[mhallin]

Hmm, I see. So how would you say "use the default value for this column" - would you just omit the argument from the object?

Conceptually, omitting an argument optional argument and passing an explicit null should be treated the same by a server. There's some discussion on this here: https://github.com/graphql/graphql-js/issues/133. Unfortunately, since JavaScript has both undefined and null, some JS implementations seem to make a distinction between the two.

I don't think this can be resolved from graphql_ppx's standpoint unfortunately. The option type only has two states, while a three-state enum would be required to represent "absent", "null", and "has a value".

[bkonkle]

Okay, this definitely makes sense. I can probably write a quick postgraphile plugin to strip the offending fields from create mutations (and yes, omitting them from the object seems to be the solution).

Looking through some bug reports elsewhere, it's also possible that this is related to my usage of uuid's for the primary key. I'm going to give it another go using classic serial ids and report back.

[bkonkle]

No luck, sadly. Classic integer-based serial id's suffer from the same issue. I'll close this ticket and pursue as a server plugin.

Thanks!

[bkonkle]

@mhallin I got some context from the postgraphile maintainer, and I think he has the right perspective:

To the best of my knowledge PostGraphile's behaviour is correct in this regard. Besides changing the behaviour would be a breaking change and it would violate the Principle of Least Astonishment given it maps a PostgreSQL database which works in the same way (omit the value to use the default, specify NULL if you want to override the default with an explicit null value) and changing it would reduce the expressiveness and functionality of the system. Ultimately passing {"a": 1"} vs {"a": 1, "b": null} to createEvent express different things - the latter is explicitly saying that b should be set to null, whereas the former has no information on b so it should be left as its default value.

I can totally relate to your issue though! I believe the issue is that your mapper is dealing with the input value as if it were a concrete type with fixed keys and values. Really it's a dictionary with a whitelist of keys and types those keys can be, but where every non-nullable key is optional (the entries in the dictionary itself are optional - not just the values).

It sounds like we need to use something other than Option to represent nullable values that have a default, because there are three possible intents: providing a value, providing a null value, and not providing a value.

What do you think?

[Neitsch]

While I see the issue in this case, it appears to be more of a weakness in the ReasonML spec, since it does not allow for the concept of absent.

For your case, why does CreateEventInput have an ID field in the first place? It looks like it will always be auto-generated.

[wokalski]

@Neitsch that's not right.

• Some(None) represents a nullable null value
• Some(Some(value)) represents a non-null value
• None represents nothing

[Neitsch]

My mistake, thank you for the correction!!

[mhallin]

Alright, so one workaround right now would be to break up the input object in the query, like

mutation ($name: String!) {
  createEvent(input: { event: { name: $name } }) {
    # ...
  }
}

There will be a lot of variables unfortunately, but it should work right now.

As for the larger point, it seems the GraphQL spec has been amended with a distinction of null and absent values. Is this only for input object fields or for all arguments? If it's only for input object fields we could probably use doubly-nested optionals like @wokalski said, but I can't really discern this from the specification.

[wokalski]

@mhallin why is it impossible to implement for all arguments?

[alexeygolev]

I like @wokalski solution. It makes sense for partial updates as well. At the moment, unless you use @mhallin solution of passing all values separately you can't really distinguish two scenarios — updating a field to null and not touching a field by omitting it from the input object

[bihnkim]

@mhallin After some back and forth, I think your idea of breaking up the input object in the query makes the most sense and is actually the most eloquent. The parameters declared in the first line of the query is the one responsible for defining the shape of the variables, rather than the input object. This gives fine-grain control and definition to our intensions, and though it may lead to more typing I think it's for the better and likely to cause less confusion than double-nesting optionals.

[tmattio]

I am using Absinthe for my backend and I ran into the same issue.

I have been using Elm for the frontend before ReasonML and the reference library (GraphqElm) implements the field with Absent | None | Maybe(v). @mhallin I am not sure I understand why it is not practical in the case of graphql_ppx?

[baransu]

I've ported #64 into baransu/graphql_ppx_re. If you need such functionality you can try the latest version :)