release(pg_tle): preserve data across 0.3 upgrades

[NikolayS]

Summary

• register the 0.3 standalone pg_tle version, tested 0.2.0 → 0.3 update path, and new default atomically
• stop directing unknown upgrade paths toward destructive uninstall
• add a real 0.2.0 TLE fixture with queue, subscription, pending, retry, and DLQ state
• verify ALTER EXTENSION preserves all state and activates the new send_idem API
• document the explicit registration/update sequence

Fixes #308.

Validation

• reproduced the old wrapper rejecting a live 0.2.0 extension and recommending uninstall
• real pg_tle v1.5.2 0.2.0 → 0.3.0-devel update test passed
• fresh pg_tle install, rerun, drop, and idempotent unregister test passed
• full tests/run_all.sql passed through a fresh pg_tle install
• generator ran twice with identical output hashes
• bash -n build/transform.sh and git diff --check passed

actionlint reports only the workflow's pre-existing shellcheck warnings; the added CI step introduces no new warning.

[NikolayS]

REV rubric review

This is a manual execution of the repository's five applicable REV rubrics because the automated ultrareview quota was unavailable. It is not bot output. SOC2 was omitted.

• Bug/security: known updates preserve state and unknown paths refuse destructive guidance; no additional high-confidence defect or security issue found.
• Tests: real pg_tle update/fresh/repeat/unsupported paths are covered; CI is 17/17 green. Initial RED is retrospective.
• Docs/guidelines: final transform shell hardening resolves the touched-file IFS gap. Historical release: is unsupported by both rule sets and exactly 50 characters; test:/style: also conflict with the repo-specific list.

Blocking patch findings: none. Remaining constraints: preserve history, use a compliant squash/merge subject/title, and post real pg_tle evidence before merge. This COMMENT review is not an approval.

[NikolayS]

Real-user pg_tle verification evidence for head b2f7786f35d90c0030ac868f3708a136da2908fd.

The upgrade test used pg_tle v1.5.2 with a real tagged v0.2.0 extension registration. The documented local sequence was exercised:

PAGER=cat psql --no-psqlrc "$PGQUE_TEST_DSN" \
  --command='create extension pg_tle'
git show v0.2.0:sql/pgque-tle.sql \
  | PAGER=cat psql --no-psqlrc "$PGQUE_TEST_DSN" --set=ON_ERROR_STOP=1
PAGER=cat psql --no-psqlrc "$PGQUE_TEST_DSN" \
  --command='create extension pgque'
PAGER=cat psql --no-psqlrc "$PGQUE_TEST_DSN" --set=ON_ERROR_STOP=1 \
  --file=tests/test_tle_upgrade.sql

tests/test_tle_upgrade.sql builds queue, subscription, pending-event, retry, and DLQ state, registers the new version/update path, runs ALTER EXTENSION pgque UPDATE, then exercises the new send_idem API. The pre-fix wrapper rejected the live extension and instructed the user to uninstall:

ERROR: pgque is already registered with pg_tle at version 0.2.0 but this script registers version 0.3.0-devel. Run devel/sql/pgque-tle-uninstall.sql first ...

The head passed:

NOTICE: pgque 0.3.0-devel already registered with pg_tle; no registration changes needed.
PASS: 0.2.0 pg_tle state survived and 0.3 APIs work
=== test_tle_upgrade: ALL PASSED ===

I also exercised an unsupported 0.1.0 path. It failed without suggesting destructive uninstall and preserved the installed state:

ERROR: pgque has no tested pg_tle update path from version 0.1.0 to 0.3.0-devel. Keep the existing extension installed ... do not uninstall it because that would drop queue data.
PASS: unsupported pg_tle update failed without changing state

Fresh pg_tle install, idempotent wrapper rerun, extension drop/unregister, and the full regression suite through the pg_tle install path passed. bash build/transform.sh produced identical generated output on repeat; bash -n build/transform.sh, ShellCheck for the changed shell, and git diff --check passed.

RED/GREEN chronology: retrospective. The main pg_tle upgrade regression was added after the implementation rather than as a published RED commit. I independently confirmed the pre-fix rejection and the head's GREEN behavior.

Full CI: 17/17 checks passed.

This comment records test evidence only; it is not a review or merge decision.

[NikolayS]

samorev Code Review Report

• PR: release(pg_tle): preserve data across 0.3 upgrades #333 - release(pg_tle): preserve data across 0.3 upgrades
• Author: @NikolayS
• AI-Assisted: Yes

Pipeline	Coverage
unknown	Not reported

---

REVIEW FINDINGS (8)

HIGH MR/PR state - Review target is draft

The review target is still marked as draft.
Fix: Mark it ready for review before merge.

CRITICAL CI/Pipeline - Pipeline status is unknown

Provider CI reported status unknown.
Fix: Fix failing checks and rerun review.

HIGH [bugs] The 0.2.0 → 0.3.0-devel update path is registered with the complete fresh-install body rather than a migration script. Because the body must be idempotent for the test to pass (alter extension pgque update succeeded against a populated 0.2.0 schema), any DDL guarded by if not exists is silently skipped on the upgrade path. New columns, constraints, indexes, or defaults added to pre-existing tables between 0.2.0 and 0.3 will not be applied, and objects removed or renamed in 0.3 survive as stale extension members. The result is an "upgraded" 0.3 database whose schema differs from a fresh 0.3 install, with no error to signal it. The test only proves old rows survive and that one new function (send_idem) exists — both of which create or replace function and create table if not exists deliver even when table migrations are skipped.

Fix: Register a dedicated 0.2.0--0.3.0 migration script (explicit alter table / drop function statements for whatever changed) instead of reusing extension_sql; if the two really are schema-identical, prove it in CI by adding a step that installs 0.3 fresh in a second database and diffs the two catalogs (e.g. compare pg_attribute/pg_proc/pg_constraint projections for the pgque schema, or pg_dump --schema-only --schema=pgque of both DBs) and fails on any difference.

MEDIUM [bugs] The wrapper decides the update source from the registered default (pgtle.available_extensions().default_version) rather than the version actually installed in this database (pg_extension.extversion). Because registration and alter extension ... update are deliberately separated (README: "The registration step does not change the active extension"), these two values legitimately diverge for as long as an operator delays the update. Concrete failure: DB is registered+installed at 0.2.0 → operator runs this wrapper (default becomes 0.3.0-devel, path 0.2.0→0.3.0-devel installed) → operator postpones alter extension pgque update → the next release's wrapper reads default_version = '0.3.0', accepts it as the source, installs a 0.3.0→0.4.0 path and sets default 0.4.0. The extension is still at 0.2.0 and alter extension pgque update now fails with "no update path from version 0.2.0 to version 0.4.0". The same mismatch makes the guard's error message name the registered version rather than the one the user actually has installed.

Fix: Read the installed version too (select extversion from pg_catalog.pg_extension where extname = 'pgque') and drive both the guard and install_update_path's source argument off it when the extension is installed, falling back to default_version only for the not-installed case. Add a regression test covering "default_version advanced, extension still at 0.2.0" so the divergence is exercised.

MEDIUM [tests] test_tle_upgrade.sql mutates queue configuration before the upgrade (max_retries 0 → 5) but never asserts it afterwards. Per-queue config is exactly the kind of user-owned state that re-executing the install body can clobber — if the 0.3 body re-seeds pgque config/defaults on the update path, max_retries would silently revert to the shipped default and every existing assertion would still pass. The test's stated purpose is "each durable event location plus the consumer cursor state", and config is a gap in that coverage.

Fix: In the post-alter extension pgque update block, assert the queue config round-trips: assert pgque.get_queue_config('tle_upgrade', 'max_retries') = '5', 'queue config was reset during pg_tle update'; (adjust to the actual getter/table).

MEDIUM [tests] The header claims the fixture covers "the consumer cursor state that users would lose on uninstall/reinstall", but no assertion verifies the cursor is still usable after the update. The post-upgrade block checks only that a pgque.subscription row exists and that the pending event row is present in the event table — an upgrade that reset the subscription's tick cursor (or re-created the subscription at the current tick) would pass all current assertions while making the pending event permanently undeliverable to worker.

Fix: After the update, drive an actual delivery: select pgque.force_next_tick('tle_upgrade'); select pgque.ticker(); then receive('tle_upgrade','worker',1) and assert the returned msg_id equals the recorded pending event id, then ack. That proves the cursor, not just the row's existence.

LOW [tests] The rejection test's success message — "unsupported pg_tle update failed without changing state" — is stronger than what is verified. psql runs each top-level statement in its own transaction, so only the final do $wrapper$ block is atomic; every preceding statement in devel/sql/pgque-tle.sql (the pg_tle preflight in Step 1/Step 2, including any role creation or other cluster-level side effects) has already committed by the time the guard raises. The assertions cover only pg_extension.extversion, default_version, and one data row, so committed pre-guard side effects are invisible to the test.

Fix: Either narrow the claim (e.g. "failed without changing the installed extension or its data") or extend the assertions to cover whatever Step 1/Step 2 can commit (roles, schemas), so the message and the checks agree.

LOW [docs] The new README section documents \i sql/pgque-tle.sql followed by alter extension pgque update, but this PR only changes devel/sql/pgque-tle.sql; sql/pgque-tle.sql is not in the changed-file list, so at this commit the documented path still ships the old wrapper that refuses a differing registration and points users at the uninstall script — the exact behaviour the README now tells them to ignore. The section also omits that registration requires pgtle_admin membership, which is the most common reason the documented sequence fails for a non-superuser.

Fix: Gate the README text on the stable-release promotion (or say explicitly that it applies from 0.3 onward and that pre-0.3 packages lack the update path), and add a note that the registering role must be a member of pgtle_admin.

---

Summary

Area	Findings	Potential	Filtered
CI/Pipeline	1	0	0
Security	0	0	0
Bugs	0	2	0
Tests	0	3	0
Guidelines	0	0	0
Docs	0	1	0
Metadata	1	0	0

Note:

• Findings: High-confidence issues (8-10/10) - blocking or non-blocking per severity
• Potential: Medium-confidence issues (4-7/10) - review manually
• Filtered: Low-confidence issues (0-3/10) - excluded as likely false positives

Review metadata

provider=github
kind=pr
project=NikolayS/PgQue
number=333
target=github:NikolayS/PgQue#333
state=OPEN
draft=true
diff_lines=610
diff_added=410
diff_removed=45
diff_bytes=24595
comments_count=1
commits_count=5
ci_status=unknown
ci_summary=total=18 success=17 failure=0 pending=0 other=1
prompt=.claude/commands/review-mr.md
blocking=false
posted_by=gh
no_comment=false
live_posting=posted

---

samorev-assisted review (AI analysis by Tanya301/samorev)