supabase / cli

Supabase CLI. Manage postgres migrations, run Supabase locally, deploy edge functions. Postgres backups. Generating types from your database schema.
https://supabase.com/docs/reference/cli/about
MIT License
1.07k stars 209 forks source link

supabase migration squash deletes storage schema migration script #1493

Closed Brentably closed 7 months ago

Brentably commented 1 year ago

Describe the bug The supabase migration squash command is supposed to flatten all the migration scripts to one file. It seems to delete one of my storage policies.

To Reproduce Pull migration script, pull storage script with supabase db pull --schema storage, run supabase migration squash

Expected behavior It should should flatten all of the logic into one file, not delete the logic.

Screenshots https://www.loom.com/share/fe6854a529044e2aba4d16e1046ebcbc

Desktop (please complete the following information):

Additional context Add any other context about the problem here.

bombillazo commented 1 year ago

Maybe adding new flag -s like with db diff to the squash command to specify the schemas would work.

bombillazo commented 9 months ago

Any update?

pidanou commented 8 months ago

This issue just makes the squash action pretty much useless at best and dangerous at worse when you're working with the storage and auth schemas Imagine squashing for your first version and deploying to production and you miss triggers or even worse RLS policies...

Am i missing something here?

pidanou commented 8 months ago

Is there a reason why supabase migration squash doesn't just merge the migration files together?

sweatybridge commented 8 months ago

Is there a reason why supabase migration squash doesn't just merge the migration files together?

If you have been working on a project for a long time, chances are there will be migrations that repeatedly change the same column, function, etc. The value proposition for squashing is to get rid of these intermediate migrations so that you can migrate once to reach the final state of the desired database.

On capturing changes in auth and storage schemas, I think we can make a simplifying assumption here that such changes do not introduce new columns to the tables defined by auth and storage service. That way, we can diff statements from pg_dump directly before and after running migrations.

I will open a PR soon to try out this approach. Would appreciate your feedback once it's released to beta channel.

sweatybridge commented 8 months ago

Please give npx supabase@beta migration squash a try and let me know if you run into any problems.

I will keep this issue open for another week or so to collect feedback.

pidanou commented 8 months ago

@sweatybridge I only have triggers on auth and storage schemas so it's not a full test but from what I see on my end it looks great

opack commented 8 months ago

@sweatybridge Same here: I had the very same problem and using the beta version correctly added my triggers in the squashed file 🥳

opack commented 8 months ago

@sweatybridge That being said, it seems that statements like the following are lost after squashing 😩

alter publication supabase_realtime add table public.posts;
opack commented 8 months ago

If someone has the same issues as we do (with the squash not gathering the alter publication statements), it is indeed logic because when I think about it, the migration system seems to ignore these statements. Seeing how the squash works (applying the migrations and then diffing the schema), it's logic that the alter publication) statements are ignored during this process too.

To work around this, here is what we did:

  1. Create a file supabase/migrations/99999999999999_publications.sql. It will contain all our publication-related statements (typically the alter publication supabase_realtime...). The fake timestamp guarantees that these statements are executed last, after all other database modifications are performed.
  2. Create a new script in our package.json:
    "supabase:migration-squash": "npx rimraf supabase/migrations/99999999999999_publications.sql && supabase migration new squashed && npx supabase@beta migration squash --local && git restore supabase/migrations/99999999999999_publications.sql",

    This script does 4 things:

    1. Remove the file containing all the publication-related statements: it is necessary because otherwise the squash will overwrite this file as it is the last migration file.
    2. Create a new empty migration file named squashed so that the squashed result is put in this file (a --file parameter to the supabase migration squash command would be nicer)
    3. Perform the squash (using the beta version 😉 until a new version is published)
    4. Restore the publication-related SQL file. We will now use the npm run supabase:migration-squash command to perform migrations.
bombillazo commented 8 months ago

For some reason, the generated squash file truncates the first parts of the storage/auth schema functions, right after the

--
-- Dumped schema changes for auth and storage
--

statement. They start at some random point. Its as if there was a limit of characters and only the last X amount of SQL schema lines were output to the squashed file.

Probably has something to do with this: https://github.com/supabase/cli/pull/2027/files#diff-a823cea48b5eddc5a9344873fa04fa3fe06df2276fd9e99348bb25d5f1c079a0R138

bombillazo commented 8 months ago

Also, the squashed migration now contains the DDL SQL statements for storage and auth schema, so when generating a new migration after the squashed migration, it seems some internal process is priorly creating those schemas. Then, my squashed migration schema re-runs the auth/storage schema changes, and errors start popping up.

sweatybridge commented 8 months ago

Hmm could you share with me the entities you've changed in auth and storage schemas? I will try to reproduce.

The reason it truncates is probably due to line by line diff detecting a change in Postgres function definition. But typically you don't want to change any internal functions defined by auth and storage.

bombillazo commented 8 months ago

We haven't changed any of the entities. The squashed migration is trying to contain all the storage/auth schema SQL regardless of whether we changed it as if it was going to create everything from scratch.

With this single file, we are attempting to run supabase db diff to check for any remaining schema changes, but the command fails as soon as the new squashed migration file is applied to the shadow database.

sweatybridge commented 8 months ago

May I double check the version of Postgres you are running? I think the new squash wouldn't work with 15.1.0.55 or below.

bombillazo commented 8 months ago

image

bombillazo commented 8 months ago

I ran the diff command in debug, and i am seeing these statements in the console:

2024/03/15 11:55:46 PG Recv: {"Type":"NoData"}
2024/03/15 11:55:46 PG Recv: {"Severity":"NOTICE","SeverityUnlocalized":"NOTICE","Code":"42P07","Message":"relation \"buckets\" already exists, skipping","Detail":"","Hint":"","Position":0,"InternalPosition":0,"InternalQuery":"","Where":"","SchemaName":"","TableName":"","ColumnName":"","DataTypeName":"","ConstraintName":"","File":"parse_utilcmd.c","Line":209,"Routine":"transformCreateStmt","UnknownFields":null}
2024/03/15 11:55:46 PG Recv: {"Type":"CommandComplete","CommandTag":"CREATE TABLE"}

It is as if the shadow database already has the auth/storage schemas set up before the squashed migration runs, so it will conflict when the squashed migration attempts to "recreate" those schemas.

sweatybridge commented 8 months ago

So the squashed migrations should not contain any entities already defined by storage because it is a supabase managed schema. That's why I suspect there might be some conflicting changes in the migrations before they were squashed. Was any of your earlier migration created by db pull? Perhaps the pulled schema conflicted with the squash.

bombillazo commented 8 months ago

Maybe, this is an old schema and we have over 40 migration files, let me check our migration history to see if we did somehow generated the schema DDL for those and added them to our history..

So to be clear, we should not have any DDL SQL related to the creation of entities or the auth/storage schemas in our migration history, only any changes we've made?

The only changes we do have is some added triggers to one auth table, my mistake earlier. Apart from that one both schemas have been untouched.

sweatybridge commented 8 months ago

So to be clear, we should not have any DDL SQL related to the creation of entities or the auth/storage schemas in our migration history, only any changes we've made?

Yup, that is correct.

I think triggers should work fine, as long as they are not replacing any existing ones predefined by supabase (we plan to lock them down in the future anyway).

If the problem persists, could you file a support ticket so I can take a closer look at your project?