fix: index subscription filters by digest so large in lists fit - #2097
Open
hamodywe wants to merge 2 commits into
Open
fix: index subscription filters by digest so large in lists fit#2097hamodywe wants to merge 2 commits into
hamodywe wants to merge 2 commits into
Conversation
realtime.subscription's unique index covers `filters` by value, so a subscription's filter array has to fit the btree tuple limit of 2704 bytes. An `id=in.(...)` list of uuids passes that at 70 values and the insert fails with 54000, even though subscription_check_filters allows 100 and the docs document 100. The largest list that can actually be subscribed is 69. Index realtime.filters_hash(filters) instead - a sha256 of the array - and match the upsert's conflict target to it. Uniqueness is unchanged, the size ceiling is gone, and the enforced maximum of 100 becomes reachable. The bundled tenant dumps are updated too: a new tenant is provisioned from priv/repo/tenant_db_dump_<major>.sql and is then marked as fully migrated without replaying migrations, so leaving them stale would give new tenants the old index and no matching conflict target. Closes supabase#1670
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1670.
The problem
realtime.subscription's unique index coversfiltersby value:so a subscription's whole filter array has to fit btree's 2704-byte tuple limit. An
id=in.(...)list of uuids passes that quickly, and the subscription fails to register with54000 index row size ... exceeds btree version 4 maximum 2704.Measured on
supabase/postgres:17.6.1.127by insertinginfilters of growing length:69. Meanwhile
realtime.subscription_check_filtersraisestoo many values for 'in' filter. Maximum 100, and the docs describe the same 100. So the limit the code enforces has never been reachable on a uuid column, and because the real constraint is total byte length rather than value count, the same list works on anintcolumn and fails on auuidone — which is what makes it look arbitrary from the client side.The change
Index a digest of the array instead of the array:
and point the upsert's conflict target at the same expression. The index row is now fixed-width, uniqueness is unchanged, and the 100-value guard becomes the thing that actually stops you.
sha256 rather than the md5 the Postgres hint suggests, since the value is user-controlled and the digest is what decides whether two subscriptions are the same row; a collision would silently fold one subscription into another.
grant executefollows the pattern of20260706120000_grant_check_equality_op_5_arg, since the blanket grant in20231204144023only covered functions that existed then.Why the tenant dumps are in this PR
Realtime.Tenants.Migrations.load_db_dump/1provisions a brand-new tenant frompriv/repo/tenant_db_dump_<major>.sqland returnsEnum.count(migrations())— it records every migration as applied without replaying any of them. A new tenant would therefore keep the old value index while being marked fully migrated, and the newon conflicttarget would have nothing to match, so everypostgres_changessubscription on that tenant would fail, not just the large ones.update-tenant-db-snapshots.ymlis gated to same-repo PRs (if: github.event.pull_request.head.repo.full_name == github.repository), so it will not run here. I updated the three snapshot files by hand — the function, its ACL block, the index, theschema_migrationsrow, and thetenant_schemaentries — and verified the result rather than trusting the edit: a fresh database loaded from the editedtenant_db_dump_17.sqlreports 82 migrations (matchingmigrations.ex), hassubscription_subscription_id_entity_filters_hash_keyandrealtime.filters_hash, and accepts a 100-uuid filter through the exact upsert statement fromsubscriptions.ex. Please regenerate them properly before merge if you would rather not carry a hand-edit.Verification
Tests added to
subscriptions_test.exs: a 100-valueinfilter is accepted, and resubscribing the same large filter updates the existing row instead of duplicating it (that one guards the conflict-target change).I could not run
mix testlocally — Windows box, no Elixir toolchain, and the suite provisions tenant databases through its own Docker backend. I verified the SQL directly instead, against a real tenant schema onsupabase/postgres:17.6.1.127:P0001 too many values for 'in' filter. Maximum 100— the intended guard, reached for the first time.