Is it possible to have only some parameters required?

[EyMaddis]

When writing a function in Postgres I can add the STRICT flag which results in all fields to be declared as required in GraphQL (e.g. String!). Omitting strict will result in all fields being optional.

Is there a way to have some values as required and some not? I was hoping that using DEFAULT null for the argument could achieve something like this (but I think Postgres does not support this).

[benjie]

Not as far as I know; you can make them all optional and then add code in the body like:

create function blah(foo varchar, bar varchar) as $$
begin;
if foo is null then
raise exception 'foo is required';
end if;
-- ...
end;
$$ language plpgsql;

(Untested pseudocode)

But of course it will still seem optional in the schema. 😞

[EyMaddis]

That is how I do it right now :(

[EyMaddis]

So would there be any way to put this into the schema? Is it for example possible to detect default values?

[benjie]

I had forgotten that Postgres functions had default arguments, yes we should totally be able to detect this. PR welcome!

[EyMaddis]

@benjie but aren't Postgres functions (without STRICT) always optional with a default of null?

[benjie]

pronargdefaults gives the number of arguments for which defaults are specified (in PGSQL once an argument has a default, all following arguments must also have a default); so we can totally use this to make those ones optional if a function is strict; then we just have to be sure to not pass those arguments if they are null.

[postgres] # create function foo(bar varchar , baz varchar='qux') returns varchar as $$ 
select bar || baz; $$ language sql strict;
CREATE FUNCTION
Time: 2.579 ms
[postgres] # select foo('a');
 foo
------
 aqux
(1 row)

Time: 0.347 ms
[postgres] # select foo('a', 'b');
 foo
-----
 ab
(1 row)

Time: 0.336 ms
[postgres] # select proisstrict, pronargdefaults from pg_catalog.pg_proc 
where proname = 'foo';
 proisstrict | pronargdefaults
-------------+-----------------
 t           |               1
(1 row)

Time: 0.525 ms

[benjie]

Step 1 would be adding pronargdefaults to the introspection query:

https://github.com/postgraphql/postgraphql/blob/master/resources/introspection-query.sql#L34

[benjie]

Turns out we already have it in the introspection query; and it's already used here:

https://github.com/postgraphql/postgraphql/blob/ade728ed8f8e3ecdc5fdad7d770c67aa573578eb/src/postgraphql/schema/procedures/createPgProcedureSqlCall.ts#L36-L39

Is it not correctly marking the arguments as optional in the schema?

[EyMaddis]

Nope. I just tested it with your function:

Strict with default

Function

create or replace function blah(foo varchar, bar varchar='default') returns bool as $$
begin
    if foo is null then
        raise exception 'foo is required';
    end if;
    return true;
end;
$$ language plpgsql STRICT;

BlahInput

clientMutationId: String
foo: String!
bar: String!

Non-Strict with default

Function

create or replace function blah(foo varchar, bar varchar='default') returns bool as $$
begin
    if foo is null then
        raise exception 'foo is required';
    end if;
    return true;
end;
$$ language plpgsql;

BlahInput

clientMutationId: String
foo: String
bar: String

This is actually correct Postgres behaviour:

CALLED ON NULL INPUT (the default) indicates that the function will be called normally when some of its arguments are null. It is then the function author's responsibility to check for null values if necessary and respond appropriately.

RETURNS NULL ON NULL INPUT or STRICT indicates that the function always returns null whenever any of its arguments are null. If this parameter is specified, the function is not executed when there are null arguments; instead a null result is assumed automatically.

https://www.postgresql.org/docs/9.1/static/sql-createfunction.html

Even though there is no "partial strict", IMO PostGraphQL should support this. Deciding between either all parameters or none are required is not exactly brilliant. Maybe there is a way to solve this.

[benjie]

It works by omitting the argument entirely (rather than passing null) as in my example above:

[postgres] # select foo('a');
 foo
------
 aqux

I saw we're using argDefaultsNum linked above in the function call, but aren't using it anywhere else - it should be used when generating the type to mark certain arguments as optional. Should be relatively easy to fix...?

[TreeMan360]

+1 definitely would like to see someone pick this up

[benjie]

If someone wants to take this on we just need to reference argDefaultsNum when executing the procedure - that number is the number of "optional" arguments to the function (so effectively we have to count backwards from the end).

I think this might be a breaking change though. Either we should opt in via a CLI argument or release it in a new major version.

[chadfurman]

@benjie What do you mean by, "when executing the procedure"? The file you linked is creating the SQL call, and it looks like it's taking default arguments into account as you pointed out.

Edit: Maybe here? https://github.com/postgraphql/postgraphql/blob/ade728ed8f8e3ecdc5fdad7d770c67aa573578eb/src/postgraphql/schema/procedures/createPgProcedureMutationGqlFieldEntry.ts#L97

[benjie]

I think I must have got confused; I meant "when defining the procedure". This code:

https://github.com/postgraphql/postgraphql/blob/ade728ed8f8e3ecdc5fdad7d770c67aa573578eb/src/postgraphql/schema/procedures/createPgProcedureMutationGqlFieldEntry.ts#L44-L50

(or specifically line 48) needs to factor in argDefaultsNum to set the required arguments (all but the last argDefaultsNum arguments) as NonNull.

However this definitely needs a CLI flag to opt in to this behaviour.

[benjie]

This functionality is implemented in graphile-build-pg via the pgStrictFunctions setting, but this isn't currently exposed via postgraphile-core to PostGraphQL itself. What it does is treats all functions as strict, requiring all arguments to be marked as required unless they have defaults.

It is possible to mark an argument as default null, but of course a strict function with a null default will automatically return null without being called.

create function a(b int, c int default null)
returns int as $$
  select b;
$$ language sql stable;

                                   List of functions
 Schema | Name | Result data type |            Argument data types             |  Type
--------+------+------------------+--------------------------------------------+--------
 a      | a    | integer          | b integer, c integer DEFAULT NULL::integer | normal
(1 row)

By default neither b nor c will be required, but with pgStrictFunctions set b will be marked as required (but c will not).

Is this in line with what you wanted?

[chadfurman]

Really cool. So you don't need to make the function strict in the schema, postgraphile will make it "strict" at the API level because the arg doesn't have a dafault

[benjie]

Yes, so long as you opt in to that functionality. All we need is someone to add pgStrictFunctions: strictFunctions here-ish (PR welcome):

https://github.com/graphile/graphile-build/blob/master/packages/postgraphile-core/src/index.js#L87

(plus the dereference) and then you can enable it from PostGraphQL in library mode. (You can do it currently via graphqlBuildOptions but that's a bit of a hack and I'll be changing the name of it shortly too.) It would be great if someone would add a CLI option to PostGraphQL to do the same also.

[benjie]

@chadfurman Have you been using this functionality? Does it work? I've not got any tests in place for it yet.

[chadfurman]

@benjie I've been making all my functions with parameters "strict" -- I have not tested this functionality yet. Assign me this ticket and I'll report back on my findings. If everything goes well, I'll add something to the docs, as well.

[pencilcheck]

Using @next at 4.0.0-alpha2.20 It seems to be working for me, I have a strict function, have some parameters with default values, and in the graphiql inspector it marks the required argument correctly.

[benjie]

Great, thanks @pencilcheck

[benjie]

@chadfurman Can you confirm @pencilcheck's findings? If so I'll happily mark this issue as fixed in v4 👍

[benjie]

Make optional function arguments work correctly

[benjie]

$ createdb test
$ psql test
[test] # create function test(a int, b int default 0, c int default 0) returns int as $$ select a + b + c; $$ language sql stable strict;
CREATE FUNCTION
Time: 17.818 ms
[test] # select test(7);
 test
------
    7
(1 row)

Time: 0.427 ms
[test] # select test(7,3);
 test
------
   10
(1 row)

Time: 0.279 ms
[test] # select test(7,null,3);
 test
------
    ¤
(1 row)

Time: 0.236 ms
[test] # select test(7,0,3);
 test
------
   10
(1 row)

Time: 0.207 ms
[test] # select test(7,4,3);
 test
------
   14
(1 row)

Time: 0.362 ms
$ postgraphile -c postgres://localhost/test

test(
a: Int!
b: Int
c: Int
): Int

{
  test1: test(a: 7)
  test2: test(a: 7, b: 3)
  test3: test(a: 7, c: 3)
  test4: test(a: 7, b: 0, c: 3)
  test5: test(a: 7, b: 4, c: 3)
}

{
  "data": {
    "test1": 7,
    "test2": 10,
    "test3": null,
    "test4": 10,
    "test5": 14
  }
}

Works as expected, except the null result on omitting b (which is covered by different issue: graphile/graphile-build#87)