feat(migrations): UNIQUE constraints on account_artist_ids + artist_organization_ids pairs#46
Conversation
…rganization_ids pairs Dedupe existing violating rows (keep lowest id per pair), then add UNIQUE(account_id, artist_id) on account_artist_ids and UNIQUE(artist_id, organization_id) on artist_organization_ids so connector-connection scoping can key on the join row (chat#1860 P1). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Updates to Preview Branch (feat/join-row-unique-constraints) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
📝 WalkthroughWalkthroughThis PR adds a SQL migration that deduplicates rows and adds idempotent composite UNIQUE constraints to the account_artist_ids table on (account_id, artist_id) and the artist_organization_ids table on (artist_id, organization_id). ChangesJoin Row Unique Constraints
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
supabase/migrations/20260708200000_join_row_unique_constraints.sql (1)
44-48: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider using
IS NOT DISTINCT FROMconsistently across both dedupe blocks.The
account_artist_idsdedupe (lines 25-26) usesIS NOT DISTINCT FROMwhile this block uses=. While=is correct here becauseartist_organization_idscolumns areNOT NULL, usingIS NOT DISTINCT FROMin both blocks would be more defensive against future schema changes and makes the intent explicitly clear.♻️ Consistency suggestion
DELETE FROM public.artist_organization_ids a USING public.artist_organization_ids b -WHERE a.artist_id = b.artist_id - AND a.organization_id = b.organization_id +WHERE a.artist_id IS NOT DISTINCT FROM b.artist_id + AND a.organization_id IS NOT DISTINCT FROM b.organization_id AND a.id > b.id;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@supabase/migrations/20260708200000_join_row_unique_constraints.sql` around lines 44 - 48, The dedupe logic in the `artist_organization_ids` cleanup block is inconsistent with the earlier `account_artist_ids` block because it uses `=` instead of `IS NOT DISTINCT FROM`. Update the `DELETE ... USING` join condition in this migration so the `a.artist_id`/`b.artist_id` and `a.organization_id`/`b.organization_id` comparisons match the more defensive pattern used in the other dedupe block, keeping the `a.id > b.id` tie-breaker unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@supabase/migrations/20260708200000_join_row_unique_constraints.sql`:
- Around line 44-48: The dedupe logic in the `artist_organization_ids` cleanup
block is inconsistent with the earlier `account_artist_ids` block because it
uses `=` instead of `IS NOT DISTINCT FROM`. Update the `DELETE ... USING` join
condition in this migration so the `a.artist_id`/`b.artist_id` and
`a.organization_id`/`b.organization_id` comparisons match the more defensive
pattern used in the other dedupe block, keeping the `a.id > b.id` tie-breaker
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: afb4e307-a3d1-44ce-9b83-626c057a6db3
📒 Files selected for processing (1)
supabase/migrations/20260708200000_join_row_unique_constraints.sql
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="supabase/migrations/20260708200000_join_row_unique_constraints.sql">
<violation number="1" location="supabase/migrations/20260708200000_join_row_unique_constraints.sql:37">
P2: Rows with a NULL `account_id` or `artist_id` can still be duplicated after this migration, because PostgreSQL's default `UNIQUE (account_id, artist_id)` treats NULLs as distinct. Since the dedupe is already using `IS NOT DISTINCT FROM`, it looks like NULL-equivalent pairs are intended to collapse too; consider making these columns `NOT NULL` before adding the constraint, or using a NULL-aware uniqueness strategy such as `UNIQUE NULLS NOT DISTINCT` if the target Postgres version supports it.</violation>
<violation number="2" location="supabase/migrations/20260708200000_join_row_unique_constraints.sql:58">
P3: `artist_organization_ids` already has a unique index on `(artist_id, organization_id)`, so this block will add a redundant second unique index rather than closing a missing race for that table. If the constraint object is needed for metadata, consider reusing/renaming the existing unique index or otherwise accounting for `artist_organization_ids_unique`; if the database-enforced uniqueness is all that is needed, this part of the migration can be skipped.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| ) THEN | ||
| ALTER TABLE "public"."account_artist_ids" | ||
| ADD CONSTRAINT "account_artist_ids_account_id_artist_id_key" | ||
| UNIQUE (account_id, artist_id); |
There was a problem hiding this comment.
P2: Rows with a NULL account_id or artist_id can still be duplicated after this migration, because PostgreSQL's default UNIQUE (account_id, artist_id) treats NULLs as distinct. Since the dedupe is already using IS NOT DISTINCT FROM, it looks like NULL-equivalent pairs are intended to collapse too; consider making these columns NOT NULL before adding the constraint, or using a NULL-aware uniqueness strategy such as UNIQUE NULLS NOT DISTINCT if the target Postgres version supports it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/migrations/20260708200000_join_row_unique_constraints.sql, line 37:
<comment>Rows with a NULL `account_id` or `artist_id` can still be duplicated after this migration, because PostgreSQL's default `UNIQUE (account_id, artist_id)` treats NULLs as distinct. Since the dedupe is already using `IS NOT DISTINCT FROM`, it looks like NULL-equivalent pairs are intended to collapse too; consider making these columns `NOT NULL` before adding the constraint, or using a NULL-aware uniqueness strategy such as `UNIQUE NULLS NOT DISTINCT` if the target Postgres version supports it.</comment>
<file context>
@@ -0,0 +1,60 @@
+ ) THEN
+ ALTER TABLE "public"."account_artist_ids"
+ ADD CONSTRAINT "account_artist_ids_account_id_artist_id_key"
+ UNIQUE (account_id, artist_id);
+ END IF;
+END $$;
</file context>
| ) THEN | ||
| ALTER TABLE "public"."artist_organization_ids" | ||
| ADD CONSTRAINT "artist_organization_ids_artist_id_organization_id_key" | ||
| UNIQUE (artist_id, organization_id); |
There was a problem hiding this comment.
P3: artist_organization_ids already has a unique index on (artist_id, organization_id), so this block will add a redundant second unique index rather than closing a missing race for that table. If the constraint object is needed for metadata, consider reusing/renaming the existing unique index or otherwise accounting for artist_organization_ids_unique; if the database-enforced uniqueness is all that is needed, this part of the migration can be skipped.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/migrations/20260708200000_join_row_unique_constraints.sql, line 58:
<comment>`artist_organization_ids` already has a unique index on `(artist_id, organization_id)`, so this block will add a redundant second unique index rather than closing a missing race for that table. If the constraint object is needed for metadata, consider reusing/renaming the existing unique index or otherwise accounting for `artist_organization_ids_unique`; if the database-enforced uniqueness is all that is needed, this part of the migration can be skipped.</comment>
<file context>
@@ -0,0 +1,60 @@
+ ) THEN
+ ALTER TABLE "public"."artist_organization_ids"
+ ADD CONSTRAINT "artist_organization_ids_artist_id_organization_id_key"
+ UNIQUE (artist_id, organization_id);
+ END IF;
+END $$;
</file context>
Pre-apply verification (read-only against prod
|
| Table | Existing constraints | Target UNIQUE present? |
|---|---|---|
account_artist_ids |
PK (id); FK account_id→accounts(id) ON DELETE CASCADE; FK artist_id→accounts(id) ON DELETE CASCADE |
❌ no UNIQUE(account_id, artist_id) |
artist_organization_ids |
PK (id); FK artist_id→accounts(id) ON DELETE CASCADE; FK organization_id→accounts(id) ON DELETE CASCADE |
❌ no UNIQUE(artist_id, organization_id) |
So the join row genuinely can't serve as a deterministic key yet — exactly the gap this migration closes.
Dedupe blast radius: zero
Counted the rows the defensive DELETE step would remove:
| Table | Duplicate pairs | Rows the dedupe would delete |
|---|---|---|
account_artist_ids |
0 | 0 |
artist_organization_ids |
0 | 0 |
Confirms the PR's "none known today." The migration is purely additive on current prod data — it adds two constraints and deletes nothing.
Idempotency
Both constraint adds are pg_constraint-guarded and the target names (account_artist_ids_account_id_artist_id_key, artist_organization_ids_artist_id_organization_id_key) are absent, so the first run adds them and any re-run is a no-op. Dedupe DELETEs are no-ops at 0 duplicates.
Verdict
Safe to merge/apply: additive, zero row loss, idempotent, and it unblocks the join-row re-key (chat#1860 P1). Post-merge I can re-query to confirm both constraints landed and that a duplicate insert is rejected — read-only, on request.
🤖 Generated with Claude Code
Post-apply verification — migration executed in prod ✅Ran the AFTER pass against prod (
Functional test (live): inserting a duplicate of an existing Note on the
|
1 participant
What
Adds
supabase/migrations/20260708200000_join_row_unique_constraints.sql:account_artist_ids—UNIQUE (account_id, artist_id). The table has only a bigint PK onidtoday, so nothing prevents duplicate pairs and the join row can't serve as a deterministic key.artist_organization_ids—UNIQUE (artist_id, organization_id).Each constraint is preceded by a defensive dedupe (keep the lowest
idper pair, delete the rest) so the migration applies cleanly against prod data even if duplicate pairs exist — none are known today. Constraint adds are guarded bypg_constraintexistence checks, so the migration is idempotent.Why
P1 prerequisite for re-keying Composio connection ownership on the join row (connector-connection scoping): recoupable/chat#1860.
Cross-reference: this closes the same check-then-insert race class tracked in recoupable/chat#1844 for these two sibling tables — concurrent duplicate inserts now fail at the database instead of minting duplicate rows. It does not fix #1844 itself (different tables).
Status
Not yet applied to any environment — migration apply + verification pending, tracked in chat#1860.
🤖 Generated with Claude Code
Summary by cubic
Add UNIQUE constraints to
account_artist_idsandartist_organization_idsso each pair is stored once and the join row can serve as a deterministic key. This prevents duplicate inserts and enables connector-connection scoping needed for chat#1860.UNIQUE(account_id, artist_id)andUNIQUE(artist_id, organization_id)if missing (idempotent).Written for commit 0ca5636. Summary will update on new commits.
Summary by CodeRabbit