id is non-nullable in mutation input in view using INSTEAD OF INSERT

[jsmnbom]

Summary

I made a view, to expose an underlying translationlayer in a way that react-admin would understand. This works great for showing it, but when trying to create a new one, I ran into an issue where I cannot get postgraphile to output a correct mutation.

Steps to reproduce

• Create a db (viewinsertbug for example purposes)
• Execute this sql (i tried removing as much as I could while still having it make sense in terms of how I use it)

create extension if not exists "hstore";

create table public.tr_language (
    code text primary key,
    display_name text not null
);

create table public.event (
    id serial primary key,
    starts_at timestamptz not null,
    ends_at timestamptz not null
);

create table public.event_tr (
    event_id integer references public.event (id),
    language_code text references public.tr_language (code),
    title text not null,
    primary key (event_id, language_code)
);

create view public.admin_event as
select
    e.id,
    e.starts_at,
    e.ends_at,
    hstore (array_agg(et.language_code), array_agg(et.title)) title
from
    public.event e
    inner join public.event_tr et on et.event_id = e.id
group by
    e.id;

comment on view public.admin_event is E'@primaryKey id';

comment on column public.admin_event.starts_at is E'@notNull';

comment on column public.admin_event.ends_at is E'@notNull';

comment on column public.admin_event.title is E'@notNull';

create function public.admin_event_insert ()
    returns trigger
    as $$
declare
    e_id int;
    f record;
begin
    insert into public.event
        values (default, new.starts_at, new.ends_at)
    returning
        id into e_id;
    for f in
    select
        title.key as language_code,
        title.value as title
    from
        each (new.title::hstore) title loop insert into public.event_tr
            values (e_id, f.language_code, f.title);
end loop;
    return new;
end;
$$
language plpgsql;

create trigger admin_event_insert
    instead of INSERT on public.admin_event for each row
    execute procedure public.admin_event_insert ();

--comment on function public.admin_event_insert() IS E'@arg0variant patch';
--comment on column public.admin_event.id IS E'@omit create'

• Start postgraphile, e.g. using

~/.node_modules/bin/postgraphile \
  --connection postgres://localhost/viewinsertbug \
  --owner-connection postgres://localhost/viewinsertbug \
  --schema public \
  --enhance-graphiql \
  --append-plugins @graphile-contrib/pg-simplify-inflector \
  --no-ignore-rbac

Expected results

The mutation below should work and create an event, as well as two event_trs.

mutation MyMutation {
  createAdminEvent(
    input: {
      adminEvent: {
        startsAt: "2021-10-20T21:49:23.051Z"
        endsAt: "2021-10-29T21:49:00.000Z"
        title: { en: "ent", da: "dat" }
      }
    }
  ) {
    endsAt
      startsAt
      id
      title
  }
}

Actual results

When running the above query the following error is outputted.
"Field \"AdminEventInput.id\" of required type \"Int!\" was not provided."

Additional context

Linux 5.14.12-arch1-1
Node v16.11.1
NPM 8.1.0
PostgreSQL 13.4
PostGraphile 4.12.4

Possible Solution

I tried solving it by adding a smart comment.
comment on column public.admin_event.id IS E'@omit create';

And this helps a lot and the mutation actually succeeds, but then we get another issue. The returned data is

{
  "data": {
    "createAdminEvent": {
      "adminEvent": {
        "endsAt": "2021-10-29T23:49:00+02:00",
        "startsAt": "2021-10-20T23:49:23.051+02:00",
        "id": null,
        "title": {
          "da": "dat",
          "en": "ent"
        }
      }
    }
  }
}

which has no id field, even tho the mutation did get an id assigned in the database, and I need the id.

Another thing i tried was adding the smart comment
comment on function public.admin_event_insert() IS E'@arg0variant patch';
but this obviously doesn't work since admin_event_insert() doesn't actually accept any arguments.

I think a solution might be to add a smart comment @newVariant, that sets the type of NEW inside the function, but I honestly have no idea anymore :/

[benjie]

Good find! So you've raised a couple different issues here.

Issue 1: how do you mark an @primaryKey column as nullable?

There should be a way to mark a view's @primaryKey column(s) as nullable. Currently you can do this with makeChangeNullabilityPlugin, but that's a bit of a hack - I've raised a PR to add a @hasDefault smart tag you can apply to the primary key column(s) to indicate that they are nullable because they have defaults: graphile/graphile-engine#760 (I'd appreciate a PR to the smart tags page detailing this tag if you have time ❤️)

Issue 2: why is id null when you @omit create?

@omit create is what I'd actually recommend using, so hitting this is frustrating. It turns out that this is due to a minor bug in your code: when you have your instead of insert trigger, you are returning the NEW record, but the NEW record is effectively the same as the input - it doesn't actually have an id value. At no point does this fact change - the NEW.id remains null forever. When PostGraphile sees the result of this it trusts it and tries to render it out, but of course there's no id so it throws an error.

Fortunately fixing this situation is as simple as assigning an id value to the NEW record:

    -- ...
    insert into public.event
        values (default, new.starts_at, new.ends_at)
    returning
        id into e_id;

    -- SET THE id OF THE NEW RECORD:
    NEW.id = e_id;
    -- 👆👆👆👆👆

    for f in
    select
    -- ...

---

Regarding your final couple paragraphs, PostGraphile knows nothing of triggers and Postgres knows nothing of smart tags, so smart tags on trigger functions/triggers would not make any difference.

---

Thank you for taking the time to make a great reproduction of the issue, this made diagnosing it much faster. The only thing it lacked was a little data in tr_language:

insert into tr_language(code, display_name) values
  ('da', 'da'),
  ('en', 'en');

Here's the full reproduction script I used (including the new.id = fix):

#!/usr/bin/env bash
set -e

# What DB are we messing with? (THIS DB WILL BE OVERWRITTEN!)
DATABASE="issue-1544"
SCHEMA="public"

# Populate database
dropdb --if-exists "$DATABASE"
createdb "$DATABASE"
psql -X1v ON_ERROR_STOP=1 "$DATABASE" <<HERE
create extension if not exists "hstore";

create table public.tr_language (
    code text primary key,
    display_name text not null
);
insert into tr_language(code, display_name) values
  ('da', 'da'),
  ('en', 'en');

create table public.event (
    id serial primary key,
    starts_at timestamptz not null,
    ends_at timestamptz not null
);

create table public.event_tr (
    event_id integer references public.event (id),
    language_code text references public.tr_language (code),
    title text not null,
    primary key (event_id, language_code)
);

create view public.admin_event as
select
    e.id,
    e.starts_at,
    e.ends_at,
    hstore (array_agg(et.language_code), array_agg(et.title)) title
from
    public.event e
    inner join public.event_tr et on et.event_id = e.id
group by
    e.id;

comment on view public.admin_event is E'@primaryKey id';

comment on column public.admin_event.starts_at is E'@notNull';

comment on column public.admin_event.ends_at is E'@notNull';

comment on column public.admin_event.title is E'@notNull';

create function public.admin_event_insert ()
    returns trigger
    as $\$
declare
    e_id int;
    f record;
begin
    insert into public.event
        values (default, new.starts_at, new.ends_at)
    returning
        id into e_id;
    new.id = e_id;
    for f in
    select
        title.key as language_code,
        title.value as title
    from
        each (new.title::hstore) title loop insert into public.event_tr
            values (e_id, f.language_code, f.title);
end loop;
    return new;
end;
$\$
language plpgsql;

create trigger admin_event_insert
    instead of INSERT on public.admin_event for each row
    execute procedure public.admin_event_insert ();

--comment on function public.admin_event_insert() IS E'@arg0variant patch';
comment on column public.admin_event.id IS E'@omit create';
HERE

# Install deps
yarn add --dev postgraphile @graphile-contrib/pg-simplify-inflector

# Run PostGraphile
yarn postgraphile \
  --subscriptions \
  --watch \
  --dynamic-json \
  --no-setof-functions-contain-nulls \
  --no-ignore-rbac \
  --no-ignore-indexes \
  --show-error-stack=json \
  --extended-errors hint,detail,errcode \
  --append-plugins @graphile-contrib/pg-simplify-inflector \
  --graphiql "/" \
  --enhance-graphiql \
  --allow-explain \
  --enable-query-batching \
  --legacy-relations omit \
  --connection "$DATABASE" \
  --schema "$SCHEMA"

[jsmnbom]

Thank you so much for the very detailed answer, and solution to my problem. Sorry about the missing data, I had some sample data to start out with, but thought it wasn't needed afterall, glad you could recreate it ^^

Yeah I'll try and make a documentation PR when I have time.

For future reference (or if anyone against all odds should be in the same situation i ended up in) I went quite a bit more complicated solution in the end, after realizing how many tables I would need to write update functions for. So I wrote one function that supports them all. Not expecting anyone to read it or anything, cause holy crap is it a mess :P

create or replace function array_diff(array1 anyarray, array2 anyarray)
returns anyarray language sql immutable as $$
    select coalesce(array_agg(elem), '{}')
    from unnest(array1) elem
    where elem <> all(array2)
$$;

CREATE OR REPLACE FUNCTION format_array(arr TEXT[], format_str TEXT, delim TEXT) RETURNS TEXT AS $$
DECLARE
	_i TEXT;
	_tmp TEXT[];
BEGIN
	_tmp := '{}';
	foreach _i in array arr loop
    	_tmp := _tmp || format(format_str, _i);
	end loop;
	RETURN array_to_string(_tmp, DELIM, '*');
END;
$$
language plpgsql;

CREATE OR REPLACE FUNCTION public.get_columns(table_name TEXT, exclude_columns TEXT[]) RETURNS TEXT[] AS $$
DECLARE
	_tmp TEXT[];
BEGIN
	EXECUTE format($q$
		SELECT array_diff(array_agg(column_name), %2$L) FROM information_schema.COLUMNS WHERE table_name = %1$L AND table_schema = 'smil_aarhus';
	$q$, table_name, exclude_columns) INTO _tmp;
	RETURN _tmp;
END;
$$
language plpgsql;

CREATE OR REPLACE FUNCTION public.tr_update_implementation() RETURNS trigger AS $func$
DECLARE
	_tbl_columns TEXT[];
	_tr_columns TEXT[];
	_query TEXT;
	_l record;
	_lang record;
	tbl TEXT := TG_ARGV[0];
	tbl_id TEXT := TG_ARGV[1];
	tr_tbl TEXT := TG_ARGV[2];
	tr_tbl_id TEXT := TG_ARGV[3];
BEGIN
	if (TG_OP = 'DELETE') THEN
		EXECUTE format($q$
			delete from public.%1$I where %2$I = old.%2$I;
		$q$, tbl, tbl_id);
		EXECUTE format($q$)
			delete from public.%1$I where %2$I = old.%3$I;
		$q$, _tr_tbl, _tr_tbl_id, _tbl_id);
		RETURN OLD;
	elsif (TG_OP = 'UPDATE' OR TG_OP = 'INSERT') THEN
		SELECT public.get_columns(tbl, ARRAY [tbl_id]) INTO _tbl_columns;
		SELECT public.get_columns(tr_tbl, ARRAY [tr_tbl_id, 'language_code']) INTO _tr_columns;

		IF (TG_OP = 'UPDATE') THEN
			IF ARRAY_length(_tbl_columns, 1) > 0 THEN
				EXECUTE format($q$
					UPDATE public.%1$I SET %3$s WHERE %2$I = $2.%2$I;
			     $q$, tbl, tbl_id,format_array(_tbl_columns, '%1$I = $1.%1$I', ',')) USING NEW,OLD;
		    END IF;
		   	_query := format($q$
		   		SELECT  code, new_has, old_has FROM public.tr_language tl FULL outer JOIN unnest(akeys($1.%1$I::hstore)) new_has ON new_has = tl.code FULL outer JOIN unnest(akeys($2.%1$I::hstore)) old_has ON old_has = tl.code;
	   		$q$, _tr_columns[1]);
		   	FOR _lang IN EXECUTE _query USING NEW, OLD LOOP
	   			IF (_lang.new_has IS NOT null AND _lang.old_has IS NOT null) then 
	   				_query := $q$UPDATE public.%1$I SET %5$s WHERE %3$I = $2.%2$I and language_code = $3;$q$;
				ELSIF (_lang.new_has IS NOT null AND _lang.old_has IS null) then 
					_query := $q$insert into public.%1$I values ($2.%2$I, $3, %4$s);$q$;
				ELSIF (_lang.new_has IS null AND _lang.old_has IS NOT null) THEN
					_query := $q$delete from public.%1$I WHERE %3$I = old.%2$I and language_code = $3;$q$;
				END IF;
				_query := format(_query, tr_tbl, tbl_id, tr_tbl_id, format_array(_tr_columns, '$1.%1$I -> $3', ','), format_array(_tr_columns, '%1$I = $1.%1$I -> $3', ','));
				EXECUTE _query USING NEW, OLD, _lang.code;
		   	END LOOP;
			RETURN NEW;
		ELSIF (TG_OP = 'INSERT') THEN
			IF ARRAY_length(_tbl_columns, 1) > 0 THEN
		   		_query := format($q$
					INSERT into public.%1$I VALUES (DEFAULT, %3$s) RETURNING %2$I
			    $q$, tbl, tbl_id, format_array(_tbl_columns, '$1.%1$I'));
			ELSE
				_query := format($q$
					INSERT into public.%1$I VALUES (DEFAULT) RETURNING %2$I
			    $q$, tbl, tbl_id);
			END IF;
			EXECUTE _query USING NEW INTO _l;
		
			NEW.id := _l.id;
			
			_query := format($q$
		   		SELECT  code, new_has FROM public.tr_language tl FULL outer JOIN unnest(akeys($1.%1$I::hstore)) new_has ON new_has = tl.code;
	   		$q$, _tr_columns[1]);
		   	FOR _lang IN EXECUTE _query USING NEW LOOP
				IF (_lang.new_has IS NOT null) then 
					_query := $q$insert into public.%1$I values ($1.%2$I, $2, %3$s);$q$;
					_query := format(_query, tr_tbl, tbl_id, format_array(_tr_columns, '$1.%1$I -> $2', ','));
					EXECUTE _query USING NEW, _lang.code;
				END IF;
		   	END LOOP;
		   RETURN NEW;
		END IF;
	END IF;
END;
$func$
language plpgsql;

create trigger update_admin_event
    instead of INSERT or update or delete on public.admin_event for each row
    execute procedure public.tr_update_implementation ('event', 'id', 'event_tr', 'event_id');