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

feat(schema-engine): use pk for implicit join tables on postgres #5057

Merged
merged 24 commits into from
Nov 28, 2024

Conversation

aqrln
Copy link
Member

@aqrln aqrln commented Nov 26, 2024

Use primary key constraint rather than a unique constraint on the (A, B) pair in implicit m:n join tables on PostgreSQL.

We gate this change to PostgreSQL only because this fixes a Postgres-specific issue with logical replication, and is not that useful for other providers, while having more complicated migration path or write performance or storage implications in other databases. On Postgres this change is fairly non-intrusive (except the one time cost of rebuilding the index and having two indexes for a short period of time).

Migrate normally generates all ALTER TABLE statements after the DROP INDEX statements. To ensure the migration is safe, when we detect that a unique index is "promoted" to a primary key (in any table, not necessarily just in this case), we reorder the DROP INDEX statement after the primary key is created. Otherwise creating the primary key may fail if non-unique records are inserted in between, since Prisma Migrate doesn't use transactions by default (except on SQL Server).

Fixes: prisma/prisma#25196

Copy link

codspeed-hq bot commented Nov 26, 2024

CodSpeed Performance Report

Merging #5057 will not alter performance

Comparing implicit-m2m-id (b95bef8) with main (1886abc)

Summary

✅ 11 untouched benchmarks

Copy link
Contributor

github-actions bot commented Nov 26, 2024

WASM Query Engine file Size

Engine This PR Base branch Diff
Postgres 2.049MiB 2.049MiB 0.000B
Postgres (gzip) 821.939KiB 821.940KiB -1.000B
Mysql 2.013MiB 2.013MiB 0.000B
Mysql (gzip) 807.917KiB 807.917KiB -1.000B
Sqlite 1.910MiB 1.910MiB 0.000B
Sqlite (gzip) 768.140KiB 768.139KiB 1.000B

@aqrln aqrln added this to the 6.0.0 milestone Nov 27, 2024
@aqrln aqrln marked this pull request as ready for review November 28, 2024 12:15
@aqrln aqrln requested a review from a team as a code owner November 28, 2024 12:15
@aqrln aqrln requested review from wmadden and removed request for a team November 28, 2024 12:15
@jkomyno jkomyno self-requested a review November 28, 2024 13:14
Comment on lines +551 to +554
// automatically derived total order relation between `SqlMigrationStep`s). If we need more
// complex ordering logic in the future, we should consider defining a partial order on
// `SqlMigrationStep` where only the pairs for which order actually matters are ordered,
// building a graph from the steps and topologically sorting it.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Topological graph + explicit partial order would probably be the best course of action in the long term.
Currently, we need to manually be careful when defining the operations that impact the schema.prisma <-> SQL translation.

Copy link
Contributor

@jkomyno jkomyno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one!

@aqrln aqrln merged commit 5dbef10 into main Nov 28, 2024
367 checks passed
@aqrln aqrln deleted the implicit-m2m-id branch November 28, 2024 15:37
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

Successfully merging this pull request may close these issues.

Prisma join tables unable to be replicated by default
3 participants