Skip to content

Commit 797bdce

Browse files
committed
fix(db): stop the re-root dedupe renaming personal workflows
workflow.workspace_id is nullable, and NULL is treated as EQUAL by PARTITION BY but UNKNOWN by the = in base_taken. So for personal workflows rn incremented across the group while no collision was ever detected: the dedupe could only fire spuriously, renaming a user-visible workflow that needed no rename, since the unique index treats NULL workspace_id rows as distinct anyway. The file block already carried the equivalent guard. Also gives step 1's inserts ON CONFLICT (id) DO NOTHING. It restates the stranded guard, closing the window between that read's snapshot and the index check — an operational re-run of 0274, or a live pod, committing into folder mid-statement would otherwise raise 23505, and migrate.ts retries only 55P03. 0272 and 0274 were already written this way; step 1 was the exception. Corrects three comments that claimed more than the code delivers: the header's 'no legacy folder is lost' (an id already present under another resource_type cannot be rescued), the reachability note (a cycle among stranded rows survives a per-row parent check), and a note made stale by the ON CONFLICT above.
1 parent c1dc251 commit 797bdce

1 file changed

Lines changed: 33 additions & 7 deletions

File tree

packages/db/migrations/0276_drop_legacy_folder_tables.sql

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@
22
-- keys and drop the two legacy folder tables.
33
--
44
-- Ordering is deliberate and each step depends on the one before it:
5-
-- 1. final insert-only reconcile, so no legacy folder is lost by the DROP;
5+
-- 1. final insert-only reconcile, so no legacy folder is lost by the DROP — with one
6+
-- inherent exception: a legacy id already present in `folder` under a DIFFERENT
7+
-- resource_type cannot be inserted (the primary key is taken) and is dropped. That needs
8+
-- an id collision across two tables whose ids were preserved from disjoint sources, so it
9+
-- is not reachable in practice;
610
-- 2. re-root any `folder_id` that still does not resolve, so the FK can be validated;
711
-- 3. adopt the FKs the expand migration deliberately left off;
812
-- 4. drop the legacy tables.
@@ -44,9 +48,10 @@ BEGIN
4448
END IF;
4549

4650
INSERT INTO "folder" (id, resource_type, name, user_id, workspace_id, parent_id, locked, sort_order, created_at, updated_at, deleted_at)
47-
-- Keyed on `id` ALONE, matching the primary key this guard protects. Narrowing it by
48-
-- resource_type would classify an id already present under a different type as stranded and
49-
-- then die on folder_pkey, since there is no ON CONFLICT here.
51+
-- Keyed on `id` ALONE, matching the primary key it protects. Narrowing it by resource_type
52+
-- would classify an id already present under a DIFFERENT type as stranded; the ON CONFLICT
53+
-- below would then silently skip it rather than rescue it, so keeping the guard aligned with
54+
-- the constraint is what makes the two agree.
5055
WITH stranded AS (
5156
SELECT l.id, l.name, l.user_id, l.workspace_id, l.parent_id, l.locked, l.sort_order,
5257
l.created_at, l.updated_at, l.archived_at AS deleted_at
@@ -67,6 +72,12 @@ BEGIN
6772
--
6873
-- Anything else re-roots to the workspace root: losing one level of nesting beats losing the
6974
-- folder and stranding every workflow inside it.
75+
--
76+
-- LIMITATION: this is a per-row check, so a CYCLE among stranded rows (a→b→a) survives it —
77+
-- every row's parent exists, is same-workspace, and is active. Such rows land in `folder`
78+
-- unreachable from the root. 0272's backfill has the identical hole, so this is not a
79+
-- regression, and the client tolerates it (`getFolderPath` and `subtree.ts` both carry cycle
80+
-- guards). Breaking cycles needs a recursive walk; it is deliberately not done here.
7081
resolved AS (
7182
SELECT s.*,
7283
CASE
@@ -147,7 +158,12 @@ BEGIN
147158
r.user_id, r.workspace_id, r.resolved_parent, r.locked, r.sort_order,
148159
r.created_at, r.updated_at, r.deleted_at
149160
FROM resolved r
150-
LEFT JOIN named n ON n.id = r.id;
161+
LEFT JOIN named n ON n.id = r.id
162+
-- Matches the `stranded` guard, which already means "no folder row with this id". Restating
163+
-- it as ON CONFLICT closes the gap between that read's snapshot and the index check: an
164+
-- operational re-run of 0274, or a live pod, committing into `folder` mid-statement would
165+
-- otherwise raise 23505 — and migrate.ts retries only 55P03, so that hard-fails the deploy.
166+
ON CONFLICT (id) DO NOTHING;
151167
END $$;
152168
--> statement-breakpoint
153169

@@ -241,7 +257,9 @@ BEGIN
241257
r.user_id, r.workspace_id, r.resolved_parent, false, r.sort_order,
242258
r.created_at, r.updated_at, r.deleted_at
243259
FROM resolved r
244-
LEFT JOIN named n ON n.id = r.id;
260+
LEFT JOIN named n ON n.id = r.id
261+
-- Same rationale as the workflow tree above.
262+
ON CONFLICT (id) DO NOTHING;
245263
END $$;
246264
--> statement-breakpoint
247265

@@ -254,8 +272,16 @@ END $$;
254272
-- under a suffixed name strictly improves on leaving it invisible.
255273
DO $$
256274
BEGIN
275+
-- `workspace_id IS NOT NULL` is load-bearing, not defensive. `workflow.workspace_id` is
276+
-- nullable (personal workflows), and NULL is treated as EQUAL by `PARTITION BY` but as
277+
-- UNKNOWN by the `=` in `base_taken`. Without this guard `rn` increments across every
278+
-- personal workflow while no collision is ever detected, so the dedup can ONLY fire
279+
-- spuriously — renaming a user-visible workflow that needed no rename, since the unique
280+
-- index treats NULL `workspace_id` rows as distinct anyway. The file block below has always
281+
-- carried the equivalent guard.
257282
WITH dangling AS (
258-
SELECT w.id, w.workspace_id, w.name, (w.archived_at IS NULL) AS is_active
283+
SELECT w.id, w.workspace_id, w.name,
284+
(w.archived_at IS NULL AND w.workspace_id IS NOT NULL) AS is_active
259285
FROM "workflow" w
260286
WHERE w."folder_id" IS NOT NULL
261287
AND NOT EXISTS (SELECT 1 FROM "folder" f WHERE f.id = w."folder_id")

0 commit comments

Comments
 (0)