Skip to content

Remove a duplicate member from an enum #835

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Remove a duplicate member from an enum #835

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3fdcf75
Write a SQL script to resolve duplicates
Ahmed-Ghanam May 7, 2025
23cb0ee
Place a lock on the table and improve the dependent recreation step
Ahmed-Ghanam May 7, 2025
531b028
Add the generated DDL script
Ahmed-Ghanam May 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 89 additions & 0 deletions src/Altinn.Notifications.Persistence/Migration/v0.43/01-alter-types.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
/*
* Script to resolve duplicate enum values in smsnotificationresulttype
*
* Problem: The enum has both 'Failed_RecipientReserved' and 'Failed_Recipientreserved' values
* Solution: Normalize to proper case version and recreate enum without duplicate value
* More details: Refer to issue https://github.com/Altinn/altinn-notifications/issues/810
*/

-- 1. Create a new enum type
DO $$
BEGIN
IF NOT EXISTS (SELECT 1 FROM pg_type WHERE typname = 'smsnotificationresulttype_new' AND typnamespace = 'public'::regnamespace) THEN
CREATE TYPE public.smsnotificationresulttype_new AS ENUM (
'New',
'Sending',
'Accepted',
'Failed_InvalidReceiver',
'Failed',
'Failed_RecipientNotIdentified',
'Delivered',
'Failed_BarredReceiver',
'Failed_Deleted',
'Failed_Expired',
'Failed_Undelivered',
'Failed_Rejected',
'Failed_InvalidRecipient',
'Failed_RecipientReserved'
);
END IF;
END
$$;

-- 2. Lock the target table
LOCK TABLE notifications.smsnotifications IN ACCESS EXCLUSIVE MODE;

-- 3. Migrate table column and normalizing existing data
ALTER TABLE notifications.smsnotifications
ALTER COLUMN result TYPE public.smsnotificationresulttype_new
USING (CASE
WHEN result::text = 'Failed_Recipientreserved' THEN 'Failed_RecipientReserved'
ELSE result::text
END)::public.smsnotificationresulttype_new;

-- 4. Recreate dependent
DO $$
DECLARE
rec RECORD;
def TEXT;
BEGIN
FOR rec IN
SELECT n.nspname, c.relname, c.relkind
FROM pg_depend d
JOIN pg_type t ON d.objid = t.oid
JOIN pg_class c ON d.refobjid = c.oid
JOIN pg_namespace n ON n.oid = c.relnamespace
WHERE t.typname = 'smsnotificationresulttype' AND c.relkind IN ('v','m')
LOOP
def := pg_get_viewdef(format('%I.%I', rec.nspname, rec.relname), true);
IF rec.relkind = 'v' THEN
EXECUTE format('CREATE OR REPLACE VIEW %I.%I AS %s', rec.nspname, rec.relname, def);
ELSE
EXECUTE format('CREATE OR REPLACE MATERIALIZED VIEW %I.%I AS %s', rec.nspname, rec.relname, def);
EXECUTE format('REFRESH MATERIALIZED VIEW %I.%I', rec.nspname, rec.relname);
END IF;
END LOOP;

FOR rec IN
SELECT n.nspname, p.proname, p.oid
FROM pg_depend d
JOIN pg_type t ON d.objid = t.oid
JOIN pg_proc p ON d.refobjid = p.oid
JOIN pg_namespace n ON n.oid = p.pronamespace
WHERE t.typname = 'smsnotificationresulttype'
LOOP
def := pg_get_functiondef(rec.oid);
def := regexp_replace(def, '\m smsnotificationresulttype \M', 'smsnotificationresulttype_new', 'g');
EXECUTE def;
END LOOP;
END
$$;
Comment on lines +45 to +80
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Harden dependent-object recreation and regex replacement

  1. The regex '\m smsnotificationresulttype \M' may miss schema-qualified names (e.g. public.smsnotificationresulttype).
  2. To catch both unqualified and qualified references, use:
- def := regexp_replace(def, '\m smsnotificationresulttype \M', 'smsnotificationresulttype_new', 'g');
+ def := regexp_replace(def,
+   '\m(\w+\.)?smsnotificationresulttype\M',
+   'public.smsnotificationresulttype_new',
+   'g');
  1. Consider filtering pg_depend on t.typnamespace to only rebuild objects in the intended schema.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
DO $$
DECLARE
rec RECORD;
def TEXT;
BEGIN
FOR rec IN
SELECT n.nspname, c.relname, c.relkind
FROM pg_depend d
JOIN pg_type t ON d.objid = t.oid
JOIN pg_class c ON d.refobjid = c.oid
JOIN pg_namespace n ON n.oid = c.relnamespace
WHERE t.typname = 'smsnotificationresulttype' AND c.relkind IN ('v','m')
LOOP
def := pg_get_viewdef(format('%I.%I', rec.nspname, rec.relname), true);
IF rec.relkind = 'v' THEN
EXECUTE format('CREATE OR REPLACE VIEW %I.%I AS %s', rec.nspname, rec.relname, def);
ELSE
EXECUTE format('CREATE OR REPLACE MATERIALIZED VIEW %I.%I AS %s', rec.nspname, rec.relname, def);
EXECUTE format('REFRESH MATERIALIZED VIEW %I.%I', rec.nspname, rec.relname);
END IF;
END LOOP;
FOR rec IN
SELECT n.nspname, p.proname, p.oid
FROM pg_depend d
JOIN pg_type t ON d.objid = t.oid
JOIN pg_proc p ON d.refobjid = p.oid
JOIN pg_namespace n ON n.oid = p.pronamespace
WHERE t.typname = 'smsnotificationresulttype'
LOOP
def := pg_get_functiondef(rec.oid);
def := regexp_replace(def, '\m smsnotificationresulttype \M', 'smsnotificationresulttype_new', 'g');
EXECUTE def;
END LOOP;
END
$$;
FOR rec IN
SELECT n.nspname, p.proname, p.oid
FROM pg_depend d
JOIN pg_type t ON d.objid = t.oid
JOIN pg_proc p ON d.refobjid = p.oid
JOIN pg_namespace n ON n.oid = p.pronamespace
WHERE t.typname = 'smsnotificationresulttype'
LOOP
def := pg_get_functiondef(rec.oid);
- def := regexp_replace(def, '\m smsnotificationresulttype \M', 'smsnotificationresulttype_new', 'g');
+ def := regexp_replace(def,
+ '\m(\w+\.)?smsnotificationresulttype\M',
+ 'public.smsnotificationresulttype_new',
+ 'g');
EXECUTE def;
END LOOP;
🤖 Prompt for AI Agents (early access) 
In src/Altinn.Notifications.Persistence/Migration/v0.43/01-alter-types.sql
around lines 45 to 80, the regex used to replace 'smsnotificationresulttype' in
function definitions does not account for schema-qualified names, which can
cause missed replacements. Update the regex to match both unqualified and
schema-qualified references of the type name. Additionally, refine the pg_depend
queries by filtering on t.typnamespace to restrict dependent object recreation
to the intended schema only, preventing unintended object modifications outside
the target schema.


-- 5. Drop the old enum type
DROP TYPE IF EXISTS public.smsnotificationresulttype;

-- 6. Rename the new enum type
ALTER TYPE public.smsnotificationresulttype_new RENAME TO smsnotificationresulttype;

-- 7. Set correct ownership permissions
ALTER TYPE public.smsnotificationresulttype OWNER TO platform_notifications_admin;
Loading
Loading