From 4a5a7c9fd5a58a5eb190bc86c33ae92ec37ca368 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 31 Jul 2026 18:43:55 -0500 Subject: [PATCH 1/6] Phase 3.5: CI hygiene - docs-only gate, all-checks-passed, single-source PG list Independent of the U&U testing work itself, but best done now that multiple CI jobs exist and before the next phase adds the most expensive one (a real pg_upgrade job): - `changes` job: computes the actual per-push diff and skips test/ extension-update-test/pg-tle-test entirely on doc-only pushes, always triggering itself (no workflow-level paths-ignore, which would leave all-checks-passed stuck Pending on doc-only pushes in branch protection). - Derives the supported-PostgreSQL-major list from ONE set of constants (NEWEST/FLOOR) in that same job, consumed by both the `test` and `extension-update-test` matrices via fromJSON - they can't silently drift onto different lists, and a new major is a one-line change. - `all-checks-passed`: single stable required-status-check name, with a self-check that its own needs list can't silently omit a newly-added job. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/ci.yml | 192 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 191 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ba9a555..1848af8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,3 +1,40 @@ +# =========================================================================== +# Test strategy +# +# count_nulls can be arrived at several ways, each of which can break +# differently, so each is exercised by its own job below: +# +# test -- FRESH install: CREATE EXTENSION at the current +# version, across every supported PostgreSQL +# major. Also proves the IN-PLACE extension +# update path (CREATE EXTENSION at 0.9.6, then +# ALTER EXTENSION UPDATE - same PostgreSQL, no +# pg_upgrade) in the same job/matrix, rather +# than a dedicated job: a load mode is just an +# input the same assertions run against, not a +# real environment difference, so giving it its +# own job would only duplicate this job's own +# per-PG-version container/checkout setup for +# no added confidence. +# pg-tle-test -- pg_tle DEPLOYMENT: fresh install registered +# through AWS pg_tle's database-backed catalog +# instead of a filesystem .control file. +# +# Every TEST_SCHEMA value (empty - no schema targeting at all - and +# 'Quoted', a name requiring SQL identifier quoting) is exercised too, via +# `make test-schema-all`'s in-Makefile loop rather than a CI matrix +# dimension - a schema name is just an input the same assertions run +# against, not a real environment difference, so crossing it into the +# matrix would only multiply job count for no added confidence (see the +# Makefile's TEST_SCHEMA_VALUES comment). Every leg passes against the SAME +# test/expected/extension_tests.out (see test/README.md for how the suite +# keeps its output schema-invariant). +# +# `changes` is a cheap gate that lets the heavy jobs above skip themselves on +# doc-only pushes, and also derives the shared PostgreSQL-major list those +# jobs consume from a single set of constants. `all-checks-passed` is the +# single stable required-status-check name. +# =========================================================================== name: CI on: push: @@ -5,6 +42,114 @@ on: - master pull_request: jobs: + # Cheap gate that lets the heavy jobs below skip themselves on commits that + # touch only docs. Must run on every push/pull_request (no paths-ignore on + # the workflow itself), otherwise the required all-checks-passed check + # would never report on doc-only pushes and get stuck Pending in branch + # protection. + # + # Also derives, from a SINGLE set of constants, the supported-PostgreSQL- + # major list the test job consumes: every job that cares which majors are + # supported reads the SAME list, so they can't silently drift onto + # different sets, and adding a new major is a one-line change here + # instead of an edit in several jobs. + changes: + name: ๐Ÿ” Detect docs-only changes & derive PG matrix + runs-on: ubuntu-latest + outputs: + docs_only: ${{ steps.diff.outputs.docs_only }} + supported_pg: ${{ steps.pg.outputs.supported_pg }} + steps: + - name: Check out the repo + uses: actions/checkout@v4 + with: + # Full history needed so BASE and HEAD below are both reachable + # for `git diff`. + fetch-depth: 0 + - name: Compute per-push changed files + id: diff + run: | + # Fail safe to running the full matrix: default docs_only to false + # immediately, before anything below has a chance to compute or + # fail. Writing the same GITHUB_OUTPUT key twice is fine (the last + # write wins), so the only way this step ends with docs_only=true + # is by genuinely proving it further down - never by skipping past + # an edge case with a default. + echo "docs_only=false" >> "$GITHUB_OUTPUT" + + if [ "${{ github.event_name }}" = "pull_request" ] && \ + [ "${{ github.event.action }}" = "synchronize" ] && \ + [ -n "${{ github.event.before }}" ]; then + # A push to an already-open PR: before/after give the true + # per-push diff, same as for a branch push. + BASE="${{ github.event.before }}" + HEAD="${{ github.event.after }}" + elif [ "${{ github.event_name }}" = "pull_request" ]; then + # First run for this PR (opened/reopened/etc, or synchronize + # without a usable before): fall back to the whole base...head + # diff. + BASE="${{ github.event.pull_request.base.sha }}" + HEAD="${{ github.event.pull_request.head.sha }}" + else + BASE="${{ github.event.before }}" + HEAD="${{ github.event.after }}" + fi + + echo "base=$BASE" + echo "head=$HEAD" + + # A missing HEAD, or an all-zeros BASE (e.g. a new branch's first + # push, where GitHub reports no prior commit), means we can't + # compute a real diff. docs_only is already false from above; + # just stop here rather than risk skipping tests. + if [ -z "$HEAD" ] || [ -z "$BASE" ] || [[ "$BASE" =~ ^0+$ ]]; then + exit 0 + fi + + CHANGED=$(git diff --name-only "$BASE" "$HEAD" || echo __DIFF_FAILED__) + + DOCS_ONLY=true + if [ "$CHANGED" = "__DIFF_FAILED__" ] || [ -z "$CHANGED" ]; then + DOCS_ONLY=false + else + while IFS= read -r f; do + if ! [[ "$f" =~ \.(md|asc)$ ]]; then + DOCS_ONLY=false + break + fi + done <<< "$CHANGED" + fi + + echo "changed files:" + echo "$CHANGED" + echo "docs_only=$DOCS_ONLY" >> "$GITHUB_OUTPUT" + + - name: Derive the supported-PostgreSQL-major list + id: pg + run: | + # A dozen-odd lines to replace what looks like a handful of version + # references, but it buys CONSISTENCY: the fresh-install/update + # `test` matrix derives its PostgreSQL set from this ONE source, so + # it cannot silently drift onto a different list. Adding a new + # major is a one-line NEWEST bump here, not an edit in N places. + # + # Only one floor is needed here: 0.9.6 (the oldest version + # count_nulls still ships a full install script for) is pure SQL + # over anyarray/json/jsonb with no catalog-version sensitivity, so + # it installs on every PostgreSQL major count_nulls supports - + # there's no separate legacy-only floor to carve out. + NEWEST=18 + FLOOR=10 + + supported=$(seq "$NEWEST" -1 "$FLOOR") + + # Emit a JSON array from a list of ints, for the job matrices to + # consume with fromJSON (GitHub evaluates a literal dollar-brace + # expression even inside a run block, so none is written here). + json() { printf '%s\n' "$@" | paste -sd, - | sed 's/^/[/; s/$/]/'; } + + echo "supported_pg=$(json $supported)" >> "$GITHUB_OUTPUT" + lint: name: ๐Ÿงน SQL lint runs-on: ubuntu-latest @@ -36,9 +181,12 @@ jobs: # test/expected/extension_tests.out (see test/README.md for how the # suite keeps its output schema-invariant). test: + needs: [changes] + if: needs.changes.outputs.docs_only != 'true' strategy: matrix: - pg: [18, 17, 16, 15, 14, 13, 12, 11, 10] + # From the single source in the changes job. + pg: ${{ fromJSON(needs.changes.outputs.supported_pg) }} name: ๐Ÿ˜ PostgreSQL ${{ matrix.pg }} runs-on: ubuntu-latest container: pgxn/pgxn-tools @@ -55,6 +203,8 @@ jobs: run: make verify-results TEST_LOAD_SOURCE=update pg-tle-test: + needs: [changes] + if: needs.changes.outputs.docs_only != 'true' strategy: matrix: # Intersection of count_nulls' own supported range (10-18, see the @@ -173,3 +323,43 @@ jobs: fi - name: Verify no stray extension control files after the fresh-install smoke test run: bin/assert_fs_clean verify ${{ matrix.pg }} /tmp/control_baseline.txt pg_tle.control + + # A single stable check name for use as a required status check in branch + # protection rules. Matrix jobs produce check names like + # "๐Ÿ˜ PostgreSQL 14 (schema none)" which would all need to be listed + # individually and updated whenever the matrix changes. This job passes if + # all others passed or were skipped (e.g. the heavy jobs gated off by the + # `changes` job on a docs-only push), and fails if any failed or were + # cancelled. + all-checks-passed: + needs: [changes, lint, test, pg-tle-test] + if: always() + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Verify all jobs are listed in needs + # Ensures this job won't silently ignore a newly-added job that was + # omitted from the needs list above. + run: | + DEFINED=$(python3 -c " + import yaml + with open('.github/workflows/ci.yml') as f: + w = yaml.safe_load(f) + print('\n'.join(sorted(j for j in w['jobs'] if j != 'all-checks-passed'))) + ") + NEEDED=$(echo '${{ toJson(needs) }}' | python3 -c " + import json, sys + print('\n'.join(sorted(json.load(sys.stdin)))) + ") + if [ "$DEFINED" != "$NEEDED" ]; then + echo "Some jobs are missing from all-checks-passed needs:" + diff <(echo "$DEFINED") <(echo "$NEEDED") + exit 1 + fi + - name: Check all jobs passed or were skipped + run: | + if [[ "${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') }}" == "true" ]]; then + echo "One or more jobs failed or were cancelled" + exit 1 + fi +# vi: expandtab ts=2 sw=2 From f7c8395080856b512ea29984db675f766879f25f Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Thu, 6 Aug 2026 17:04:18 -0500 Subject: [PATCH 2/6] CI: gate draft PRs down to lint + newest-PG test only The org-wide Actions runner queue backs up easily; a draft PR being actively iterated on doesn't need the full PG matrix or the heavy pg-tle-test job re-run on every push. Add a newest_pg scalar output (single source alongside supported_pg) and reduce the test job's matrix to just that value on a draft PR, while skipping pg-tle-test (and any later heavy job following the same needs:[changes]/if: docs_only pattern) outright. Non-draft PRs and push events (e.g. post-merge on master) are unaffected. --- .github/workflows/ci.yml | 48 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 44 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1848af8..1170b2b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,6 +34,26 @@ # doc-only pushes, and also derives the shared PostgreSQL-major list those # jobs consume from a single set of constants. `all-checks-passed` is the # single stable required-status-check name. +# +# Draft PRs get a further reduction, independent of `changes`/docs_only, +# aimed at cutting shared-runner load while a PR is still being iterated on +# (this repo's org-wide Actions queue backs up easily): `lint` always runs +# in full; `test`'s matrix drops to just the newest supported PostgreSQL +# major (see its own comment) instead of running full or being skipped +# outright, since it's cheap per-leg and a draft author still wants signal +# on every push; every other heavy job (`pg-tle-test`, and `pg-upgrade-test` +# etc. from later phases) is skipped entirely via an added +# `&& github.event.pull_request.draft != true` on its existing `if:`. None +# of this applies to a `push` event (e.g. the post-merge run on master) or +# a non-draft PR, both of which always run the full suite exactly as +# before. `github.event.pull_request.draft` reflects the PR's CURRENT +# draft status at the time each event fires, so once a PR is marked +# ready-for-review, its next actual trigger (a `synchronize` push - this +# workflow's `pull_request:` has no `types:` override, so it only runs on +# the GitHub default of opened/synchronize/reopened, NOT the +# `ready_for_review` action by itself) correctly sees draft=false and runs +# the full suite; the reduced draft-time result on prior commits is not +# retroactively re-run. # =========================================================================== name: CI on: @@ -52,13 +72,18 @@ jobs: # major list the test job consumes: every job that cares which majors are # supported reads the SAME list, so they can't silently drift onto # different sets, and adding a new major is a one-line change here - # instead of an edit in several jobs. + # instead of an edit in several jobs. `newest_pg` is the same NEWEST + # constant emitted again as a bare scalar (not wrapped in the JSON-array + # `supported_pg`), consumed only by the `test` job's draft-PR matrix + # reduction (see the top-of-file comment and that job's own comment) - so + # NEWEST still only needs to change in one place. changes: name: ๐Ÿ” Detect docs-only changes & derive PG matrix runs-on: ubuntu-latest outputs: docs_only: ${{ steps.diff.outputs.docs_only }} supported_pg: ${{ steps.pg.outputs.supported_pg }} + newest_pg: ${{ steps.pg.outputs.newest_pg }} steps: - name: Check out the repo uses: actions/checkout@v4 @@ -150,6 +175,12 @@ jobs: echo "supported_pg=$(json $supported)" >> "$GITHUB_OUTPUT" + # Also emitted as a bare scalar (not a JSON array) so the `test` + # job's draft-PR matrix reduction (see its own comment) can build a + # single-element list from it via fromJSON(format(...)) without a + # second hardcoded "18" anywhere in this file. + echo "newest_pg=$NEWEST" >> "$GITHUB_OUTPUT" + lint: name: ๐Ÿงน SQL lint runs-on: ubuntu-latest @@ -185,8 +216,14 @@ jobs: if: needs.changes.outputs.docs_only != 'true' strategy: matrix: - # From the single source in the changes job. - pg: ${{ fromJSON(needs.changes.outputs.supported_pg) }} + # From the single source in the changes job. On a draft PR, reduced + # to just the newest supported major (never skipped outright, unlike + # the other heavy jobs below - this is the one signal a draft author + # still wants on every push): `github.event.pull_request.draft` is + # null/falsy for a push event (e.g. the post-merge run on master), so + # this expression falls through to the full list there with no extra + # guard needed. + pg: ${{ github.event.pull_request.draft && fromJSON(format('[{0}]', needs.changes.outputs.newest_pg)) || fromJSON(needs.changes.outputs.supported_pg) }} name: ๐Ÿ˜ PostgreSQL ${{ matrix.pg }} runs-on: ubuntu-latest container: pgxn/pgxn-tools @@ -204,7 +241,10 @@ jobs: pg-tle-test: needs: [changes] - if: needs.changes.outputs.docs_only != 'true' + # Skipped outright (not just matrix-reduced like `test` above) on a + # draft PR: this is a heavy job, and a draft author doesn't need the + # pg_tle deployment path re-proven on every push while still iterating. + if: needs.changes.outputs.docs_only != 'true' && github.event.pull_request.draft != true strategy: matrix: # Intersection of count_nulls' own supported range (10-18, see the From b11df4d6679b5ce68b0d328be0c904037303cc6c Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Sat, 1 Aug 2026 16:11:44 -0500 Subject: [PATCH 3/6] Phase 4: real pg_upgrade support via a reduced bin/test_existing Adds the pg-upgrade-test CI job: install 0.9.6 on an old PostgreSQL major, plant + prove a dependency guard, binary pg_upgrade to a newer major, ALTER EXTENSION UPDATE the migrated objects, then run the suite against the real upgraded database in existing mode. bin/test_existing is much smaller than the equivalent script would have been pre-test/install: only prepare-old and run-suite are genuinely external-to-pg_regress concerns (a real pg_upgrade binary run isn't something pg_regress can invoke itself), plus a small `update` subcommand for the post-upgrade ALTER EXTENSION UPDATE step. There's no update-scenario subcommand at all - that entire scenario is just `make test-update` now (test/install/load.sql's own 'update' mode, added in phase 3), since an in-place update has no external step to drive. run_suite() gates on plain `make test`, not the old belt-and-suspenders `make test && make verify-results` - pgxntool 2.3.0 (this repo's phase 0) already made `make test` itself exit non-zero on regression failures. Not yet crossed with TEST_SCHEMA - that's the next phase, once both this job and extension-update-test can cross it together. Verified locally against PG17 (prepare-old -> update -> run-suite, without a real pg_upgrade - this container's clusters are persistent shared infra, so the actual binary pg_upgrade leg is left for CI's ephemeral containers, same reasoning as the pg-tle-test work). Co-Authored-By: Claude Sonnet 5 --- .github/workflows/ci.yml | 108 ++++++++++++- bin/test_existing | 210 +++++++++++++++++++++++++ bin/test_existing.sql/assert_guard.sql | 35 +++++ bin/test_existing.sql/drop_guard.sql | 11 ++ bin/test_existing.sql/plant_guard.sql | 30 ++++ 5 files changed, 393 insertions(+), 1 deletion(-) create mode 100755 bin/test_existing create mode 100644 bin/test_existing.sql/assert_guard.sql create mode 100644 bin/test_existing.sql/drop_guard.sql create mode 100644 bin/test_existing.sql/plant_guard.sql diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1170b2b..94733f0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,6 +16,16 @@ # own job would only duplicate this job's own # per-PG-version container/checkout setup for # no added confidence. +# pg-upgrade-test -- BINARY pg_upgrade: install 0.9.6 on an OLD +# PostgreSQL major, binary-upgrade the cluster +# to a NEWER major, then update the extension +# to current - proves objects created on an +# old server still work when read on a new +# one. A smaller old_pg/new_pg matrix (not the +# full PG matrix - by far the most expensive +# job here, installing two full PostgreSQL +# majors and running the real pg_upgrade +# binary per leg). # pg-tle-test -- pg_tle DEPLOYMENT: fresh install registered # through AWS pg_tle's database-backed catalog # instead of a filesystem .control file. @@ -239,6 +249,102 @@ jobs: - name: Update 0.9.6 -> current and run the suite run: make verify-results TEST_LOAD_SOURCE=update + # Proves count_nulls survives a BINARY pg_upgrade (in-place catalog + # migration to a newer PostgreSQL major), not just an in-place extension + # update. Installs 0.9.6 on an old cluster, plants a dependency guard, + # binary-pg_upgrades to a newer cluster, updates the extension to current, + # then runs the suite against the REAL migrated objects in existing mode. + # No bridge-update step first: count_nulls has always been pure SQL + # functions with no SELECT-*-over-catalog views, so it has no known + # pg_upgrade-unsafe old version to bridge past. + # + # Deliberately not doing a stepwise every-major-in-sequence climb (one + # cluster walking 10->11->12->...->newest, vs. the single big jumps here): + # that would catch a regression specific to one particular major-to-major + # boundary, which would matter if count_nulls had views/functions touching + # catalog internals, but it doesn't - pure SQL functions over anyarray/ + # json/jsonb, nothing version-sensitive to break at a specific boundary. + # Revisit if count_nulls ever grows something catalog-touching. + # + # Not yet crossed with TEST_SCHEMA (a later phase adds that, once it can + # do so for both this job and the test job's update leg together). + pg-upgrade-test: + needs: [changes] + if: needs.changes.outputs.docs_only != 'true' + strategy: + matrix: + old_pg: ["10", "12"] + new_pg: ["18"] + name: ๐Ÿ”„ Binary pg_upgrade ${{ matrix.old_pg }} โ†’ ${{ matrix.new_pg }} + runs-on: ubuntu-latest + container: pgxn/pgxn-tools + env: + # Both clusters must use the same initdb options or pg_upgrade + # refuses to run. + INITDB_OPTS: --data-checksums --auth trust + steps: + - name: Start PostgreSQL ${{ matrix.old_pg }} + run: pg-start ${{ matrix.old_pg }} + - name: Recreate old cluster with data checksums enabled + run: | + pg_ctlcluster ${{ matrix.old_pg }} test stop + pg_dropcluster ${{ matrix.old_pg }} test + # -p 5432: pg_createcluster assigns the next available port, which + # may not be 5432 after pg-start has claimed and released it. + # Force 5432 so subsequent psql/createdb calls connect without -p. + pg_createcluster -p 5432 ${{ matrix.old_pg }} test -- $INITDB_OPTS + pg_ctlcluster ${{ matrix.old_pg }} test start + pg_isready -t 30 + - name: Check out the repo + uses: actions/checkout@v4 + - name: Install count_nulls into old cluster + run: make install + - name: Prepare the old cluster (install + dependency guard) + # prepare-old installs count_nulls at 0.9.6, then plants + proves + # the dependency guard, so a later accidental CASCADE drop anywhere + # in this job cannot silently make the eventual existing-mode run + # test a fresh install instead. + run: bin/test_existing prepare-old count_nulls_upgrade "" 0.9.6 + - name: Install PostgreSQL ${{ matrix.new_pg }} + run: apt-get install -y postgresql-${{ matrix.new_pg }} postgresql-server-dev-${{ matrix.new_pg }} + - name: Install count_nulls into new cluster + # PG_CONFIG must be specified explicitly: at this point both old + # and new PostgreSQL are installed, and the default pg_config on + # PATH may not be the new version's. + run: make install PG_CONFIG=/usr/lib/postgresql/${{ matrix.new_pg }}/bin/pg_config + - name: Stop old cluster, binary pg_upgrade to PostgreSQL ${{ matrix.new_pg }}, start new cluster + run: | + pg_ctlcluster ${{ matrix.old_pg }} test stop + pg_createcluster -p 5432 ${{ matrix.new_pg }} test -- $INITDB_OPTS + # PG17+ writes logs to $new_datadir/pg_upgrade_output.d/; older + # versions write to CWD. Search both on failure. + mkdir -p /tmp/pg_upgrade_logs + chown postgres:postgres /tmp/pg_upgrade_logs + su -c "cd /tmp/pg_upgrade_logs && /usr/lib/postgresql/${{ matrix.new_pg }}/bin/pg_upgrade \ + -b /usr/lib/postgresql/${{ matrix.old_pg }}/bin \ + -B /usr/lib/postgresql/${{ matrix.new_pg }}/bin \ + -d /var/lib/postgresql/${{ matrix.old_pg }}/test \ + -D /var/lib/postgresql/${{ matrix.new_pg }}/test \ + -o '-c config_file=/etc/postgresql/${{ matrix.old_pg }}/test/postgresql.conf' \ + -O '-c config_file=/etc/postgresql/${{ matrix.new_pg }}/test/postgresql.conf'" postgres \ + || { find /tmp/pg_upgrade_logs \ + /var/lib/postgresql/${{ matrix.new_pg }}/test/pg_upgrade_output.d \ + -name '*.log' 2>/dev/null | sort | xargs -r tail -n +1; exit 1; } + pg_ctlcluster ${{ matrix.new_pg }} test start + - name: Update the pg_upgraded extension to the current version + # Exercises ALTER EXTENSION UPDATE on genuinely pg_upgraded objects + # (the extension binary pg_upgrade just migrated), running the + # 0.9.6->stable update script. + run: bin/test_existing update count_nulls_upgrade + - name: Run the suite against the pg_upgraded database (existing mode) + # run-suite asserts the version, re-proves the dependency guard + # still blocks a non-CASCADE drop (i.e. it survived pg_upgrade), + # drops the guard, then runs the suite against the REAL pg_upgraded + # + updated database via --use-existing (so pg_regress does not + # drop/recreate it) - a plain fresh `make test` would silently test + # a fresh install instead of the migrated objects. + run: bin/test_existing run-suite count_nulls_upgrade "" + pg-tle-test: needs: [changes] # Skipped outright (not just matrix-reduced like `test` above) on a @@ -372,7 +478,7 @@ jobs: # `changes` job on a docs-only push), and fails if any failed or were # cancelled. all-checks-passed: - needs: [changes, lint, test, pg-tle-test] + needs: [changes, lint, test, pg-upgrade-test, pg-tle-test] if: always() runs-on: ubuntu-latest steps: diff --git a/bin/test_existing b/bin/test_existing new file mode 100755 index 0000000..d0c2cb6 --- /dev/null +++ b/bin/test_existing @@ -0,0 +1,210 @@ +#!/usr/bin/env bash +# +# Exercise the count_nulls test suite against a REAL database whose extension +# was installed/upgraded OUTSIDE this pg_regress invocation ("existing" mode) +# - specifically, a real binary pg_upgrade run. Unlike an in-place ALTER +# EXTENSION UPDATE (which test/install/load.sql's own 'update' mode handles +# entirely by itself - see `make test-update`), a real pg_upgrade is an +# external binary process pg_regress can't invoke itself, so THAT leg needs +# an external script to drive it. This is a smaller script than it might look +# like it needs to be: only the pieces that genuinely can't live inside a +# single pg_regress invocation are here. +# +# The pg-upgrade-test CI job repeats the same sequence: +# +# prepare-old (install + plant guard) -> [real pg_upgrade binary, in CI] -> +# update (ALTER EXTENSION UPDATE) -> run-suite (assert + run existing-mode) +# +# so it lives here once instead of being duplicated as inline YAML. Not +# CI-only: a developer can run any subcommand locally against a scratch +# database. Modeled on Postgres-Extensions/cat_tools's bin/test_existing. +# Two differences from that script: count_nulls ships no +# SELECT-*-over-catalog views, so it has no known pg_upgrade-unsafe old +# version to bridge past before running pg_upgrade; and its own suite has a +# legitimate (though harmless - always rolled back) DROP EXTENSION test, so +# here the guard is dropped before run-suite instead of surviving through it. +# +# USAGE: bin/test_existing [args] +# +# prepare-old DB SCHEMA INSTALL_VERSION +# Old-cluster prep for pg-upgrade-test: create DB + extension at +# INSTALL_VERSION in SCHEMA, then plant + prove the dependency guard. +# +# update DB [TO_VERSION] +# ALTER EXTENSION count_nulls UPDATE [TO 'TO_VERSION'] (empty => current). +# +# run-suite DB SCHEMA +# Assert the current version, re-prove the guard, drop it, then run the +# suite in existing mode (extension must be at the current version). +# +# Run `bin/test_existing` with no subcommand to print usage. +# +# Why the dependency guard: "existing" mode must run the suite against the +# ACTUAL upgraded objects. If anything silently dropped + reinstalled the +# extension (a stray CASCADE, a logic bug, a bad CI step), the suite would +# test a FRESH install and hide a regression. We plant an object that HARD- +# references a count_nulls member so a non-CASCADE DROP EXTENSION fails, and +# actively PROVE that (see bin/test_existing.sql/assert_guard.sql): if the +# drop unexpectedly succeeds, this script fails CI rather than silently +# passing. +set -euo pipefail + +# Run from the repository root (where `make` works and test paths resolve), +# regardless of the caller's cwd. bin/ sits directly under the repo root, so +# its parent is the root. readlink -f resolves any path the script was +# invoked through. +cd "$(dirname "$(readlink -f "$0")")/.." + +# --------------------------------------------------------------------------- +# psql helpers +# --------------------------------------------------------------------------- + +psql_value() { + local db=$1 sql=$2 + psql -d "$db" -tAc "$sql" +} + +psql_do() { + local db=$1 + shift + psql -d "$db" -v ON_ERROR_STOP=1 "$@" +} + +# --------------------------------------------------------------------------- +# Version / guard helpers +# --------------------------------------------------------------------------- + +current_version() { + # EXTENSION_count_nulls_VERSION (the .control file's default_version), NOT + # PGXNVERSION (the PGXN distribution version, from META.in.json) - a + # version-less CREATE EXTENSION/ALTER EXTENSION UPDATE installs whatever + # the control file's default_version says, and count_nulls' is currently + # the 'stable' pseudo-version, not the last real release. Using PGXNVERSION + # here would compare an installed 'stable' against an expected real version + # number and always report a mismatch. See RELEASE.md's note on + # distribution vs. extension versions; the pg-tle-test CI job makes the + # same distinction for the same reason. + make -s print-EXTENSION_count_nulls_VERSION 2>/dev/null | sed -n 's/.*set to "\(.*\)"$/\1/p' +} + +installed_version() { + psql_value "$1" \ + "SELECT extversion FROM pg_extension WHERE extname = 'count_nulls'" +} + +# Plant the guard and PROVE it blocks a non-CASCADE drop. Call right after +# CREATE EXTENSION (and before any update/upgrade) so it persists through them. +plant_guard() { + local db=$1 schema=$2 + psql -d "$db" -v ON_ERROR_STOP=1 -v schema="$schema" -f bin/test_existing.sql/plant_guard.sql + assert_drop_blocked "$db" +} + +# The core safeguard self-check: a non-CASCADE DROP EXTENSION MUST fail while +# the guard exists. assert_guard.sql fails loudly (nonzero exit) if the drop +# unexpectedly succeeds, or if the extension/guard are missing afterward. +assert_drop_blocked() { + local db=$1 + psql -d "$db" -v ON_ERROR_STOP=1 -f bin/test_existing.sql/assert_guard.sql + echo "OK: non-CASCADE DROP EXTENSION is blocked in '$db' (dependency guard effective)" +} + +drop_guard() { + local db=$1 + psql -d "$db" -v ON_ERROR_STOP=1 -f bin/test_existing.sql/drop_guard.sql +} + +assert_version() { + local db=$1 expected=$2 installed + [ "$expected" = current ] && expected=$(current_version) + installed=$(installed_version "$db") + echo "version check '$db': installed='$installed' expected='$expected'" + if [ -z "$installed" ] || [ -z "$expected" ] || [ "$installed" != "$expected" ]; then + echo "FAIL: count_nulls in '$db' is '$installed', expected '$expected'" >&2 + exit 1 + fi +} + +update_ext() { + local db=$1 to=${2:-} + # Prepend "TO " only when a target version is given, so a single statement + # covers both cases (empty $to => bare "ALTER EXTENSION ... UPDATE" to + # current). Use `if`, not `&&`: a false test under `set -e` would abort. + if [ -n "$to" ]; then to="TO '$to'"; fi + psql_do "$db" -c "ALTER EXTENSION count_nulls UPDATE $to" +} + +# CREATE EXTENSION count_nulls at VERSION, targeting SCHEMA - unless SCHEMA +# is empty, in which case it's created untouched, wherever the session's +# own default search_path resolves (ordinarily 'public'). A quoted empty +# identifier ("") is a real Postgres syntax error, so this can't just always +# emit `CREATE SCHEMA IF NOT EXISTS "$schema"` - the empty case has to skip +# that entirely, mirroring test/install/load.sql's own :count_nulls_has_schema +# branch. +create_extension_in_schema() { + local db=$1 schema=$2 version=$3 sql="" + if [ -n "$schema" ]; then + sql="CREATE SCHEMA IF NOT EXISTS \"$schema\"; SET search_path = \"$schema\"; " + fi + psql_do "$db" -c "${sql}CREATE EXTENSION count_nulls VERSION '$version'" +} + +# --------------------------------------------------------------------------- +# Subcommand implementations +# --------------------------------------------------------------------------- + +# prepare-old DB SCHEMA INSTALL_VERSION +# Old-cluster preparation for pg-upgrade-test: create the database and the +# extension at INSTALL_VERSION in SCHEMA, then plant + prove the guard. No +# bridge-update step first: count_nulls ships no SELECT-*-over-catalog +# views, so it has no known pg_upgrade-unsafe old version to bridge past. +prepare_old() { + local db=$1 schema=$2 install=$3 + createdb "$db" + create_extension_in_schema "$db" "$schema" "$install" + plant_guard "$db" "$schema" +} + +# Run the pgTAP suite against an already-populated database in existing mode. +# Verifies count_nulls is at the current version, re-proves the guard still +# blocks a drop (i.e. it survived the update/upgrade), drops the guard (see +# the file header for why - count_nulls's own suite legitimately drops the +# extension, harmlessly, inside a transaction that's always rolled back), +# then runs the suite via --use-existing so pg_regress does NOT drop/recreate +# the database. +run_suite() { + local db=$1 schema=$2 + assert_version "$db" current + assert_drop_blocked "$db" + drop_guard "$db" + # In existing mode pg_regress runs against $db via --use-existing and must + # NOT create/drop its own database. `make test` (not just `make + # verify-results`) is a real gate as of pgxntool 2.3.0 - it now exits + # non-zero on regression failures instead of always exiting 0 regardless + # of pg_regress's result (see this repo's pgxntool 2.3.0 bump). + make test TEST_LOAD_SOURCE=existing TEST_SCHEMA="$schema" CONTRIB_TESTDB="$db" EXTRA_REGRESS_OPTS=--use-existing +} + +usage() { + echo "usage: bin/test_existing [args]" >&2 + echo " prepare-old DB SCHEMA INSTALL_VERSION" >&2 + echo " update DB [TO_VERSION]" >&2 + echo " run-suite DB SCHEMA" >&2 + exit 2 +} + +# Explicit subcommand dispatch on $1. Defined first for readability; INVOKED +# at the very bottom, after every helper it calls is defined (bash resolves +# calls at runtime, so main() appearing first is fine). +main() { + local cmd=${1:-} + shift || true + case "$cmd" in + prepare-old) prepare_old "$@" ;; + update) update_ext "$@" ;; + run-suite) run_suite "$@" ;; + *) usage ;; + esac +} + +main "$@" diff --git a/bin/test_existing.sql/assert_guard.sql b/bin/test_existing.sql/assert_guard.sql new file mode 100644 index 0000000..33946cf --- /dev/null +++ b/bin/test_existing.sql/assert_guard.sql @@ -0,0 +1,35 @@ +/* + * Proves the guard planted by plant_guard.sql actually blocks a + * non-CASCADE DROP EXTENSION - prove it, don't assume it. Re-run after + * every step (install, pg_upgrade, post-upgrade ALTER EXTENSION UPDATE): + * the guard disappearing at any point means a CASCADE drop happened + * somewhere upstream, i.e. the "existing" run downstream would actually be + * a silent fresh install. + * + * Usage: psql -v ON_ERROR_STOP=1 -f assert_guard.sql + */ +\set ON_ERROR_STOP on + +DO $$ +BEGIN + DROP EXTENSION count_nulls; + -- Only reached if the drop above unexpectedly succeeded. + RAISE EXCEPTION 'GUARD FAILURE: non-CASCADE DROP EXTENSION count_nulls unexpectedly succeeded'; +EXCEPTION WHEN dependent_objects_still_exist THEN + RAISE NOTICE 'guard held: DROP EXTENSION count_nulls correctly blocked'; +END +$$; + +DO $$ +BEGIN + IF NOT EXISTS (SELECT 1 FROM pg_extension WHERE extname = 'count_nulls') THEN + RAISE EXCEPTION 'GUARD FAILURE: count_nulls extension missing after guard check'; + END IF; + IF NOT EXISTS ( + SELECT 1 FROM pg_class c JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE c.relname = 'guard' AND n.nspname = 'count_nulls_drop_guard' + ) THEN + RAISE EXCEPTION 'GUARD FAILURE: count_nulls_drop_guard.guard view missing'; + END IF; +END +$$; diff --git a/bin/test_existing.sql/drop_guard.sql b/bin/test_existing.sql/drop_guard.sql new file mode 100644 index 0000000..744c3b5 --- /dev/null +++ b/bin/test_existing.sql/drop_guard.sql @@ -0,0 +1,11 @@ +/* + * Removes the guard (plant_guard.sql) once its job is done - proving the + * install/pg_upgrade/update steps didn't corrupt the real extension - so it + * doesn't then block the pgTap suite's own DROP EXTENSION test + * (test__shutdown__drop_all, run in a transaction that's rolled back + * regardless, so re-dropping the real extension there is harmless). + * + * Usage: psql -v ON_ERROR_STOP=1 -f drop_guard.sql + */ +\set ON_ERROR_STOP on +DROP SCHEMA count_nulls_drop_guard CASCADE; diff --git a/bin/test_existing.sql/plant_guard.sql b/bin/test_existing.sql/plant_guard.sql new file mode 100644 index 0000000..1c3e84d --- /dev/null +++ b/bin/test_existing.sql/plant_guard.sql @@ -0,0 +1,30 @@ +/* + * Dependency guard: plants an object with a hard pg_depend dependency on a + * stable, never-dropped/redefined extension member (null_count(anyarray), + * unchanged since 0.9.0), so that a non-CASCADE DROP EXTENSION count_nulls + * is blocked. Used by the pg_upgrade CI job to prove a real pg_upgrade/ + * update run didn't silently destroy the extension it's meant to be + * testing (a stray CASCADE drop, a logic bug, a bad CI step would + * otherwise fall through to a silent fresh reinstall and the job would + * still report green). + * + * Usage: psql -v ON_ERROR_STOP=1 -v schema= -f plant_guard.sql + * (empty schema means "wherever null_count already resolves unqualified" - + * i.e. count_nulls was installed without targeting a schema). + */ +\set ON_ERROR_STOP on + +/* + * schema_prefix: either empty, or the quoted schema name followed by a + * literal '.' - so the view definition below is a single statement with a + * plain (unquoted) substitution, rather than branching the whole CREATE + * VIEW on whether a schema was given. quote_ident(), not :"schema" - + * :schema_prefix is pasted as-is (unquoted substitution), so it must + * already be valid, properly-quoted SQL text by the time it lands there. + */ +SELECT CASE WHEN :'schema' <> '' THEN quote_ident(:'schema') || '.' ELSE '' END AS schema_prefix +\gset + +CREATE SCHEMA IF NOT EXISTS count_nulls_drop_guard; +CREATE OR REPLACE VIEW count_nulls_drop_guard.guard AS + SELECT :schema_prefix null_count(NULL::int, NULL::int) AS guarded_member; From a3b7c810e15e9eb31858d1569f289815dad021f3 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Wed, 5 Aug 2026 16:41:23 -0500 Subject: [PATCH 4/6] 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. --- .github/workflows/ci.yml | 65 ++++++++++++++++++++++++---------------- bin/test_existing | 14 ++++++--- 2 files changed, 50 insertions(+), 29 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 94733f0..189119c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,15 +17,18 @@ # per-PG-version container/checkout setup for # no added confidence. # pg-upgrade-test -- BINARY pg_upgrade: install 0.9.6 on an OLD -# PostgreSQL major, binary-upgrade the cluster -# to a NEWER major, then update the extension -# to current - proves objects created on an -# old server still work when read on a new -# one. A smaller old_pg/new_pg matrix (not the -# full PG matrix - by far the most expensive -# job here, installing two full PostgreSQL -# majors and running the real pg_upgrade -# binary per leg). +# PostgreSQL major, update the extension to +# current (still on the old major), THEN +# binary-upgrade the cluster to a NEWER major - +# proves pg_upgrade correctly migrates the +# objects the extension actually creates +# TODAY, not objects frozen at some past +# version (which would be untestable anyway - +# that old version already shipped). A smaller +# old_pg/new_pg matrix (not the full PG matrix +# - by far the most expensive job here, +# installing two full PostgreSQL majors and +# running the real pg_upgrade binary per leg). # pg-tle-test -- pg_tle DEPLOYMENT: fresh install registered # through AWS pg_tle's database-backed catalog # instead of a filesystem .control file. @@ -250,13 +253,19 @@ jobs: run: make verify-results TEST_LOAD_SOURCE=update # Proves count_nulls survives a BINARY pg_upgrade (in-place catalog - # migration to a newer PostgreSQL major), not just an in-place extension - # update. Installs 0.9.6 on an old cluster, plants a dependency guard, - # binary-pg_upgrades to a newer cluster, updates the extension to current, + # migration to a newer PostgreSQL major). Installs 0.9.6 on an old + # cluster, plants a dependency guard, updates the extension to CURRENT + # (still on the old major), THEN binary-pg_upgrades to a newer cluster, # then runs the suite against the REAL migrated objects in existing mode. - # No bridge-update step first: count_nulls has always been pure SQL - # functions with no SELECT-*-over-catalog views, so it has no known - # pg_upgrade-unsafe old version to bridge past. + # Updating before the binary upgrade (not after) is deliberate: the whole + # point of this job is proving pg_upgrade correctly migrates the objects + # count_nulls' CURRENT code actually creates - migrating 0.9.6's objects + # and updating afterward would instead test whether pg_upgrade can + # migrate a legacy structure frozen in the past, which isn't actionable + # (that version already shipped; nothing to fix if it turned out + # fragile). No bridge-update step first: count_nulls has always been + # pure SQL functions with no SELECT-*-over-catalog views, so it has no + # known pg_upgrade-unsafe old version to bridge past. # # Deliberately not doing a stepwise every-major-in-sequence climb (one # cluster walking 10->11->12->...->newest, vs. the single big jumps here): @@ -305,6 +314,17 @@ jobs: # in this job cannot silently make the eventual existing-mode run # test a fresh install instead. run: bin/test_existing prepare-old count_nulls_upgrade "" 0.9.6 + - name: Update the extension to the current version (still on the old cluster) + # Exercises ALTER EXTENSION UPDATE on the OLD cluster, BEFORE the + # binary pg_upgrade below, running the 0.9.6->stable update script - + # deliberately in this order (not update-after-upgrade): this job + # exists to prove pg_upgrade correctly migrates the objects + # count_nulls' CURRENT code creates, so pg_upgrade must run against + # already-current objects, not 0.9.6 ones. `make install` above + # already installed the current version's update scripts/control + # file into this (old) cluster's sharedir, so they're in place for + # this ALTER EXTENSION UPDATE to use. + run: bin/test_existing update count_nulls_upgrade - name: Install PostgreSQL ${{ matrix.new_pg }} run: apt-get install -y postgresql-${{ matrix.new_pg }} postgresql-server-dev-${{ matrix.new_pg }} - name: Install count_nulls into new cluster @@ -331,18 +351,13 @@ jobs: /var/lib/postgresql/${{ matrix.new_pg }}/test/pg_upgrade_output.d \ -name '*.log' 2>/dev/null | sort | xargs -r tail -n +1; exit 1; } pg_ctlcluster ${{ matrix.new_pg }} test start - - name: Update the pg_upgraded extension to the current version - # Exercises ALTER EXTENSION UPDATE on genuinely pg_upgraded objects - # (the extension binary pg_upgrade just migrated), running the - # 0.9.6->stable update script. - run: bin/test_existing update count_nulls_upgrade - name: Run the suite against the pg_upgraded database (existing mode) # run-suite asserts the version, re-proves the dependency guard - # still blocks a non-CASCADE drop (i.e. it survived pg_upgrade), - # drops the guard, then runs the suite against the REAL pg_upgraded - # + updated database via --use-existing (so pg_regress does not - # drop/recreate it) - a plain fresh `make test` would silently test - # a fresh install instead of the migrated objects. + # still blocks a non-CASCADE drop (i.e. it survived both the update + # and pg_upgrade), drops the guard, then runs the suite against the + # REAL pg_upgraded database via --use-existing (so pg_regress does + # not drop/recreate it) - a plain fresh `make test` would silently + # test a fresh install instead of the migrated objects. run: bin/test_existing run-suite count_nulls_upgrade "" pg-tle-test: diff --git a/bin/test_existing b/bin/test_existing index d0c2cb6..f0b12f5 100755 --- a/bin/test_existing +++ b/bin/test_existing @@ -12,11 +12,17 @@ # # The pg-upgrade-test CI job repeats the same sequence: # -# prepare-old (install + plant guard) -> [real pg_upgrade binary, in CI] -> -# update (ALTER EXTENSION UPDATE) -> run-suite (assert + run existing-mode) +# prepare-old (install + plant guard) -> update (ALTER EXTENSION UPDATE, +# on the OLD cluster, before pg_upgrade) -> [real pg_upgrade binary, in +# CI] -> run-suite (assert + run existing-mode) # -# so it lives here once instead of being duplicated as inline YAML. Not -# CI-only: a developer can run any subcommand locally against a scratch +# so it lives here once instead of being duplicated as inline YAML. update +# runs BEFORE the binary pg_upgrade, not after: the point of this job is +# proving pg_upgrade correctly migrates the objects count_nulls' CURRENT +# code creates, so pg_upgrade needs to run against already-current objects, +# not ones still frozen at the old INSTALL_VERSION. +# +# Not CI-only: a developer can run any subcommand locally against a scratch # database. Modeled on Postgres-Extensions/cat_tools's bin/test_existing. # Two differences from that script: count_nulls ships no # SELECT-*-over-catalog views, so it has no known pg_upgrade-unsafe old From 3c19783cdba6b3f071858b5eea7c1de2da0d895d Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Thu, 6 Aug 2026 17:09:54 -0500 Subject: [PATCH 5/6] CI: skip pg-upgrade-test on draft PRs too Propagates the draft-PR gating from phase3.5-ci-hygiene to the pg-upgrade-test job introduced by this branch: same needs:[changes]/ if: docs_only pattern as pg-tle-test, so it gets the same && github.event.pull_request.draft != true guard. --- .github/workflows/ci.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 189119c..89d4a86 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -279,7 +279,10 @@ jobs: # do so for both this job and the test job's update leg together). pg-upgrade-test: needs: [changes] - if: needs.changes.outputs.docs_only != 'true' + # Skipped outright (not just matrix-reduced like `test`) on a draft PR: + # this is a heavy job, and a draft author doesn't need a real binary + # pg_upgrade re-proven on every push while still iterating. + if: needs.changes.outputs.docs_only != 'true' && github.event.pull_request.draft != true strategy: matrix: old_pg: ["10", "12"] From 508e17afbbf039b73b2428e52dda95d70ddaefde Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 4 Aug 2026 13:47:06 -0500 Subject: [PATCH 6/6] Cross extension-update-test/pg-upgrade-test with TEST_SCHEMA via make/shell loops, not a matrix Redesign of the original approach (which crossed TEST_SCHEMA into both jobs' CI matrices) per the same reasoning as the `test` job's collapse: a schema name is just an input the same assertions run against, not a real environment difference. - extension-update-test: added `make test-update-schema-all` (Makefile), the same TEST_SCHEMA loop as test-schema-all but with TEST_LOAD_SOURCE=update. Job step calls it instead of crossing schema into the matrix. - pg-upgrade-test: no make-level loop is possible here (bin/test_existing's steps are shell, not `make test`), so instead prepares TWO databases - count_nulls_upgrade_none and count_nulls_upgrade_quoted, one per TEST_SCHEMA value - before the SINGLE pg_upgrade call, which migrates the whole cluster (every database in it) in one pass. This is strictly better than a doubled matrix would have been: it also halves the number of actual pg_upgrade binary invocations (the single most expensive operation in this job), not just container/checkout overhead. Verified locally against PG17: prepare-old -> update -> run-suite passes for both databases in the same cluster/session (no real pg_upgrade run, same reasoning as prior phases - this container's clusters are persistent shared infra); make test-update-schema-all passes both TEST_SCHEMA legs. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/ci.yml | 75 ++++++++++++++++++++++++++-------------- Makefile | 10 ++++++ 2 files changed, 60 insertions(+), 25 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 89d4a86..7b92057 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,12 +34,19 @@ # instead of a filesystem .control file. # # Every TEST_SCHEMA value (empty - no schema targeting at all - and -# 'Quoted', a name requiring SQL identifier quoting) is exercised too, via -# `make test-schema-all`'s in-Makefile loop rather than a CI matrix -# dimension - a schema name is just an input the same assertions run -# against, not a real environment difference, so crossing it into the -# matrix would only multiply job count for no added confidence (see the -# Makefile's TEST_SCHEMA_VALUES comment). Every leg passes against the SAME +# 'Quoted', a name requiring SQL identifier quoting) is exercised in every +# job above too, but never as a CI matrix dimension - a schema name is just +# an input the same assertions run against, not a real environment +# difference, so crossing it into the matrix would only multiply job count +# for no added confidence (see the Makefile's TEST_SCHEMA_VALUES comment). +# `test` loops it (both its fresh and update legs) via `make +# test-schema-all` / `make test-update-schema-all`; `pg-upgrade-test` +# (shell, not `make test`, for the parts that matter here) prepares two +# databases - one per schema - +# ahead of a single pg_upgrade call that migrates both at once, which is +# strictly better than a doubled matrix would have been: it also halves the +# number of actual pg_upgrade binary invocations, not just container/ +# checkout overhead. Every leg passes against the SAME # test/expected/extension_tests.out (see test/README.md for how the suite # keeps its output schema-invariant). # @@ -249,8 +256,8 @@ jobs: run: make test-schema-all - name: Install count_nulls run: make install - - name: Update 0.9.6 -> current and run the suite - run: make verify-results TEST_LOAD_SOURCE=update + - name: Update 0.9.6 -> current and run the suite, across every TEST_SCHEMA value + run: make test-update-schema-all # Proves count_nulls survives a BINARY pg_upgrade (in-place catalog # migration to a newer PostgreSQL major). Installs 0.9.6 on an old @@ -275,8 +282,16 @@ jobs: # json/jsonb, nothing version-sensitive to break at a specific boundary. # Revisit if count_nulls ever grows something catalog-touching. # - # Not yet crossed with TEST_SCHEMA (a later phase adds that, once it can - # do so for both this job and the test job's update leg together). + # Every TEST_SCHEMA value is exercised here too, but NOT via a matrix + # dimension (would double this job's already-expensive count) and not + # via a make-level loop either (bin/test_existing's steps below are + # shell, not `make test`) - instead, TWO databases (one per schema) are + # prepared before the SINGLE pg_upgrade call, which migrates the WHOLE + # cluster (every database in it) in one pass. This is strictly better + # than a doubled matrix would have been, not just cheaper: it also + # halves the number of actual pg_upgrade binary invocations (the single + # most expensive operation in this job) instead of just avoiding + # redundant container/checkout overhead. pg-upgrade-test: needs: [changes] # Skipped outright (not just matrix-reduced like `test`) on a draft PR: @@ -311,23 +326,31 @@ jobs: uses: actions/checkout@v4 - name: Install count_nulls into old cluster run: make install - - name: Prepare the old cluster (install + dependency guard) + - name: Prepare the old cluster (install + dependency guard), across every TEST_SCHEMA value # prepare-old installs count_nulls at 0.9.6, then plants + proves # the dependency guard, so a later accidental CASCADE drop anywhere # in this job cannot silently make the eventual existing-mode run - # test a fresh install instead. - run: bin/test_existing prepare-old count_nulls_upgrade "" 0.9.6 - - name: Update the extension to the current version (still on the old cluster) + # test a fresh install instead. Two separate databases (distinct + # names, one per TEST_SCHEMA value) so both exist in the SAME + # cluster ahead of the single pg_upgrade call below - that one + # binary upgrade migrates both at once. + run: | + bin/test_existing prepare-old count_nulls_upgrade_none "" 0.9.6 + bin/test_existing prepare-old count_nulls_upgrade_quoted Quoted 0.9.6 + - name: Update the extension to the current version (still on the old cluster), across every TEST_SCHEMA value # Exercises ALTER EXTENSION UPDATE on the OLD cluster, BEFORE the - # binary pg_upgrade below, running the 0.9.6->stable update script - - # deliberately in this order (not update-after-upgrade): this job - # exists to prove pg_upgrade correctly migrates the objects - # count_nulls' CURRENT code creates, so pg_upgrade must run against - # already-current objects, not 0.9.6 ones. `make install` above - # already installed the current version's update scripts/control - # file into this (old) cluster's sharedir, so they're in place for - # this ALTER EXTENSION UPDATE to use. - run: bin/test_existing update count_nulls_upgrade + # binary pg_upgrade below, running the 0.9.6->stable update script, + # once per database prepared above - deliberately in this order + # (not update-after-upgrade): this job exists to prove pg_upgrade + # correctly migrates the objects count_nulls' CURRENT code creates, + # so pg_upgrade must run against already-current objects, not 0.9.6 + # ones. `make install` above already installed the current + # version's update scripts/control file into this (old) cluster's + # sharedir, so they're in place for this ALTER EXTENSION UPDATE to + # use. + run: | + bin/test_existing update count_nulls_upgrade_none + bin/test_existing update count_nulls_upgrade_quoted - name: Install PostgreSQL ${{ matrix.new_pg }} run: apt-get install -y postgresql-${{ matrix.new_pg }} postgresql-server-dev-${{ matrix.new_pg }} - name: Install count_nulls into new cluster @@ -354,14 +377,16 @@ jobs: /var/lib/postgresql/${{ matrix.new_pg }}/test/pg_upgrade_output.d \ -name '*.log' 2>/dev/null | sort | xargs -r tail -n +1; exit 1; } pg_ctlcluster ${{ matrix.new_pg }} test start - - name: Run the suite against the pg_upgraded database (existing mode) + - name: Run the suite against the pg_upgraded database (existing mode), across every TEST_SCHEMA value # run-suite asserts the version, re-proves the dependency guard # still blocks a non-CASCADE drop (i.e. it survived both the update # and pg_upgrade), drops the guard, then runs the suite against the # REAL pg_upgraded database via --use-existing (so pg_regress does # not drop/recreate it) - a plain fresh `make test` would silently # test a fresh install instead of the migrated objects. - run: bin/test_existing run-suite count_nulls_upgrade "" + run: | + bin/test_existing run-suite count_nulls_upgrade_none "" + bin/test_existing run-suite count_nulls_upgrade_quoted Quoted pg-tle-test: needs: [changes] diff --git a/Makefile b/Makefile index 1199bb6..0c55852 100644 --- a/Makefile +++ b/Makefile @@ -101,3 +101,13 @@ export PGOPTIONS := $(PGOPTIONS) -c count_nulls.test_load_mode=$(TEST_LOAD_SOURC .PHONY: test-update test-update: $(MAKE) test TEST_LOAD_SOURCE=update + +# Same TEST_SCHEMA loop as test-schema-all, but in update mode - used by the +# test CI job's update leg instead of crossing TEST_SCHEMA into ITS matrix +# too, same reasoning as test-schema-all above. +.PHONY: test-update-schema-all +test-update-schema-all: + @for schema in $(TEST_SCHEMA_VALUES); do \ + echo "=== TEST_SCHEMA=$$schema (update) ==="; \ + $(MAKE) test TEST_LOAD_SOURCE=update TEST_SCHEMA="$$schema" || exit 1; \ + done