Skip to content

Require preloading and guard the drop with an object access hook. - #58

Open
ibrarahmad wants to merge 4 commits into
lo-migrate-storage-rewritefrom
lo-require-preload
Open

ibrarahmad wants to merge 4 commits into
lo-migrate-storage-rewritefrom
lo-require-preload

Conversation

@ibrarahmad

@ibrarahmad ibrarahmad commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Follow-up to the design review on #55. Stacked on #56.

  • lolor must be listed in shared_preload_libraries. Loading it on demand is refused, so CREATE EXTENSION fails without it and a missing or broken library fails at server start rather than at the first lo_open().
  • An object_access_hook refuses to remove the extension while it is enabled or while lolor storage still holds large objects, on every path that reaches it (DROP EXTENSION, DROP SCHEMA lolor CASCADE, DROP OWNED BY). It checks catalog state at deletion time, from whichever backend does the drop. A superuser can set lolor.allow_unsafe_drop to skip both checks.
  • The drop no longer migrates anything. The event trigger only puts the native pg_catalog names back; migrate_to_native() is a manual step before the drop.
  • enable(), disable() and both migrations refuse to run outside a client session, so replicated DDL cannot execute them inside an apply worker.

Tests: a regression block exercises both refusals with the event trigger disabled and the unsafe drop inside a rolled-back transaction; t/001 checks the preload refusal; t/008 checks the refusal for DROP SCHEMA CASCADE and DROP OWNED BY; every TAP cluster and the CI docker nodes now preload lolor. README, docs pages, release notes and the pg_upgrade note are updated.

@coderabbitai

coderabbitai Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: d591196c-6d0b-4a4f-9388-ef8198357a1c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@codacy-production

codacy-production Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 17 complexity · 0 duplication

Metric Results
Complexity 17
Duplication 0

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@ibrarahmad
ibrarahmad force-pushed the lo-migrate-storage-rewrite branch from 02a1772 to 48d51e1 Compare September 29, 2026 09:59
lolor must now be listed in shared_preload_libraries; loading it on
demand is refused. An object_access_hook refuses to remove the
extension while it is enabled or while lolor storage still holds
large objects, on every path that reaches it, checking the catalog
state at deletion time rather than a flag left by an earlier session.
lolor.allow_unsafe_drop skips both checks. enable(), disable() and
the migrations refuse to run outside a client session, so replicated
DDL cannot execute them inside an apply worker.
@ibrarahmad
ibrarahmad force-pushed the lo-migrate-storage-rewrite branch from 48d51e1 to 2ea271d Compare September 29, 2026 19:08
The event trigger no longer runs migrate_to_native() when the
extension is dropped; it only puts the native pg_catalog names back.
With the object access hook refusing every drop path while lolor
storage holds objects, moving data from inside a DROP buys nothing
and inherits whatever context the drop runs in. Migration is a
manual step in both directions: run lolor.migrate_to_native(), then
drop. Tests and docs updated accordingly.
@mason-sharp
mason-sharp requested a review from rasifr October 7, 2026 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant