Skip to content

Avoid backfilling remote quote policies#460

Merged
dahlia merged 1 commit intofedify-dev:mainfrom
dahlia:fix/legacy-remote-quotes
Apr 29, 2026
Merged

Avoid backfilling remote quote policies#460
dahlia merged 1 commit intofedify-dev:mainfrom
dahlia:fix/legacy-remote-quotes

Conversation

@dahlia
Copy link
Copy Markdown
Member

@dahlia dahlia commented Apr 29, 2026

Summary

This PR updates the unreleased quote-control migrations so existing remote posts are not mass-updated when introducing nullable quote approval policies.

The previous migration sequence first added quote_approval_policy as DEFAULT 'public' NOT NULL, then used the follow-up migration to drop NOT NULL and rewrite cached remote public rows to NULL. That worked functionally, but it was expensive on large posts tables and could take many minutes in production-like databases.

This PR makes the migration create the final column shape directly: nullable from the start, with only local posts backfilled. Existing remote posts remain NULL, which preserves the legacy remote-post behavior added by the previous PR without requiring a table-wide remote-row rewrite.

Changes

  • Update drizzle/0086_quote_controls.sql to add quote_approval_policy as nullable and without an immediate default.
  • Backfill quote_approval_policy only for local posts, where Hollo owns the quote policy and can derive it from visibility.
  • Set the default to 'public' only after the local backfill, so new local posts keep the expected default.
  • Update drizzle/0087_quote_authorization_requirement.sql to keep the cheap DROP NOT NULL for databases that already applied the earlier 0086 draft, while removing the expensive remote-row backfill.
  • Add migration comments documenting why remote posts are intentionally left NULL.

Why this is needed

On instances with many cached remote posts, the old follow-up migration spent too long rewriting remote rows from 'public' to NULL. That rewrite is unnecessary if the column is nullable from the beginning.

It also avoids encoding misleading data: before this feature, cached remote posts did not have stored FEP-044f quote policy information, so defaulting every old remote row to 'public' made legacy remote posts indistinguishable from remote posts that truly advertised a public FEP-044f quote policy.

By leaving existing remote rows NULL, the database accurately represents “no quote policy was observed,” and the application can treat those posts as legacy quote targets.

Testing

  • pnpm migrate:generate
  • pnpm run migrate:test
  • pnpm check
  • git diff --check

@dahlia dahlia added this to the Hollo 0.9 milestone Apr 29, 2026
@dahlia dahlia self-assigned this Apr 29, 2026
@dahlia dahlia added bug Something isn't working performance labels Apr 29, 2026
@dahlia
Copy link
Copy Markdown
Member Author

dahlia commented Apr 29, 2026

@codex review

@dahlia dahlia force-pushed the fix/legacy-remote-quotes branch from 345f3af to 00406cd Compare April 29, 2026 08:43
@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 👍

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Create the quote approval policy column nullable from the start so old
cached remote posts remain NULL without a later table-wide rewrite.  Only
local posts are backfilled because Hollo owns their quote policy and can
derive it from visibility.

Leave the follow-up migration as an explicit no-op with context about the
tradeoff and the prior expensive backfill approach.

Assisted-by: Codex:gpt-5.5
@dahlia dahlia force-pushed the fix/legacy-remote-quotes branch from 00406cd to 5852320 Compare April 29, 2026 08:47
@dahlia dahlia merged commit d646736 into fedify-dev:main Apr 29, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working performance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant