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

Warn about PostgreSQL FDW on introspection #4441

Open
JEbertPrime opened this issue Nov 14, 2023 · 4 comments
Open

Warn about PostgreSQL FDW on introspection #4441

JEbertPrime opened this issue Nov 14, 2023 · 4 comments

Comments

@JEbertPrime
Copy link

This issue is for tracking an item in prisma/prisma#16311 (discussed in depth here), related to foreign data wrappers in PostgreSQL databases.

The problem

Foreign tables in PostgreSQL databases occasionally have different properties from normal tables. As such they are left out of the query in schema-engine/sql-schema-describer/src/postgres/tables_query.sql during introspection, but ideally the user would be warned when a foreign table is left out.

The solution

#3953 is a similar PR, and the same general process can probably be implemented for this. Foreign tables can be queried for in the database and identified, and a warning message can notify users that they are present in the database but won't be represented in their schemas.

I'm not particularly savvy with rust, but this process seems straightforward enough that I'd be happy to submit a PR if nobody more experienced has the time - I know the issue on the prisma repo is referenced on the roadmap, but it seems pretty low priority so I'd be happy to take it on

@janpio janpio self-assigned this Nov 14, 2023
@janpio
Copy link
Member

janpio commented Nov 14, 2023

Go for it! We currently do not have the time to work on these warnings, or "stopgaps" as we call them internally. So if you find a good way to identify a FDW during Introspection, the pumbling from #3953 for the warnings should be the perfect approach.

@JEbertPrime
Copy link
Author

Last week I found #4020, which seems like it implements the stopgap (but is failing a test on an old version of postgres). Not sure if this is the only reason it wasn't merged, possibly it was forgotten because it's not associated with the right related issue. Anyways, I'm gonna get it to pass rather than building out my own solution

@janpio
Copy link
Member

janpio commented Nov 30, 2023

That sounds like a reasonable assumption. @Druue, can you provide context?

@Druue
Copy link
Contributor

Druue commented Dec 14, 2023

Hey @JEbertPrime so I tried to re-build my context for this 😅.
Looking at that PR, I can't currently see why it was dropped there. I did some investigation into the Postgres9 test failure and I can't see why it's failing.

Prisma minimally supports Postgres v9.6 and Foreign Data Wrappers reached full read-write support in version v9.3 so maybe there's a difference with how FDW's are interpreted in versions 9 and 10+. (Also, even in the context of a Prisma v4 world, we minimally supported Postgres v9.4 which also already had full FDW support, so really extra unsure 🤷)

They currently render as follows:

/// The underlying table does not contain a valid unique identifier and can therefore currently not be handled by Prisma Client.
/// This model represents a foreign data wrapper which requires additional setup for migrations. Visit https://pris.ly/d/fdw for more info.
// model bar {
// id Int @default(autoincrement())
// @@ignore
// }

Next step to me would be:

  • Investigate the failure on 9.6 and check why it's not rendering

Feel free to reach out if you have further questions :)

@Druue Druue self-assigned this Dec 14, 2023
@apolanc apolanc unassigned janpio and Druue Mar 8, 2024
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