prisma / prisma

Next-generation ORM for Node.js & TypeScript | PostgreSQL, MySQL, MariaDB, SQL Server, SQLite, MongoDB and CockroachDB
https://www.prisma.io
Apache License 2.0
38.97k stars 1.53k forks source link

Running `prisma migrate dev --create-only` actually applies previous (non-deployed) migrations #11184

Open manassra opened 2 years ago

manassra commented 2 years ago

Bug description

Imagine a situation where you are iterating on your schema, and using the following command to create the migration files without actually applying any of them.

prisma migrate dev --create-only

Suppose you initially run it the first time where you want to add migration A. When you run it a second time to add migration B, you get the following output from the same command:

The following migration(s) have been applied:

migrations/
  └─ 20220114234207_add_A/

This is problematic as the user may not have intended to actually apply migration A

How to reproduce

1) Modify your schema 2) Run prisma migrate dev --create-only to create the migration file for the change 3) Modify your schema again 4) Run prisma migrate dev --create-only to create the new migration file for the second change

Expected behavior

Either:

Prisma information

In my test, each run of the prisma migrate dev --create-only command was adding a new column to existing tables in my schema.

Environment & setup

Prisma Version

prisma                : 2.30.2
@prisma/client        : 2.30.2
Current platform      : darwin
Query Engine (Binary) : query-engine b8c35d44de987a9691890b3ddf3e2e7effb9bf20 (at node_modules/@prisma/engines/query-engine-darwin)
Migration Engine      : migration-engine-cli b8c35d44de987a9691890b3ddf3e2e7effb9bf20 (at node_modules/@prisma/engines/migration-engine-darwin)
Introspection Engine  : introspection-core b8c35d44de987a9691890b3ddf3e2e7effb9bf20 (at node_modules/@prisma/engines/introspection-engine-darwin)
Format Binary         : prisma-fmt b8c35d44de987a9691890b3ddf3e2e7effb9bf20 (at node_modules/@prisma/engines/prisma-fmt-darwin)
Default Engines Hash  : b8c35d44de987a9691890b3ddf3e2e7effb9bf20
Studio                : 0.422.0
AntoniusGolly commented 2 years ago

I find it very problematic that Prisma does not separate the commands for "generate a migration" and "run all queued migrations" more clearly. Compare TypeOrm:

$ typeorm migration:generate -n PostRefactoring // to generate
$ typeorm migration:run // run all queued

I don't intent to simply blaming Prisma for lacking features that I'm used to. But this is relevant due to a typical workflow. I disagree hereby with what is called a "typical development workflow" here.

  1. Developer makes changes to schema
  2. Generates SQL migration file
  3. might "visually" check SQL file
  4. checks in migration file
  5. CI pipeline executes migration file on production database (at some point)

Surely, generating and excecuting are separated things in terms of the workflow, as a developer is not to change a schema of a production database from console.

Currently, Prisma has a combination of these commands behind prisma migrate dev.

Yes, we could use --create-only flag for step 2. and not use the flag in step 5., but:

a. it feels unnatural to use a flag for something so fundamental for a developer to do b. it has the above mentioned (by @manassra) downsides of accidentally execute migrations c. it could prompt the CI command to give a name to the migration in case there are diffs d. I don't want the CI pipeline to analyze a database schema and calculate the diffs. Also not very efficient. I just want it to blindly execute prior tested migration files if not already applied. I can use prisma migrate deploy for this.

IMO not separating concerns nicely. Please, help with this development flow.

janpio commented 2 years ago

We agree more and more with your view @AntoniusGolly and are collecting input on how we can get there. Thanks for spelling it out so clearly.

stephyswe commented 2 years ago

Can we run a custom migration file ? If we change it . can you run that ?

Example. This is my order Table

-- CreateTable
CREATE TABLE "Order" (
    "id" SERIAL NOT NULL,
    "customerId" INTEGER NOT NULL,
    "driverId" INTEGER NOT NULL,
    "restaurantId" INTEGER NOT NULL,
    "total" INTEGER NOT NULL,
    "status" "OrderStatus" NOT NULL DEFAULT 'Pending',

    CONSTRAINT "Order_pkey" PRIMARY KEY ("id")
);

If I change field "driverId" and remove "NOT NULL". can I run that?

-- CreateTable
CREATE TABLE "Order" (
    "id" SERIAL NOT NULL,
    "customerId" INTEGER NOT NULL,
    "driverId" INTEGER,
    "restaurantId" INTEGER NOT NULL,
    "total" INTEGER NOT NULL,
    "status" "OrderStatus" NOT NULL DEFAULT 'Pending',

    CONSTRAINT "Order_pkey" PRIMARY KEY ("id")
);
janpio commented 2 years ago

That sounds like an unrelated questions @stephyswe - please open a new issue so we can debug the original bug here. Thanks.

JonathanWilbur commented 1 year ago

I just found this problem as well. My local database is down and Prisma is giving me:

$ npx prisma migrate dev --create-only --schema apps/meerkat/src/prisma/schema.prisma
Environment variables loaded from .env
Prisma schema loaded from apps/meerkat/src/prisma/schema.prisma
Datasource "db": MySQL database "directory" at "localhost:3306"

Error: P1001: Can't reach database server at `localhost`:`3306`

Please make sure your database server is running at `localhost`:`3306`.

Why does it need access to my local database? I think this is a bug.

janpio commented 1 year ago

Because migrate dev uses a local temporary database to generate the migrations: https://www.prisma.io/docs/concepts/components/prisma-migrate/shadow-database

JonathanWilbur commented 1 year ago

Okay. Well that settles the issue for me, then. Thank you for clarifying, @janpio !

Druue commented 1 year ago

Can confirm that this is still the case:

image
janpio commented 1 year ago

Note: Although we marked this as a confirmed bug, we currently think this is the intended design, but might be looking into revisiting this design decision in the future.

andyfleming commented 1 year ago

It's too easy to run npx prisma migrate dev without knowing what effects are going to happen. Some actions have safeguards, but even still, I agree that generating a migration and applying migrations should be separate commands.

olivierlacan commented 10 months ago

I think migrate dev does too much for one command. It's really three different commands:

This would make it less confusing than passing flags to migrate dev to disable the running migrations on the database bit (--create-only ) and then having to re-run the whole command (which is likely to have side-effects, documented in this issue and others) just to run the previously created SQL migration.

I don't want Prisma to re-evaluate the schema.prisma when I ask it to run a migration. It's fine for a shortcut command like migrate dev to do that, but that's why I'd argue you need new independent commands, not just flags to modify existing ones.

I think it's reasonable for prisma generate to run when migrate run and migrate dev are called, but not in other cases. If that makes sense.

PS: As a sidenote, it's a bit weird that you can't call prisma generate without the import help output from the CLI, while migrate dev is obviously running it behind the scenes.

mbsanchez01 commented 9 months ago

Any news regarding this topic? I have the same issue, and it is very annoying

fern-sp commented 2 weeks ago

Any news on this? Having a "generate migration" command have unintended side-effects is dangerous, and is definitely a conceptual bug even that's how you implemented it. :(

This is a big pain point, since we develop on a shared db, and we easily break it for other developers when it applies temporary migrations that we never meant to apply.

Along the "generate migration" command to give the user the option to "reset" the whole database. So dangerous.