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

Table can't reference another table twice #188

Closed
bacebu4 opened this issue Oct 16, 2022 · 4 comments · Fixed by #191
Closed

Table can't reference another table twice #188

bacebu4 opened this issue Oct 16, 2022 · 4 comments · Fixed by #191
Labels
bug Something isn't working

Comments

@bacebu4
Copy link
Contributor

bacebu4 commented Oct 16, 2022

Given the following schema

CREATE TABLE quotes (
  id INTEGER PRIMARY KEY,
  quote TEXT NOT NULL,
  said_by VARCHAR(255) NOT NULL,
  created_at DATETIME DEFAULT CURRENT_TIMESTAMP
);

CREATE TABLE movies (
  id INTEGER PRIMARY KEY,
  name TEXT NOT NULL UNIQUE
);

ALTER TABLE quotes ADD COLUMN movie_id INTEGER REFERENCES movies(id);
ALTER TABLE quotes ADD COLUMN another_movie_id INTEGER REFERENCES movies(id);

I can't fetch another_movie entity from graphQL. By the way, it's a real case scenario, table names were changed on purpose

Tried finding a problem by myself but haven't got a clue where to start

@mcollina mcollina added the bug Something isn't working label Oct 16, 2022
@bacebu4
Copy link
Contributor Author

bacebu4 commented Oct 16, 2022

Ok, I found out the line that can be fixed

https://github.com/platformatic/platformatic/blob/main/packages/sql-graphql/lib/relationship.js#L26

It should probably rely on relation.column_name

The more harder topic – how should we determine how we should call that relation? It's easy if it's called some_entity_id (we can simply cut out the _id part)
But what is someone not following this guideline? I suppose this framework doesn't want to insist on that

So there could be a numerous solutions but I have no idea which one better suits this framework

Also would be glad to help if needed :)

@bacebu4
Copy link
Contributor Author

bacebu4 commented Oct 16, 2022

Inverse side of the relation is also gonna be broken in that case

movies {
    id
    quotes {
      id
    }
  }

quotes will only look for another_movie_id references

@mcollina
Copy link
Member

A PR would be awesome, thanks!

As for naming, I would recommend using the same logic of the other properties.

@bacebu4
Copy link
Contributor Author

bacebu4 commented Oct 16, 2022

A PR would be awesome, thanks!

As for naming, I would recommend using the same logic of the other properties.

Got you, I'll let you know if I'm gonna get stuck again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants