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

me: implement multi-schema reset on postgres #3459

Merged
merged 2 commits into from Dec 2, 2022
Merged

Conversation

tomhoule
Copy link
Contributor

@tomhoule tomhoule commented Dec 1, 2022

Before this commit, the migration engine reset command resets the schema in the search path, on postgresql and cockroachdb.

This is not the expected behaviour when working with multiple schemas defined in the schemas datasource property: users expect all schemas to be reset, moreover migrate dev will be broken if we do not do that, because it expects reset to do its job before re-applying migrations.

In this commit, we take into account the namespaces defined in the Prisma schema the migration engine is initialized with. We reset these schemas, when defined, in addition to the schema in the search path. The reason we still the search path into account is because that is where we create and expect the _prisma_migrations table to live. I expect we may want to revisit that design choice
(issue: prisma/prisma#16565).

closes prisma/prisma#16561

Before this commit, the migration engine `reset` command resets the
schema in the search path, on postgresql and cockroachdb.

This is not the expected behaviour when working with multiple schemas
defined in the `schemas` datasource property: users expect all schemas
to be reset, moreover migrate dev will be broken if we do not do that,
because it expects reset to do its job before re-applying migrations.

In this commit, we take into account the namespaces defined in the
Prisma schema the migration engine is initialized with. We reset these
schemas, when defined, *in addition to* the schema in the search path.
The reason we still the search path into account is because that is
where we create and expect the _prisma_migrations table to live. I
expect we may want to revisit that design choice
(issue: prisma/prisma#16565).

closes prisma/prisma#16561
@@ -71,6 +71,7 @@ impl SchemaAssertion {
}
}

#[track_caller]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This puts the error on the right line in the test, instead of inside the assertion function.

@tomhoule tomhoule added this to the 4.8.0 milestone Dec 1, 2022
@tomhoule tomhoule marked this pull request as ready for review December 1, 2022 07:23
@tomhoule tomhoule requested a review from a team as a code owner December 1, 2022 07:23
In multi-schema mode, instead of wiping out the schema from the search
path in addition to the schemas defined in the datasource, we now only
drop the migrations table in the main schema. This should match migrate
dev's expectations around resets.
Copy link
Contributor

@pimeys pimeys left a comment

Choose a reason for hiding this comment

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

Bigger review happened in a zoom call. We got into a conclusion how this should work, and I just approve the code style here.

@tomhoule tomhoule merged commit f8add13 into main Dec 2, 2022
@tomhoule tomhoule deleted the migrate-reset-schema-arg branch December 2, 2022 13:49
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.

multiSchema: migrate reset only resets first schema
3 participants