Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interfaces/Union #198

Open
jlukas99 opened this issue Jul 24, 2022 · 8 comments
Open

Interfaces/Union #198

jlukas99 opened this issue Jul 24, 2022 · 8 comments
Assignees

Comments

@jlukas99
Copy link

Hi, how do I need to create the tables to generate my interfaces and unions, or how do I edit the scheme file so I can implement this myself?

Zrzut ekranu 2022-07-24 o 10 19 11
Zrzut ekranu 2022-07-24 o 10 19 05

@olirice
Copy link
Contributor

olirice commented Sep 21, 2022

so sorry for the delay getting back to you! that isn't usual, I had notifications misconfigured

There will likely be a node interface once a global id has been implemented but one of the goals for pg_graphql is to be as configuration free as possible, so we don't have any plans to implement user defined interfaces.

as far as I can tell that is consistent with similar "reflect graphql from db" projects but if you're aware of any prior art e.g. from Graphile, Hasura etc, for how they do it I'd love to take a look!

@olirice olirice self-assigned this Sep 21, 2022
@bryanmylee
Copy link
Contributor

bryanmylee commented Jun 26, 2023

@olirice Just wanted to bring this up again, but it would be really useful to generate type unions based on PostgreSQL's table inheritance.

Assuming I have a person table and a employee table that inherits from it. These tables automatically generate the Person and Employee GraphQL types but makes no relationships between them. Ideally, the generated schema would look like:

type Employee implements Person {
  salary: Int!
}

which would allow a query returning [Person!]! to include Employee objects in the result and allow inline fragments to discriminate the types properly.

@bryanmylee
Copy link
Contributor

I would love to contribute to the project @olirice, but could use some help understanding where I should look with regards to the PSQL -> GraphQL type generation, and the GraphQL -> PSQL query transformation.

@olirice
Copy link
Contributor

olirice commented Jun 26, 2023

using table inheritance for type unions is a very interesting idea @bryanmylee

table inheritance is pretty out-of-fashion and I recall being told that its no longer recommended. I'll ask the postgres team and see if they have any strong opinions

There could be some edge cases around computed columns and views not being able to conform to the interface

any thoughts @alaister?


help understanding where I should look with regards to the PSQL -> GraphQL type generation

Contributions to the project would be great. This feature is pretty significant though so possibly not the best place to start:

These would be the steps:

  1. Update the query that loads the database schema state to track table inheritance https://github.com/supabase/pg_graphql/blob/master/sql/load_sql_context.sql (must not have significant impact on query performance)

  2. Update the relevant SQL types to deserialize it https://github.com/supabase/pg_graphql/blob/master/src/sql_types.rs

  3. Add a new generic __Type to represent inherited interfaces. You could model it after __Type::NodeInterface and NodeInterfaceType

  4. Add the interface/s to the relevant NodeTypes

    pg_graphql/src/graphql.rs

    Lines 1676 to 1689 in f07ac71

    fn interfaces(&self) -> Option<Vec<__Type>> {
    let mut interfaces = vec![];
    if self.table.primary_key().is_some() {
    interfaces.push(__Type::NodeInterface(NodeInterfaceType {
    schema: Arc::clone(&self.schema),
    }))
    }
    match interfaces.is_empty() {
    false => Some(interfaces),
    true => None,
    }
    }

I don't think any of the resolver logic would need to change off the top of my head since all the relevant fields are already present on the inherited types

@alaister
Copy link
Member

alaister commented Jun 27, 2023

Interfaces + Unions (Polymorphism) would certainly be a useful feature addition. However, I think inherited tables are not the best way of implementing them. Personally, I don't find them to be very ergonomic.

Perhaps we could hint to pg_graphql (via the @graphql({}) comment) that we want a type to appear as part of an interface/union. This is similar to how Postgraphile V5 is handling this.

@bryanmylee
Copy link
Contributor

I've only started using PostgreSQL extensively recently so I'm curious: what seems to be the issue with inherited tables?

@olirice
Copy link
Contributor

olirice commented Jun 27, 2023

IIRC its a bunch of little things

  • can negatively impact query performance when there are lots of child tables
  • constraints/indexes aren't inherited
  • can't cascade alter statements to child tables
  • a big fraction of its use-case was supplanted by declarative partitioning in pg 10
  • limited support in ORMS
  • can be complex

but if you know what you're doing I don't know of anything that's really wrong with inheritance in pg

@bryanmylee
Copy link
Contributor

bryanmylee commented Aug 13, 2023

Just as an update for others who have the same problem to solve, our team has gotten rid of table inheritance in favor of composed tables due to some glaring issues with inheritance.

The most egregious behavior was that entries in child tables will show up in queries on the parent table, but those entries will not be referencable in foreign keys, which broke our use case.

create table user (
  id uuid not null default gen_random_uuid (),
  constraint user_pkey primary key (id)
);

create table employee (
  name text not null
) inherits (user);

create table member (
  user_id uuid not null,
  constraint member_user_id_fk foreign key (user_id)
    references user (id) on delete cascade
);

insert into employee(id, name)
  values ('b28e7d43-d46f-4d20-8a06-db6342569553', 'Bryan');

select * from user; /**
+--------------------------------------+
|                                   id |
+--------------------------------------+
| b28e7d43-d46f-4d20-8a06-db6342569553 |
+--------------------------------------+
*/

insert into member(user_id)
  values ('b28e7d43-d46f-4d20-8a06-db6342569553'); /**
'b28e7d43-d46f-4d20-8a06-db6342569553' does not exist in foreign table user.
*/

Composition resolves all our use cases, and is compatible with pg_graphql's interface as it is now.

create table user (
  id uuid not null default gen_random_uuid (),
  constraint user_pkey primary key (id)
);

create table employee (
  id uuid not null default gen_random_uuid (),
  constraint employee_pkey primary key (id),
  -- the unique constraint allows pg_graphql to create a 1-to-1
  user_id uuid not null unique,
  constraint employee_user_id_fk foreign key (user_id)
    references user (id) on delete cascade,
  name text not null
) inherits (user);

create table member (
  user_id uuid not null,
  constraint member_user_id_fk foreign key (user_id)
    references user (id) on delete cascade
);

insert into user(id)
  values ('b28e7d43-d46f-4d20-8a06-db6342569553');
insert into employee(id, user_id, name)
  values (
    'b28e7d43-d46f-4d20-8a06-db6342569553',
    'b28e7d43-d46f-4d20-8a06-db6342569553',
    'Bryan'
  );

-- works fine now.
insert into member(user_id)
  values ('b28e7d43-d46f-4d20-8a06-db6342569553');
query {
  userCollection {
    edges { 
      node {
        employee { # any extra child fields from employees
          name
        }
      }
    }
  }
}

One small rough edge is that we have to insert records into two tables whenever we want to create new entries, but that's something that can be worked around, and #294 will solve the pain points eventually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants