Rework large object storage migration and fix related defects.

.github/workflows/workflow.yml

@@ -22,6 +22,10 @@ jobs:
             spock_ref: main
 
     runs-on: ${{ matrix.os }}
+    # Without this a hung tester runs until the 6 hour job ceiling, on every
+    # matrix entry, and the run is cancelled rather than failed -- which skips
+    # the diagnostic steps below and leaves nothing to debug with.
+    timeout-minutes: 45
 
     steps:
     - name: Checkout lolor
@@ -115,7 +119,36 @@ jobs:
       id: test_step
       continue-on-error: true
       run: |
+        # The tester exits when run-tests.sh finishes.  Cap the wait so that a
+        # test that blocks -- on a lock, or on spock.sub_wait_for_sync() --
+        # fails the step while the cluster is still up to be inspected.
+        deadline=$(( SECONDS + 25 * 60 ))
         while [ "$(docker inspect -f '{{.State.Running}}' tester)" == "true" ]; do
+          if [ "$SECONDS" -ge "$deadline" ]; then
+            echo "::error::Tests did not finish within 25 minutes; dumping cluster state"
+            PGV=${{ matrix.pgver }}
+            echo "===== tester output so far ====="
+            tail -n 200 tests/out.txt || true
+            for n in n1 n2 n3; do
+              echo "===== $n: sessions and what they are waiting on ====="
+              docker exec "$n" bash -c "source /home/pgedge/pgedge/pg${PGV}/pg${PGV}.env && psql -U admin -d demo -x -c \"
+                SELECT pid, state, wait_event_type, wait_event, now() - xact_start AS xact_age,
+                       left(query, 400) AS query
+                FROM pg_stat_activity
+                WHERE datname = 'demo' AND pid <> pg_backend_pid()
+                ORDER BY xact_start\"" || true
+              echo "===== $n: ungranted locks ====="
+              docker exec "$n" bash -c "source /home/pgedge/pgedge/pg${PGV}/pg${PGV}.env && psql -U admin -d demo -c \"
+                SELECT l.pid, l.locktype, l.mode, l.granted,
+                       coalesce(c.relname, l.classid::text) AS object
+                FROM pg_locks l LEFT JOIN pg_class c ON c.oid = l.relation
+                WHERE NOT l.granted OR l.mode LIKE 'Share%' OR l.mode LIKE '%Exclusive%'
+                ORDER BY l.granted, l.pid\"" || true
+              echo "===== $n: subscription status ====="
+              docker exec "$n" bash -c "source /home/pgedge/pgedge/pg${PGV}/pg${PGV}.env && psql -U admin -d demo -c 'SELECT * FROM spock.sub_show_status()'" || true
+            done
+            exit 1
+          fi
           echo "Waiting for tests to complete..."
           sleep 1
         done
@@ -124,6 +157,17 @@ jobs:
         docker logs n3
         grep -Eq "FAIL|ERROR" tests/out.txt && exit 1 || exit 0
 
+    - name: Dump container logs when the tests failed
+      if: steps.test_step.outcome == 'failure'
+      run: |
+        cd docker
+        for c in n1 n2 n3 tester; do
+          echo "===== $c ====="
+          docker logs "$c" > /tmp/$c.log 2>&1 || true
+          echo "--- first 120 lines (setup) ---"; head -n 120 /tmp/$c.log || true
+          echo "--- last 120 lines ---"; tail -n 120 /tmp/$c.log || true
+        done
+
     - name: Upload Log File as Artifact
       uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4
       with:

.gitignore

@@ -1,3 +1,10 @@
 *.so
 *.bc
 *.o
+*.dylib
+results/
+regression.diffs
+regression.out
+tmp_check/
+log/
+delete_old_cluster.sh

Makefile

@@ -5,10 +5,11 @@ MODULE_big = lolor
 EXTENSION = lolor
 DATA = lolor--1.0.sql \
 	   lolor--1.0--1.2.1.sql lolor--1.2.1--1.2.2.sql \
-	   lolor--1.2.2--1.3.0.sql
+	   lolor--1.2.2--1.3.0.sql lolor--1.3.0--1.4.0.sql
 PGFILEDESC = "lolor - drop in large objects replacement for logical replication"
 
-OBJS = src/lolor.o src/lolor_fsstubs.o src/lolor_inv_api.o src/lolor_largeobject.o
+OBJS = src/lolor.o src/lolor_fsstubs.o src/lolor_inv_api.o src/lolor_largeobject.o \
+       src/lolor_migrate.o
 
 REGRESS = lolor
 TAP_TESTS = 1

README.md

@@ -88,7 +88,24 @@ SELECT lolor.migrate_to_native();  -- manual
 DROP EXTENSION lolor;              -- automatic
 ```
 
-Both directions preserve original OIDs, owners, ACLs, and data.
+Both directions preserve original OIDs, owners, ACLs, comments and data, and
+copy pages verbatim so sparse large objects stay sparse. Moving an object to
+native storage also reinstates its `pg_shdepend` entries, so `DROP ROLE`,
+`REASSIGN OWNED` and `DROP OWNED` continue to see it.
+
+`migrate_to_native()` does not require lolor to be enabled; it operates on the
+catalogs directly.
+
+Security labels on large objects cannot be represented in lolor storage and
+cannot be reinstated without their label provider, so `migrate_from_native()`
+refuses rather than discarding them. Remove them first if you hit this.
+
+Two helpers support verifying a migration:
+
+```sql
+SELECT * FROM lolor.digest();         -- per-object checksum, compare across nodes
+SELECT * FROM lolor.check_orphans();  -- lolor objects whose owner no longer exists
+```
 
 When the spock extension is installed, both migration functions run under
 `spock.repair_mode()`, so the row-shuffling migration DML is **not**
@@ -101,6 +118,7 @@ Even with spock, migration is refused if a non-spock logical replication slot
 own output plugin and those consumers would still decode the migration DML:
 `migrate_to_native()` raises an `ERROR`, while `migrate_from_native()` warns and
 returns -1 without doing anything. Drop the offending slots before migrating.
+The check identifies spock's slots by their `spock_output` plugin.
 
 Without spock, the migration DML cannot be excluded from logical decoding, so
 both functions refuse to migrate while logical replication slots exist in the
@@ -111,8 +129,19 @@ doing anything (0 is reserved for "nothing to migrate"), while
 migrating; merely disabling a subscription is not sufficient, since its slot
 retains the changes and delivers them when replication resumes.
 
+### Security
+
+`lo_import()` and `lo_export()` read and write files on the server host. As in
+core PostgreSQL, `EXECUTE` on them is revoked from `PUBLIC`; grant it
+deliberately if a non-superuser needs server-side file access.
+
+Versions 1.0 through 1.3.0 left these two functions executable by every
+database user. Upgrading to 1.4.0 revokes the privilege; see the release notes.
+
 ### Limitations
 
 - Native large object functionality cannot be used while you are using the lolor extension.
-- lolor does not support the following statements: `ALTER LARGE OBJECT`, `GRANT ON LARGE OBJECT`, `COMMENT ON LARGE OBJECT`, and `REVOKE ON LARGE OBJECT`.
-- Large object migration is node-local. Native large objects live in `pg_catalog.pg_largeobject`, which is never replicated, so each node holds an independent set and `migrate_from_native()` migrates only the local node's objects; with spock installed, the migration DML runs in repair mode and is not replicated. Run the migration on every node that holds native large objects — for example with `spock.replicate_ddl('SELECT lolor.migrate_from_native()')`, which queues the command so that each node executes it locally. Migrated objects keep their original native OIDs, which are not node-encoded: if different nodes hold different objects under the same OID, the nodes' lolor contents will diverge and later replicated changes to those objects can conflict. Newly created large objects are collision-free, since new OIDs are node-encoded via `lolor.node` and checked against existing rows.
+- lolor does not support the following statements against objects held in lolor storage: `ALTER LARGE OBJECT`, `GRANT ON LARGE OBJECT`, `COMMENT ON LARGE OBJECT`, and `REVOKE ON LARGE OBJECT`. Owners, ACLs and comments set while an object was in native storage are preserved across migration in both directions.
+- Objects in lolor storage are rows in ordinary tables and so cannot participate in `pg_shdepend`: `DROP ROLE` will not notice that a role still owns them, the way it does for native large objects. Use `lolor.check_orphans()` to find objects whose owner has been dropped.
+- `lolor.enable()` and `lolor.disable()` change which function OID owns each `pg_catalog.lo_*` name. libpq resolves those OIDs once per connection and caches them, so existing client sessions must reconnect afterwards.
+- Large object migration is node-local. Native large objects live in `pg_catalog.pg_largeobject`, which is never replicated, so each node holds an independent set and `migrate_from_native()` migrates only the local node's objects; with spock installed, the migration DML runs in repair mode and is not replicated. Run the migration on every node that holds native large objects — for example with `spock.replicate_ddl('SELECT lolor.migrate_from_native()')`, which queues the command so that each node executes it locally. Migrated objects keep their original native OIDs, which are not node-encoded: if different nodes hold different objects under the same OID, the nodes' lolor contents will diverge and later replicated changes to those objects can conflict. Newly created large objects are collision-free, since new OIDs are node-encoded via `lolor.node` and checked against existing rows. To make that hazard an error rather than silent divergence, collect the other nodes' OIDs with `lolor.native_lo_oids()` and pass them in: `SELECT lolor.migrate_from_native(peer_oids => ARRAY[...])` refuses when any of them collide. After migrating every node, compare `lolor.digest()` across nodes to confirm they converged.

docker/entrypoint.sh

@@ -9,8 +9,20 @@ echo ". /home/pgedge/pgedge/pg${PG_VER}/pg${PG_VER}.env" >> /home/pgedge/.bashrc
 # deprecated pgedge CLI.
 initdb -D "$PGDATA" -U admin --encoding=UTF8 --locale=C
 
+# PostgreSQL 16.15, 17.11, 18.6 and 19 only allow logical decoding through an
+# output plugin named in output_plugin_libraries, which defaults to
+# 'pgoutput, test_decoding'.  spock's plugin is not in that list, so
+# CREATE_REPLICATION_SLOT fails, every apply worker exits and restarts, and no
+# subscription ever reaches sync.  Older minors do not have the parameter and
+# refuse to start when it appears in the configuration, so ask the binary.
+OUTPUT_PLUGIN_LIBRARIES=""
+if postgres --describe-config 2>/dev/null | grep -q '^output_plugin_libraries'; then
+  OUTPUT_PLUGIN_LIBRARIES="output_plugin_libraries = 'pgoutput, test_decoding, spock_output'"
+fi
+
 cat >> "$PGDATA/postgresql.conf" <<_EOF_
 listen_addresses = '*'
+$OUTPUT_PLUGIN_LIBRARIES
 wal_level = logical
 track_commit_timestamp = on
 max_worker_processes = 32
@@ -33,6 +45,7 @@ while ! pg_isready -h /tmp; do
   sleep 1
 done
 
+
 # The admin user is what the tests connect as; the pgedge user is used for
 # the spock node and subscription DSNs (and matches the OS user, so plain
 # psql on the nodes works).
@@ -51,20 +64,30 @@ _EOF_
 
 IFS=',' read -r -a peer_names <<< "$PEER_NAMES"
 
+# Bounded: an unbounded wait here leaves the container alive with the
+# temporary postgres still listening, so pg_isready and the health check keep
+# succeeding while setup never finishes.  A node stuck in that state looks
+# healthy to the test harness, which then blocks with no indication of why.
 for PEER_HOSTNAME in "${peer_names[@]}";
 do
-  while :
+  peer_ready=0
+  for attempt in $(seq 1 300); do
+    mapfile -t node_array < <(psql -A -t demo -h $PEER_HOSTNAME -c "SELECT node_name FROM spock.node;")
+    for element in "${node_array[@]}";
     do
-      mapfile -t node_array < <(psql -A -t demo -h $PEER_HOSTNAME -c "SELECT node_name FROM spock.node;")
-      for element in "${node_array[@]}";
-      do
-        if [[ "$element" == "$PEER_HOSTNAME" ]]; then
-            break 2
-        fi
-      done
-      sleep 1
-      echo "Waiting for $PEER_HOSTNAME..."
+      if [[ "$element" == "$PEER_HOSTNAME" ]]; then
+          peer_ready=1
+          break
+      fi
     done
+    [ "$peer_ready" = "1" ] && break
+    sleep 1
+    echo "Waiting for $PEER_HOSTNAME..."
+  done
+  if [ "$peer_ready" != "1" ]; then
+    echo "ERROR: peer $PEER_HOSTNAME did not register a spock node within 300s" >&2
+    exit 1
+  fi
 done
 
 # spock.sub_create connects to the provider synchronously, and the peer

docs/lolor_release_notes.md

@@ -1,5 +1,26 @@
 # lolor Release Notes
 
+## lolor 1.4.0
+
+* Rewrote large object migration between native and lolor storage as a direct relation-to-relation copy in C (`lolor.migrate_storage()`), replacing the previous reverse-migration loop that rewrote every object through the large object API. Fidelity and correctness fixes that follow from it:
+  * **Ownership and ACLs are now recorded in `pg_shdepend`.** Through 1.3.0 the reverse migration set `lomowner` with a raw catalog `UPDATE`, which left no shared dependency: `DROP ROLE` would succeed on a role that still owned migrated large objects, `REASSIGN OWNED` skipped them, and `DROP OWNED BY` the migrating superuser could delete other users' objects.
+  * **Comments are preserved.** `COMMENT ON LARGE OBJECT` text was silently discarded on migration. It is now parked in `lolor.pg_largeobject_description` while the object is in lolor storage and reinstated on the way back.
+  * **Sparse large objects stay sparse.** Pages are copied verbatim instead of being rewritten through the write API, which previously zero-filled every gap — a sparse 10 MB object materialised as 10 MB of pages.
+  * Native objects are now removed through `performMultipleDeletions()`, the same path `DROP` uses, so shared dependencies, comments and security labels are cleaned up rather than left behind.
+  * Migration no longer routes through the renamed `_orig` functions, so `migrate_to_native()` (and `DROP EXTENSION`) works whether or not lolor is enabled. This resolves the previous "lolor must be enabled before migration to native" failure.
+  * Storage layouts are verified against the running server's catalogs at migration time, so a future PostgreSQL catalog change produces a clear error instead of silent corruption.
+  * `migrate_from_native()` refuses rather than discarding large object security labels, which lolor storage cannot represent.
+  * OID conflicts are reported with the conflicting OIDs instead of only their existence.
+* **Security fix: `lo_import()` and `lo_export()` were executable by any database user.** These functions read and write files on the server as the operating system account PostgreSQL runs under, and core revokes `EXECUTE` on them from `PUBLIC`. lolor replaces them by renaming the originals to `*_orig`; an ACL belongs to a function rather than to a name, so the restriction stayed behind on the parked original while each replacement was created with the default of `EXECUTE TO PUBLIC`. Any user could therefore read an arbitrary server file with `lo_import()` or overwrite one with `lo_export()`. The replacements are now locked down at install time, and the upgrade to 1.4.0 revokes the privilege on existing installations in either the enabled or the disabled state. All versions from 1.0 through 1.3.0 are affected.
+* Fixed the `lolor.node` upper bound. The GUC accepted 0..16 while a generated OID reserves only four bits for the node id, so node 16 did not fit and was silently encoded as node 0. The bound is now derived from the encoding (`LOLOR_MAX_NODE_ID`), giving a valid range of 0..15; a configuration using node 16 will now be rejected rather than mis-encoding its OIDs.
+* Migration now holds `ShareRowExclusiveLock` on both stores until the transaction commits. Ordinary large object reads and writes take `RowExclusiveLock`, which does not conflict with itself, so a concurrent `lo_write()` could previously commit between the point where the migration copied a page and the point where it emptied the source store — losing the write with no error. Two migrations running in opposite directions also now take their locks in a fixed order and cannot deadlock against each other.
+* Cleanup now runs for every spelling of the drop. `DROP SCHEMA lolor CASCADE` and `DROP OWNED BY` reach the extension by dependency cascade rather than as `DROP EXTENSION`; the event trigger did not fire for them, so the large objects were destroyed along with the lolor tables and `pg_catalog` was left without a working `lo_open()`.
+* `lolor.enable()`, `lolor.disable()` and `lolor.is_enabled()` now probe exact function signatures in `pg_catalog`. They previously matched on `proname` across every schema, so any user with `CREATE` on any schema could create a function named `lolor_lo_open` and permanently wedge lolor into an "inconsistent state" — which also blocked `DROP EXTENSION`. Both functions now also take an advisory lock so concurrent calls serialise.
+* The extension is no longer marked `trusted`. Installing lolor renames functions in `pg_catalog` for the whole database, which is not an operation a non-superuser should be able to perform.
+* New helpers: `lolor.digest()` for comparing lolor storage across nodes after a node-local migration, `lolor.check_orphans()` for finding objects whose owner has been dropped, and `lolor.native_lo_oids()` plus a `peer_oids` argument to `lolor.migrate_from_native()` that turns the documented cross-node OID collision hazard into a pre-flight refusal.
+* `lolor.enable()` and `lolor.disable()` now emit a notice that client sessions must reconnect, since libpq caches the large object function OIDs per connection.
+* Regression tests added for sparse objects, comment round trips, `pg_shdepend` restoration, `DROP SCHEMA CASCADE` cleanup, dropping the extension while disabled, and name-squatting resistance. The committed expected output was stale and has been regenerated.
+
 ## lolor 1.3.0
 
 * Add bidirectional large object migration between native PostgreSQL and lolor storage:

docs/pg_upgrade_with_lolor.md

@@ -25,3 +25,9 @@ Then, use psql to enable `lolor`:
 ```
 db1_18=# SELECT lolor.enable();
 ```
+
+Reconnect any client sessions afterwards. `lolor.enable()` and
+`lolor.disable()` change which function OID owns each `pg_catalog.lo_*` name,
+and libpq resolves those OIDs once per connection and caches them for the life
+of the connection. A session that used a large object before the switch would
+otherwise keep calling the previous implementation.