crystal icon indicating copy to clipboard operation
crystal copied to clipboard

Smart comment `@nonNull` does not work for computed columns

Open bajtos opened this issue 3 years ago โ€ข 1 comments

Summary

I have a computed column that I would like to mark as non-null using the smart comment feature.

Based on the discussion in https://github.com/graphile/postgraphile/issues/906, the solution is to use the plugin @graphile-contrib/pg-non-null.

However, that plugin seems to be mostly abandoned. Quoting from https://github.com/graphile-contrib/pg-non-null/issues/2#issuecomment-851025078:

Hey there, this plugin is a bit obsolete. The @notNull smart comment is available in PostGraphile as a built-in. This plugin is mostly useful for its PgNonNullRelationsPlugin part.

Unfortunately, the built-in support for @nonNull does not seem to work for computed columns :(

Steps to reproduce

Using a slightly modified example from https://github.com/graphile-contrib/pg-non-null#usage

CREATE TABLE public.customers (
  id         serial PRIMARY KEY,
  first_name text NOT NULL,
  last_name  text
);

CREATE FUNCTION public.customers_full_name("user" public.customers) RETURNS text AS
$$
  SELECT "user".first_name || ' ' || "user".last_name
$$
LANGUAGE SQL STABLE;
COMMENT ON FUNCTION public.customers_full_name(public.customers) IS E'@nonNull';
$ postgraphile -c postgres://YOUR_CONNECTION_STRING --export-schema-graphql schema.graphql

Expected results

The generated GraphQL schema marks the field Customers.fullName as non-nullable.

type Customer implements Node {
  """
  A globally unique identifier. Can be used in various places throughout the system to identify this single value.
  """
  nodeId: ID!
  id: Int!
  firstName: String!
  lastName: String
  fullName: String!
}

Actual results

The generated GraphQL schema marks the field Customers.fullName as nullable.

type Customer implements Node {
  """
  A globally unique identifier. Can be used in various places throughout the system to identify this single value.
  """
  nodeId: ID!
  id: Int!
  firstName: String!
  lastName: String
  fullName: String
}

Additional context

To get the desired behaviour, I have to configure the @graphile-contrib/pg-non-null plugin.

$ postgraphile -c postgres://YOUR_CONNECTION_STRING --export-schema-graphql schema.graphql --append-plugins "@graphile-contrib/pg-non-null"

Possible Solution

It would be awesome if Postgraphile supported this feature out of the box so that I don't have to install that plugin.

The docs at https://www.graphile.org/postgraphile/why-nullable/#what-about-computed-fields say:

I'd be happy to accept a Pull Request that adds functionality marking a function as non-nullable via a smart comment (e.g. COMMENT ON FUNCTION foo_func(foo) IS E'@nonNull';) - do raise an issue if this is of interest to you.

I am happy to contribute to this feature if you could provide me with a few pointers on where to start looking in the codebase and where/how to add tests.

bajtos avatar Apr 05 '22 15:04 bajtos

At the moment weโ€™re prioritising getting V5 over the finish line; in the mean time consider using https://www.graphile.org/postgraphile/make-change-nullability-plugin/ instead to achieve this goal ๐Ÿ‘

benjie avatar Apr 05 '22 17:04 benjie

This works fine in V5, but the smart tag is @notNull (to match the database language) rather than non-null which would match GraphQL.

benjie avatar Sep 29 '23 15:09 benjie