Postgraphile: Updating a compsite type nulls other fields that are not defined in the mutation

[jvice152]

I'm submitting a ...

• [x ] bug report
• [ ] feature request
• [ ] question

PostGraphile version:

when updating fields under a composite type, it will null every field i have not defined...

Minimal SQL file that can be loaded into a clean database:

create type my_type as (
  foo integer,
  bar integer
);

create table post (
  id               serial primary key,
  customType       my_type,
  created_at       timestamp default now()
);

Steps to reproduce:

Current behavior:

i am running via npm, with the debug for sql on below is my create and update, only modifying the foo

mutation testCreate($post:CreatePostInput!){
  createPost(input: $post){
    clientMutationId
    post{
      id
      customtype{
        foo
        bar
      }
    }
  }
}

mutation testUpdate($updatePost:UpdatePostByIdInput!){
  updatePostById(input: $updatePost){
    clientMutationId
    post{
      id
      customtype{
        foo
      }
    }
  }
}

query getPosts{
  posts{
    nodes{
      id
      customtype{
        foo
        bar
      }
    }
  }
}
{
  "post": {
    "clientMutationId": "123",
    "post": {
      "customtype": {
        "foo": 0,
        "bar": 0
      }
    }
  },
  "updatePost": {
    "id": 1,
    "patch": {
      "customtype": {
            "foo": 1
      }
    }
  }
}

The update will null out any field not defined in the mutation, pay attention to the update "set "customtype" = row($1::"pg_catalog"."int4",NULL)"

 postgraphile:postgres begin +57s
  postgraphile:postgres SAVEPOINT graphql_mutation +3ms
  postgraphile:postgres 
  postgraphile:postgres       with __local_0__ as (
  postgraphile:postgres         
  postgraphile:postgres                       update "public"."post" set "customtype" = row($1::"pg_catalog"."int4",NULL)::"public"."my_type"
  postgraphile:postgres                       where ("id" = $2)
  postgraphile:postgres                       returning *
  postgraphile:postgres       )
  postgraphile:postgres       select ((case when __local_0__ is null then null else __local_0__ end))::text from __local_0__ +1ms
  postgraphile:postgres               with __local_0__ as (
  postgraphile:postgres                 select (str::"public"."post").*
  postgraphile:postgres                 from unnest(($1)::text[]) str
  postgraphile:postgres               )
  postgraphile:postgres               
  postgraphile:postgres       select to_json((__local_0__."id")) as "id", to_json((case when (__local_0__."customtype") is not distinct from null then null else json_build_object('foo'::text, ((__local_0__."customtype")."foo")) end)) as "customtype"
  postgraphile:postgres       from __local_0__ as __local_0__
  postgraphile:postgres       
  postgraphile:postgres       where (TRUE) and (TRUE)
  postgraphile:postgres       
  postgraphile:postgres       
  postgraphile:postgres       
  postgraphile:postgres     
  postgraphile:postgres                +4ms
  postgraphile:postgres RELEASE SAVEPOINT graphql_mutation +1ms
  postgraphile:postgres commit +1ms

Expected behavior:

i expect the update to only update the foo field in composite type "customType" and not set everything else to NULL

[jvice152]

i see you are converting the entire object to json... perhaps a json delta where the value is not supplied. perhaps a merge instead of an entire blow away?

see: http://blog.bguiz.com/2017/json-merge-postgresql/

[benjie]

Only the fields on the patch object itself are applied as a patch, the values specified in the patch (e.g. ints, strings, your composite type) are then used to replace the previous values. This behaviour is intentional.

Supporting patch on sub-fields is an interesting idea, but a custom or additional mutation might be a better way of defining the exact behaviour you want. What exactly is the use case for your customtype so I can understand your intentions better? Often in PostgreSQL it's better to use a separate table rather than a compound type column, and GraphQL mutations should be fairly lean and model a specific behaviour well rather than trying to be too generic (see for example this post).

[jvice152]

thanks for the quick response!

I would love to do away with the anemic model i have currently in regards to mutations, but the first step in this legacy migration is to allow generic crud operations on the entities without introducing too many variables to the migration. Also I wanted to encapsulate all of the child entities, which is why i made rowtypes/composite to types that i didnt treat as the root level object

https://martinfowler.com/bliki/DDD_Aggregate.html

id love to have subfields support the same patch behavior as top level fields. I figure such behavior could potentially be supported because these are known postgres types, not generic json/jsonb

[benjie]

Yeah; they definitely could be supported. Doing so would be a breaking change (and is not default behaviour I desire) but you could do it via a Graphile Engine flag. If you wanted to do so,

Here's where the type for a column is determined, you'd want to return something different if isPgPatch is true:

https://github.com/graphile/graphile-engine/blob/e5b1ba93ce550ba51da5e11cd644e3efdfc93cbd/packages/graphile-build-pg/src/plugins/PgColumnsPlugin.js#L232-L242

Here's where the patch type for a _table_ is generated:

https://github.com/graphile/graphile-engine/blob/e5b1ba93ce550ba51da5e11cd644e3efdfc93cbd/packages/graphile-build-pg/src/plugins/PgTablesPlugin.js#L211-L240

Types are technically stored in the same relation (pg_class) so you might need to just soften this check:

https://github.com/graphile/graphile-engine/blob/e5b1ba93ce550ba51da5e11cd644e3efdfc93cbd/packages/graphile-build-pg/src/plugins/PgTablesPlugin.js#L205

---

Another option, which is probably easier (and arguably better), is to add a plugin that adds additional fields to your patch type. That way it would have both customType: CustomType and customTypePatch: CustomTypePatch present. I'm not exactly sure how that would work with the actual update mutation though:

https://github.com/graphile/graphile-engine/blob/e5b1ba93ce550ba51da5e11cd644e3efdfc93cbd/packages/graphile-build-pg/src/plugins/PgMutationUpdateDeletePlugin.js#L122-L133

We may need to tweak the hooks around that to make it more feasible.

---

Another option is to replace the auto CRUD mutations with your own versions. The Cookbook discusses how you might go about doing that here. My preferred approach would be the makeExtendSchemaPlugin because it's very explicit, quite short, and fairly flexible. It does mean writing a new one manually for each mutation though (currently at least...)

[jvice152]

you will have to forgive me, as i have just been playing with this engine as of this week. If i hear you correctly, option 1 & 2 (two being the preferred) will require a fork in either instance, and 3 is just explicitly creating the mutations?

sounds like i have to deal with composite types being treated as primitives until further notice

[benjie]

(1) would be a pull request, or you can just replace an internal plugin which means you’re only “forking” one file, you would still use official postgraphile from npm to run your app;

(2) would be a small plugin (postgraphile is all about plugins to customise things for your needs) which means no need to fork, but I think it might not work, alas. Input types don’t have as powerful hooks as output types yet.

(3) is indeed explicitly creating the mutations, but presumably you don’t need this for every single mutation; so really it’s just replacing the couple of mutations you need this behaviour with. https://www.graphile.org/postgraphile/make-extend-schema-plugin/

Another option, and I’m not sure if this is appropriate for your needs (are nulls in the composite type desirable?) would be to create a pg trigger which performs the old/new merge on update, copying old values over nulls.

You may sponsor the development of this feature if you prefer.

[benjie]

I'm closing this for now as "works as intended". If you're interested in implementing this feature (or sponsoring its development) then please get in touch 👍

[jvice152]

sorry, not running away. Currently evaluating different postgres to graphql implementations. If we go this route it is definitely something I would be interested in implementing :)

[benjie]

I’d definitely be interested to hear your conclusion and reasoning; if you care to share you can email me benjie at graphile org.

[srp]

For what it's worth, query conditions on composites also only work atomically. As an example let's say I have a composite and use it in a table:

CREATE TYPE address AS (street text, city text, state text, zip text);
CREATE TABLE user(id SERIAL, username text, addr address);

Issuing a query against one of it's fields returns 0 results.

query {
   users(addr: {city: "Austin"}) {
      ...
   }
}

Running in debug, the query sent to the database is:

...
where (
  __local_1__."addr" = row(
    NULL,
    $1::"pg_catalog"."text",
    NULL,
    NULL
 )::"public"."address"
)
...

Yes, this is probably not an idea data model to begin with, but I would have expected something more like:

where (
  __local_1__."addr"."city" = $1::"pg_catalog"."text"
)

[benjie]

You probably want the filter plugin for that kind of behaviour.

[srp]

If you're referring to _postgraphile-plugin-connection-filter_ it doesn't look like it's generating any filters for my composite field. That said, I was trying to use a composite as a poor man's substitute until _postgraphile-plugin-nested-mutations_ learns how to do updates. I should probably bite the bullet and figure out how to improve the later so I can model my data the right way to begin with.