Skip to content

Commit 80d37b7

Browse files
committed
pg-upgrade-test: run ALTER EXTENSION UPDATE before binary pg_upgrade, not after
Reorders prepare-old -> update -> pg_upgrade -> run-suite (was prepare-old -> pg_upgrade -> update -> run-suite). The old order proved pg_upgrade could migrate 0.9.6's frozen objects, then updated afterward - not actionable, since that version already shipped. This job's whole point is proving pg_upgrade correctly migrates the objects count_nulls' CURRENT code creates, which requires updating BEFORE the binary upgrade runs. make install (into the old cluster) already happens earlier in the job, so the current version's update scripts are on disk in time for the moved step. Updates the job's step names/comments and bin/test_existing's own file-header sequence description to match the new order.
1 parent 819132e commit 80d37b7

2 files changed

Lines changed: 50 additions & 29 deletions

File tree

.github/workflows/ci.yml

Lines changed: 40 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,18 @@
1717
# per-PG-version container/checkout setup for
1818
# no added confidence.
1919
# pg-upgrade-test -- BINARY pg_upgrade: install 0.9.6 on an OLD
20-
# PostgreSQL major, binary-upgrade the cluster
21-
# to a NEWER major, then update the extension
22-
# to current - proves objects created on an
23-
# old server still work when read on a new
24-
# one. A smaller old_pg/new_pg matrix (not the
25-
# full PG matrix - by far the most expensive
26-
# job here, installing two full PostgreSQL
27-
# majors and running the real pg_upgrade
28-
# binary per leg).
20+
# PostgreSQL major, update the extension to
21+
# current (still on the old major), THEN
22+
# binary-upgrade the cluster to a NEWER major -
23+
# proves pg_upgrade correctly migrates the
24+
# objects the extension actually creates
25+
# TODAY, not objects frozen at some past
26+
# version (which would be untestable anyway -
27+
# that old version already shipped). A smaller
28+
# old_pg/new_pg matrix (not the full PG matrix
29+
# - by far the most expensive job here,
30+
# installing two full PostgreSQL majors and
31+
# running the real pg_upgrade binary per leg).
2932
# pg-tle-test -- pg_tle DEPLOYMENT: fresh install registered
3033
# through AWS pg_tle's database-backed catalog
3134
# instead of a filesystem .control file.
@@ -213,13 +216,19 @@ jobs:
213216
run: make verify-results TEST_LOAD_SOURCE=update
214217

215218
# Proves count_nulls survives a BINARY pg_upgrade (in-place catalog
216-
# migration to a newer PostgreSQL major), not just an in-place extension
217-
# update. Installs 0.9.6 on an old cluster, plants a dependency guard,
218-
# binary-pg_upgrades to a newer cluster, updates the extension to current,
219+
# migration to a newer PostgreSQL major). Installs 0.9.6 on an old
220+
# cluster, plants a dependency guard, updates the extension to CURRENT
221+
# (still on the old major), THEN binary-pg_upgrades to a newer cluster,
219222
# then runs the suite against the REAL migrated objects in existing mode.
220-
# No bridge-update step first: count_nulls has always been pure SQL
221-
# functions with no SELECT-*-over-catalog views, so it has no known
222-
# pg_upgrade-unsafe old version to bridge past.
223+
# Updating before the binary upgrade (not after) is deliberate: the whole
224+
# point of this job is proving pg_upgrade correctly migrates the objects
225+
# count_nulls' CURRENT code actually creates - migrating 0.9.6's objects
226+
# and updating afterward would instead test whether pg_upgrade can
227+
# migrate a legacy structure frozen in the past, which isn't actionable
228+
# (that version already shipped; nothing to fix if it turned out
229+
# fragile). No bridge-update step first: count_nulls has always been
230+
# pure SQL functions with no SELECT-*-over-catalog views, so it has no
231+
# known pg_upgrade-unsafe old version to bridge past.
223232
#
224233
# Deliberately not doing a stepwise every-major-in-sequence climb (one
225234
# cluster walking 10->11->12->...->newest, vs. the single big jumps here):
@@ -268,6 +277,17 @@ jobs:
268277
# in this job cannot silently make the eventual existing-mode run
269278
# test a fresh install instead.
270279
run: bin/test_existing prepare-old count_nulls_upgrade "" 0.9.6
280+
- name: Update the extension to the current version (still on the old cluster)
281+
# Exercises ALTER EXTENSION UPDATE on the OLD cluster, BEFORE the
282+
# binary pg_upgrade below, running the 0.9.6->stable update script -
283+
# deliberately in this order (not update-after-upgrade): this job
284+
# exists to prove pg_upgrade correctly migrates the objects
285+
# count_nulls' CURRENT code creates, so pg_upgrade must run against
286+
# already-current objects, not 0.9.6 ones. `make install` above
287+
# already installed the current version's update scripts/control
288+
# file into this (old) cluster's sharedir, so they're in place for
289+
# this ALTER EXTENSION UPDATE to use.
290+
run: bin/test_existing update count_nulls_upgrade
271291
- name: Install PostgreSQL ${{ matrix.new_pg }}
272292
run: apt-get install -y postgresql-${{ matrix.new_pg }} postgresql-server-dev-${{ matrix.new_pg }}
273293
- name: Install count_nulls into new cluster
@@ -294,18 +314,13 @@ jobs:
294314
/var/lib/postgresql/${{ matrix.new_pg }}/test/pg_upgrade_output.d \
295315
-name '*.log' 2>/dev/null | sort | xargs -r tail -n +1; exit 1; }
296316
pg_ctlcluster ${{ matrix.new_pg }} test start
297-
- name: Update the pg_upgraded extension to the current version
298-
# Exercises ALTER EXTENSION UPDATE on genuinely pg_upgraded objects
299-
# (the extension binary pg_upgrade just migrated), running the
300-
# 0.9.6->stable update script.
301-
run: bin/test_existing update count_nulls_upgrade
302317
- name: Run the suite against the pg_upgraded database (existing mode)
303318
# run-suite asserts the version, re-proves the dependency guard
304-
# still blocks a non-CASCADE drop (i.e. it survived pg_upgrade),
305-
# drops the guard, then runs the suite against the REAL pg_upgraded
306-
# + updated database via --use-existing (so pg_regress does not
307-
# drop/recreate it) - a plain fresh `make test` would silently test
308-
# a fresh install instead of the migrated objects.
319+
# still blocks a non-CASCADE drop (i.e. it survived both the update
320+
# and pg_upgrade), drops the guard, then runs the suite against the
321+
# REAL pg_upgraded database via --use-existing (so pg_regress does
322+
# not drop/recreate it) - a plain fresh `make test` would silently
323+
# test a fresh install instead of the migrated objects.
309324
run: bin/test_existing run-suite count_nulls_upgrade ""
310325

311326
pg-tle-test:

bin/test_existing

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,17 @@
1212
#
1313
# The pg-upgrade-test CI job repeats the same sequence:
1414
#
15-
# prepare-old (install + plant guard) -> [real pg_upgrade binary, in CI] ->
16-
# update (ALTER EXTENSION UPDATE) -> run-suite (assert + run existing-mode)
15+
# prepare-old (install + plant guard) -> update (ALTER EXTENSION UPDATE,
16+
# on the OLD cluster, before pg_upgrade) -> [real pg_upgrade binary, in
17+
# CI] -> run-suite (assert + run existing-mode)
1718
#
18-
# so it lives here once instead of being duplicated as inline YAML. Not
19-
# CI-only: a developer can run any subcommand locally against a scratch
19+
# so it lives here once instead of being duplicated as inline YAML. update
20+
# runs BEFORE the binary pg_upgrade, not after: the point of this job is
21+
# proving pg_upgrade correctly migrates the objects count_nulls' CURRENT
22+
# code creates, so pg_upgrade needs to run against already-current objects,
23+
# not ones still frozen at the old INSTALL_VERSION.
24+
#
25+
# Not CI-only: a developer can run any subcommand locally against a scratch
2026
# database. Modeled on Postgres-Extensions/cat_tools's bin/test_existing.
2127
# Two differences from that script: count_nulls ships no
2228
# SELECT-*-over-catalog views, so it has no known pg_upgrade-unsafe old

0 commit comments

Comments
 (0)