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

Naming reverse related fields #502

Open
Aichnerc opened this issue Mar 19, 2024 · 8 comments
Open

Naming reverse related fields #502

Aichnerc opened this issue Mar 19, 2024 · 8 comments

Comments

@Aichnerc
Copy link

Aichnerc commented Mar 19, 2024

Summary

I have a table customers that contains two foreign keys (coach, partner), which reference its own table. Using GraphQL, I aim to design a query that would allow me to retrieve customers along with their associated coach and partner, both of which are also customers. However, the current GraphQL schema only provides a single customers field on customersEdge, which does not differentiate between coach and partner.

query getCustomers {
  customersCollection {
    edges {
      node {
        id
        name
        coach { <-- Should resolve the related customer
          name
        }
        partner { <-- Should resolve the related customer
          name
        }
      }
    }
  }
}

Instead, my query looks like this, combining both foreign keys under a single customers field without a clear way to distinguish between the two roles:

query getCustomers {
  customersCollection {
    edges {
      node {
        id
        name
        customers { <-- Resolves me just one foreign key. No way for me to seperate between coaches and partners.
          name
        }
      }
    }
  }
}

Is there a method or workaround to achieve the desired query structure?

Rationale

Creating distinct fields for coach and partner within the customers query is essential for clarity and to accurately represent the relationships within our data model. This would allow consumers of the GraphQL API to specifically request the data they need and understand the relationships between different customers.

Design

I do not have any known solutions.

Alternatives

  • Use a Custom Resolver: Not sure on how to to that though.
  • Database Views: Create database views that pre-join the necessary data. Increases complexity for one-to-many relationsships. Not suitable.
  • Supabase Functions: Not sure this is performant, nor the right approach.

Unresolved Questions

  • Schema Generation: How can auto-generated GraphQL schemas be adjusted to accurately represent these kinds of relationships?
  • Best Practices: Are there established best practices or patterns within the Supabase community for handling similar scenarios?
@olirice
Copy link
Contributor

olirice commented Mar 20, 2024

dupe of #345

the explanation of how to resolve is here: #345 (comment)

Previously we decided that the current behavior is expected but this has come up enough times we might need to revisit.

Do you have any suggestions for what behavior you'd find more intuitive?

@olirice
Copy link
Contributor

olirice commented Mar 20, 2024

for example we could do

customers and customers2 to make both accessible and nudge people to manually rename one or both of the constraints with a comment directive

a challenge there would be that if the foreign key under customers gets changed, customers2 would get renamed to customers which could be confusing

@Aichnerc
Copy link
Author

Thanks for the quick response!

I'm inclined towards the first option of making every relationship accessible and encouraging users to rename them. Is it feasible to display the actual column name in this process?

Additionally, I've scoured the documentation, searched online, reached out to the Supabase community, and even consulted GPT, but unfortunately, I couldn't find any information on this topic.

@olirice
Copy link
Contributor

olirice commented Mar 20, 2024

The solution is in docs here https://supabase.github.io/pg_graphql/configuration/#relationships-field but you're right that we should explicitly call out this issue as something it can address if we don't resolve it with the customers, customers2 solution in the next release

@pawarren
Copy link

pawarren commented Jul 7, 2024

I'm running into the same issue, but with unique partial indices, and I'm unable to resolve it using comment directives

(if I'm misunderstanding and this is better posted as a separate issue, let me know and I'm happy to move it!)

I have a table that looks something like this:

CREATE TABLE IF NOT EXISTS userspace.readthroughs (
    user_id UUID NOT NULL,
    work_id UUID NOT NULL,
    status userspace.READTHROUGH_STATUS NOT NULL,
    FOREIGN KEY (user_id) REFERENCES auth.users (id) ON DELETE CASCADE ON UPDATE CASCADE,
    FOREIGN KEY (work_id) REFERENCES catalog.works (work_id) ON UPDATE CASCADE,
    FOREIGN KEY (user_id, work_id) REFERENCES userspace.works (user_id, work_id) ON DELETE CASCADE ON UPDATE CASCADE,
);

When I look at userspaceWorks in the GraphiQL docs, I see:

userspaceReadthroughsCollection(
first: Int
last: Int
before: Cursor
after: Cursor
offset: Int
filter: userspaceReadthroughsFilter
orderBy: [userspaceReadthroughsOrderBy!]
): userspaceReadthroughsConnection!

So far so good. But now I want to add a unique partial index to ensure data integrity:

CREATE UNIQUE INDEX idx_unique_active_readthrough ON userspace.readthroughs (user_id, work_id)
WHERE status IN ('in_progress', 'want_to_read');

When I do that, and go back to userspaceWorks in the GraphiQL docs, userspaceReadthroughsCollection is gone and replaced with userspaceReadthroughs: userspaceReadthroughs!

This is exactly the same behavior I see when I run into name conflict issues with foreign key references (table1.foo references FIELD, and table1.bar also references FIELD), so I think this is caused by having both the (user_id, work_id) compound foreign key and the (user_id, work_id) as part of a unique partial index

...but I think the unique partial indices don't actually create an fkey, and comments on the indices seem to be ignored, so I'm actually unable to resolve this

I'm currently using triggers to ensure data integrity, but would of course prefer to use a much cleaner unique partial index.

@pawarren
Copy link

pawarren commented Jul 7, 2024

Also: I would've preferred customer, customer1 instead of simply colliding the names

This is also a great fit for another linter rule, I think

When I ran into this in the wild, it was a headache to chase down

I had a table like this:

CREATE TABLE IF NOT EXISTS userspace.list_items (
    list_id UUID NOT NULL,
    user_id UUID NOT NULL,
    work_id UUID DEFAULT NULL,
    FOREIGN KEY (list_id) REFERENCES userspace.lists (list_id) ON DELETE CASCADE ON UPDATE CASCADE,
    FOREIGN KEY (user_id, list_id) REFERENCES userspace.lists (user_id, list_id) ON DELETE CASCADE ON UPDATE CASCADE,
);

Then we decided to make it so lists could contain other lists, so we changed it to something like:

CREATE TABLE IF NOT EXISTS userspace.list_items (
    list_id UUID NOT NULL,
    user_id UUID NOT NULL,
    work_id UUID DEFAULT NULL,
    other_list_id UUID DEFAULT NULL,
    FOREIGN KEY (list_id) REFERENCES userspace.lists (list_id) ON DELETE CASCADE ON UPDATE CASCADE,
    FOREIGN KEY (other_list_id) REFERENCES userspace.lists (list_id) ON DELETE CASCADE ON UPDATE CASCADE,
    FOREIGN KEY (user_id, list_id) REFERENCES userspace.lists (user_id, list_id) ON DELETE CASCADE ON UPDATE CASCADE,
    FOREIGN KEY (user_id, other_list_id) REFERENCES userspace.lists (
        user_id, list_id
    ) ON DELETE CASCADE ON UPDATE CASCADE,
);

All of a sudden, lists broke

Took a while to chase down, but it was the same issue as above:

    FOREIGN KEY (list_id) REFERENCES userspace.lists (list_id) ON DELETE CASCADE ON UPDATE CASCADE,
    FOREIGN KEY (other_list_id) REFERENCES userspace.lists (list_id) ON DELETE CASCADE ON UPDATE CASCADE,

because they reference the same field, they collided in the GraphQL docs, and the "list_id" reference was silently overwritten by "other_list_id" reference

I had to add this so the two relationships were considered separate by pg_graphql:

COMMENT ON CONSTRAINT list_items_other_list_id_fkey ON "userspace"."list_items" IS E'@graphql({"local_name": "otherListId", "foreign_name": "otherList"})';

Similarly, I had to do the same for the compound fkey: COMMENT ON CONSTRAINT list_items_user_id_other_list_id_fkey ON "userspace"."list_items" IS E'@graphql({"local_name": "userOtherList", "foreign_name": "useOtherList"})';

I definitely would've preferred that the two relationships were considered separate by in pg_graphql

And this also feels like something that would be a good warning in a linter, "hey you have two fkeys in the same table pointing to the same field, consider adding a comment directive to distinguish them" (or to rename them)

@olirice
Copy link
Contributor

olirice commented Jul 8, 2024

@pawarren this sounds like, at minimum, we may be handling partial unique indexes as fully unique indexes.

Could you please try to create a minimal reproducible example and open a separate issue for it. You covered a lot of ground in the explanation there. The more narrowly focused the issue is the easier it'll be for us to track down

@pawarren
Copy link

Done!

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

3 participants