Better domain type support

[andrew-w-ross]

Describe the bug

Types for scalar domain types return type Opaque and for array domain types they just fail.

To Reproduce

Given the following schema:

create domain pos_int as int check (value > 0);
create domain non_empty_array as int[] check (array_length(value, 1) > 0);
create domain pos_array as pos_int[];

create table domain_test(
    id serial primary key,
    int_field int,
    array_field int[],
    pos_int pos_int,
    non_empty_array non_empty_array,
    pos_array pos_array
);

insert into domain_test(int_field, array_field, pos_int, non_empty_array, pos_array)
values (1, '{1,2,3}', 1, '{1,2,3}', '{1,2,3}');

Querying for the types on domain_test returns the following:

select graphql.resolve($$
query {
  __type(name: "domain_test") {
    fields {
      name
      type {
        name
        kind
        ofType {
          name
          kind
          ofType {
            name
            kind
          }
        }
      }
    }
  }
}
$$);

{
  "data": {
    "__type": {
      "fields": [
        {
          "name": "nodeId",
          "type": {
            "kind": "NON_NULL",
            "name": null,
            "ofType": {
              "kind": "SCALAR",
              "name": "ID",
              "ofType": null
            }
          }
        },
        {
          "name": "id",
          "type": {
            "kind": "NON_NULL",
            "name": null,
            "ofType": {
              "kind": "SCALAR",
              "name": "Int",
              "ofType": null
            }
          }
        },
        {
          "name": "int_field",
          "type": {
            "kind": "SCALAR",
            "name": "Int",
            "ofType": null
          }
        },
        {
          "name": "array_field",
          "type": {
            "kind": "LIST",
            "name": null,
            "ofType": {
              "kind": "SCALAR",
              "name": "Int",
              "ofType": null
            }
          }
        },
        {
          "name": "pos_int",
          "type": {
            "kind": "SCALAR",
            "name": "Opaque",
            "ofType": null
          }
        }
      ]
    }
  }
}

Querying the data will work for the fields on the Opaque types:

select graphql.resolve($$
query {
  domain_testCollection{
    edges {
        node {
            id
            int_field
            array_field
            pos_int
        }
    }
  }
}
$$);

{
  "data": {
    "domain_testCollection": {
      "edges": [
        {
          "node": {
            "id": 1,
            "pos_int": 1,
            "int_field": 1,
            "array_field": [
              1,
              2,
              3
            ]
          }
        }
      ]
    }
  }
}

And fails for fields with an array domain_type:

select graphql.resolve($$
query {
  domain_testCollection{
    edges {
        node {
            id
            non_empty_array
        }
    }
  }
}
$$);

{
  "data": null,
  "errors": [
    {
      "message": "Unknown field 'non_empty_array' on type 'domain_test'"
    }
  ]
}

You can attempt the above with pos_array and get a similar result.

Expected behavior

For fields with domain types to resolve to there domain type or at least the domain base type.

E.g.

select graphql.resolve($$
query {
  __type(name: "domain_test") {
    fields {
      name
      type {
        name
        kind
        ofType {
          name
          kind
          ofType {
            name
            kind
          }
        }
      }
    }
  }
}
$$);

Would return:

{
  "data": {
    "__type": {
      "fields": [
        {
          "name": "nodeId",
          "type": {
            "kind": "NON_NULL",
            "name": null,
            "ofType": {
              "kind": "SCALAR",
              "name": "ID",
              "ofType": null
            }
          }
        },
        {
          "name": "id",
          "type": {
            "kind": "NON_NULL",
            "name": null,
            "ofType": {
              "kind": "SCALAR",
              "name": "Int",
              "ofType": null
            }
          }
        },
        {
          "name": "int_field",
          "type": {
            "kind": "SCALAR",
            "name": "Int",
            "ofType": null
          }
        },
        {
          "name": "array_field",
          "type": {
            "kind": "LIST",
            "name": null,
            "ofType": {
              "kind": "SCALAR",
              "name": "Int",
              "ofType": null
            }
          }
        },
        {
          "name": "pos_int",
          "type": {
            "kind": "SCALAR",
            "name": "Int",
            "ofType": null
          }
        },
        {
          "name": "non_empty_array",
          "type": {
            "kind": "LIST",
            "name": null,
            "ofType": {
              "kind": "SCALAR",
              "name": "Int",
              "ofType": null
            }
          }
        },
        {
          "name": "pos_array",
          "type": {
            "kind": "LIST",
            "name": null,
            "ofType": {
              "kind": "SCALAR",
              "name": "Int",
              "ofType": null
            }
          }
        }
      ]
    }
  }
}

The behavior of the field itself on querying would be the same as it's base type.

Screenshots
N/A

Versions:

• PostgreSQL: 15
• pg_graphql commit ref: ec3f7d5

Additional context
I think I have a possible fix for this and I'l like to try opening a pr to fix this.

A simple solution would be to resolve the base type of the domain object itself.
To my understanding domain types always have to eventually resolve to a built in sql scalar or array type so this seems like a valid solution.
While this would technically be a breaking change Opaque isn't particularly useful so it could also hide behind a schema directive flag but that would be opt out. This is also a possible fix for #370.

Let me know if you agree and I'll submit a pr.

The complete fix would be to generate new types for domain types.
That is breaking change and some client might not care to know about anything but the base type. It'll have to be behind a schema directive flag that's opt in.
I won't be attempting this approach right away just spitballing a final solution.

[olirice]

Resolving to the base type of the domain makes sense to me

While this would technically be a breaking change

Moving from Opaque to a descriptive type is non-breaking. Opaque types use default postgres json serialization (vs always a string etc) and support the fewest operations (like filtering) for that reason

What breakage were you expecting?

[olirice]

Although I guess a domain based on a bigint (which we serialize as a string vs the default of an int) or json (doesn't support equality but Opaque does) could introduce a data format and API change respectively

[andrew-w-ross]

Good point I didn't even think about bigint.

I was mainly thinking about fields that weren't surfaced before, domain array types, will now all of a sudden show up. On a lesser extent it's a contract change for users doing introspection on the graphql types and have made workaround for Opaque.

I might be overly safe that's why I was going to put it behind a opt out schema config and then that could be removed on the next major release. It's your call just want to get requirements down before working on a pr.

[olirice]

yeah you're right. I scanned some of the public facing schemas on the platform and there would be impact

behind a opt out schema config

a schema level comment directive to enable domain types sounds great

If you create a PR, please also add it to the 2.0 breaking changes checklist

thanks!

[andrew-w-ross]

Will do, just to confirm I would like to do this in two pr's.

The first will be a very simple resolve to the base types for domain types.
In the example I posted it would be a simply report and resolve to there base types.

E.g. pos_int would just be int and pos_array would be int[].

Putting the resolve base type behind a opt out of flag just in case someone is reliant on the old behaviour.
This pr would be relatively quick and doesn't need a lot of documentation.
Personally this partially unblocks my work and would also be a possible fix for #370.

The second pr would take a while longer and probably needs some further discussion and that would be to surface the domain types.

e.g. pos_int would resolve to pos_int ( or should it be PosInt?) on introspection and add it as a scalar type

This probably be opt in just in case clients only care about the base types.
I'll raise a new issue for this once the resolve base type pr is in to hash out the particulars.

Does that work for you?

[olirice]

To roll it out safely I think

• PR A: array domains added to schema as [Opaque] (non-breaking, comment directives)
• PR B: schema level comment directive to opt-in that changes scalar domains and array domains into their base types

and in version 2.0 we remove the schema directive and make that default behavior

---

The second pr would take a while longer and probably needs some further discussion and that would be to surface the domain types.

e.g. pos_int would resolve to pos_int ( or should it be PosInt?) on introspection and add it as a scalar type

there would be no equivalent way to declare the array of positive integer example so I'm not sold on using the domain's name directly, but I'd love to hear any ideas you have

[andrew-w-ross]

there would be no equivalent way to declare the array of positive integer example so I'm not sold on using the domain's name directly, but I'd love to hear any ideas you have

I haven't given this too much thought yet but it could be embedded in a directive.

Something like directive @DomainType(typename: String!, elementType: String) on FIELD_DEFINITION.

And then it could be defined on the schema like so:

directive @DomainType(type: String!, elementType: String) on FIELD_DEFINITION

type domain_test {
  id: ID!
  
  pos_int: Int @DomainType('pos_int`)
  
  pos_int: [Int!] @DomainType('pos_array`,`pos_type`)
}

I haven't even tried this out so I don't know if it's valid or how the introspection would work.
Might be a better solution for pr B as the type information is available for those that care and the types look the same for those that don't.

[imor]

I tried the same example above so see what other tools do. PostGraphile emits the following type:

type DomainTest {
  id: Int!
  intField: Int
  arrayField: [Int]
  posInt: PosInt
  nonEmptyArray: [Int]
  posArray: [PosInt]
}

Which looks like quite a reasonable solution to me.

there would be no equivalent way to declare the array of positive integer example so I'm not sold on using the domain's
name directly, but I'd love to hear any ideas you have

I don't fully understand what is the concern here? @olirice could you please show an example?

Hasura produces the following type (which I don't like at all):

type domain_test {
  array_field: [Int!]
  id: Int!
  int_field: Int
  non_empty_array: _int4
  pos_array: _pos_int
  pos_int: Int
}

[dvv]

I attempted to step down to base domain types here master...dvv:pg_graphql:master
It can help variables validation (String != Opaque) while we're thinking over the better domain type support.