prisma / prisma-engines

🚂 Engine components of Prisma ORM
https://www.prisma.io/docs/concepts/components/prisma-engines
Apache License 2.0
1.14k stars 218 forks source link

Warn about PostgreSQL FDW on introspection #4441

Open JEbertPrime opened 7 months ago

JEbertPrime commented 7 months ago

This issue is for tracking an item in https://github.com/prisma/prisma/issues/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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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

Druue commented 6 months ago

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:

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