cli icon indicating copy to clipboard operation
cli copied to clipboard

`db diff`: Incorrect migration for enum (Declarative Schema)

Open Revadike opened this issue 9 months ago • 7 comments

I added new enums to the end of my notification enum definition: Image

Running the diff tool did something unexpected:

alter type "public"."notification" rename to "notification__old_version_to_be_dropped";

create type "public"."notification" as enum ('new_trade', 'accepted_trade', 'new_vault_entry', 'unread_messages', 'disputed_trade', 'resolved_trade');

alter table "public"."notifications" alter column type type "public"."notification" using type::text::"public"."notification";

drop type "public"."notification__old_version_to_be_dropped";

I expected it to do the following:

-- Add new enum values to the existing type instead of recreating it
ALTER TYPE "public"."notification" ADD VALUE IF NOT EXISTS 'disputed_trade';
ALTER TYPE "public"."notification" ADD VALUE IF NOT EXISTS 'resolved_trade';

Revadike avatar May 11 '25 12:05 Revadike

Ok, this may be the reason?

Applying migration 20250511124316_patch.sql...
ERROR: unsafe use of new value "disputed_trade" of enum type notification (SQLSTATE 55P04)                                  
                                                             
At statement 2:                                    
alter table "public"."preferences" alter column "enabled_notifications" set default ARRAY['new_trade'::notification, 'accepted_trade'::notification, 'new_vault_entry'::notification, 'unread_messages'::notification, 'disputed_trade'::notification, 'resolved_trade'::notification]

Apparently, PostgreSQL doesn't allow you to use newly added enum values in the same transaction where they're created.

This fixes it:

ALTER TYPE "public"."notification" ADD VALUE IF NOT EXISTS 'disputed_trade';
ALTER TYPE "public"."notification" ADD VALUE IF NOT EXISTS 'resolved_trade';

-- Commit the transaction to make the new enum values available for use
COMMIT;

-- Start a new transaction for using the new enum values
BEGIN;

--- The rest of the migration

Revadike avatar May 11 '25 13:05 Revadike

Scratch that, It seems to have ignored everything after BEGIN;

Perhaps the docs should just suggest users to put altering enums in a separate migration.

Revadike avatar May 11 '25 13:05 Revadike

@Revadike from my experience with postgres enums, beyond the issue you just mentioned, there are other limitations - I believe you cannot edit or remove a value (e.g. in your notification enum, I don't think you can rename or remove unread_messages). The way changes to enums is currently implemented with supabase diff covers all use cases (add an enum value, but also edit / remove enum values) in one go. I assume that's why the diff works like that. I don't work at supabase or anything, just sharing my two cents.

cohlar avatar May 13 '25 12:05 cohlar

@Revadike from my experience with postgres enums, beyond the issue you just mentioned, there are other limitations - I believe you cannot edit or remove a value (e.g. in your notification enum, I don't think you can rename or remove unread_messages). The way changes to enums is currently implemented with supabase diff covers all use cases (add an enum value, but also edit / remove enum values) in one go. I assume that's why the diff works like that. I don't work at supabase or anything, just sharing my two cents.

Nothing was changed or removed, just added. Yet, it made the migration as if something was modified/removed, which is the whole problem.

Revadike avatar May 13 '25 12:05 Revadike

Well, let's see if Supabase sees this as an issue. I personally don't see an issue here.

cohlar avatar May 13 '25 12:05 cohlar

I have a similar challenge where ALTER TYPE ADD VALUE is a better solution.

I see that it would be hard to correctly support in the diff tool, since PostgreSQL enumeration values are ordered, rename could be dangerous, and removing a value strictly requires the enumeration to be rebuilt. Moreover, the documentation warns that ALTER TYPE ADD VALUE might have a negative performance impact. (I wonder why.)

I have a "permissions" enumeration, which is used in numerous policies. The supabase diff tool doesn't find or rewrite the policies. So it's impossible to maintain the enumeration using the supabase diff tool.

It would be helpful if the tool could at least detect that it cannot provide a complete update and output a comment or error.

joshuanapoli avatar May 23 '25 20:05 joshuanapoli