feat: test pg_upgrade compatibility with older extension versions

[jfroche]

Add test to verify that all extension versions from PostgreSQL 15 can successfully upgrade to PostgreSQL 17 using pg_upgrade.

The test now validates:

• Each PG 15 extension version can be upgraded to PG 17
• Extension update scripts are properly generated during upgrade
• Version handling works correctly with and without update scripts
• Final extension versions match expected values after upgrade

Summary by CodeRabbit

• New Features

  • pgVersion-driven version handling and automatic generation of SQL-only packages/assets plus migration copies for older PostgreSQL versions to support pg_upgrade workflows.

• Tests

  • Expanded automated upgrade tests covering pg_upgrade 15→17 across many extensions; added a dedicated pg_stat_monitor test and standardized per-version test configurations and workflows.

• Bug Fixes

  • Extension removal now cascades to drop dependent objects during teardown.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[samrose]

Looks great! Will just need to test before we merge it

[coderabbitai]

Walkthrough

Adds pgVersion-driven build logic and SQL-only artifacts for unsupported Postgres versions; applies pgUpgradeCompatible filtering; and expands/refactors tests to exercise pg_upgrade (Postgres 15→17) across multiple extensions, including a new pg_stat_monitor NixOS test.

Changes

Cohort / File(s)	Summary
PG version variable & unsupported artifacts nix/ext/pg_graphql/default.nix, nix/ext/pg_jsonschema/default.nix	Introduces pgVersion and replaces inline major-version usage; adds unsupportedVersions/unsupportedPackages and buildUnsupported to emit SQL-only assets for older Postgres versions; expands build inputs and adds postBuild/postInstall logic to assemble migration SQL and handle mergeless upgrades.
Upgrade-compatibility filtering & metadata nix/ext/pg_stat_monitor.nix, nix/ext/versions.json	Adds pgUpgradeCompatible-based filtering to version lists; introduces pgUpgradeCompatibleVersions usage for package construction; marks pg_stat_monitor v1.0.1 with pgUpgradeCompatible = false in versions.json.
Test harness core & utilities nix/ext/tests/lib.py, nix/ext/tests/default.nix	Changes DROP EXTENSION to CASCADE; centralizes system/pg15_configuration/pg17_configuration bindings; updates exported extension list (removes pg_stat_monitor, adds pg_partman, postgis); adds pg_upgrade test scaffolding.
New and updated per-extension tests with pg_upgrade flow nix/ext/tests/pg_stat_monitor.nix, nix/ext/tests/pgmq.nix, nix/ext/tests/pgroonga.nix, nix/ext/tests/pgsodium.nix, nix/ext/tests/vault.nix	Adds a new pg_stat_monitor runTest and refactors other extension tests to use system-derived pg15_configuration/pg17_configuration (or psql_15/psql_17); adds subtests that iterate 15-era extension versions to exercise pg_upgrade, detect/execute update scripts, run pg_regress, and validate resulting extension versions.

Sequence Diagram(s)

sequenceDiagram
    participant Tester
    participant PG15 as "PostgreSQL 15"
    participant FS as "File System"
    participant Ext as "Extension SQL"
    participant PG17 as "PostgreSQL 17"

    Tester->>PG15: Switch to PG15 config / stop PG17
    Tester->>FS: Reset data dir / init DB
    Tester->>Ext: Install older extension SQL (15-era)
    Tester->>PG17: Switch to PG17 config (trigger pg_upgrade)
    PG15->>PG17: Run pg_upgrade (data migration)
    Tester->>FS: Check for update_extensions.sql
    alt update_extensions.sql exists
        Tester->>Ext: Execute update_extensions.sql
        Ext->>PG17: Apply migration SQL -> update schema/version
        Tester->>Tester: Assert extension at latest v17
    else no update script
        Tester->>Tester: Assert extension remains at 15-era version
    end
    Tester->>PG17: Run pg_regress validation

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested reviewers

• samrose
• hunleyd

Poem

🐰 I hopped through nix and version rows,

Fifteen to seventeen, the upgrade goes,
SQL patches stitched each migration seam,
Tests now guard the upgrade dream,
Hooray — extensions hop in peace! 🥕

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	The PR description is incomplete; it lacks the required template structure (Default or Extension Upgrade) specified in the description_template, making validation against required sections impossible.	Use the appropriate sub-template from the repository (Default or Extension Upgrade) and structure the description accordingly to ensure all required sections are properly filled.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main objective of the PR - adding tests for pg_upgrade compatibility with older extension versions across PostgreSQL 15 to 17.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@nix/ext/pg_stat_monitor.nix`:
- Around line 20-29: latestVersion is computed from allVersionsList but packages
are built from pgUpgradeCompatibleVersions, causing a mismatch if the newest
supported version is not pgUpgradeCompatible; change the latestVersion
definition to derive from the pgUpgradeCompatible set (e.g. use lib.last on the
attrNames of pgUpgradeCompatibleVersions or on a sorted list of
pgUpgradeCompatible version keys) so latestVersion and packages come from the
same pg_upgrade-compatible collection (update the latestVersion variable to
reference pgUpgradeCompatibleVersions instead of allVersionsList).

In `@nix/ext/tests/pg_stat_monitor.nix`:
- Around line 190-191: The PostgresExtensionTest is being instantiated without
the computed lib_name, causing the constructor to default to extension_name;
update the PostgresExtensionTest(...) call that currently uses (server,
extension_name, versions, sql_test_directory, support_upgrade, ext_schema) so it
includes the computed lib_name argument (the variable lib_name) in the parameter
list passed to PostgresExtensionTest, ensuring subsequent uses (e.g.,
test.create_schema() and later references to lib_name) target the correct
library name.

🧹 Nitpick comments (2)

nix/ext/pg_jsonschema/default.nix (1)

96-96: Consider removing or making debug output explicit.

The find $out command appears to be debugging output that lists build artifacts. If intentional for troubleshooting, consider adding a comment explaining its purpose or wrapping it in an echo statement. If unintentional, it can be removed.

🔧 Suggested change if keeping for debugging

 postInstall = ''
-  find $out
+  # Debug: list build artifacts
+  echo "Build artifacts:" && find $out
   mv $out/lib/${pname}${postgresql.dlSuffix} $out/lib/${pname}-${version}${postgresql.dlSuffix}

Or remove if not needed:

 postInstall = ''
-  find $out
   mv $out/lib/${pname}${postgresql.dlSuffix} $out/lib/${pname}-${version}${postgresql.dlSuffix}

nix/ext/tests/pgsodium.nix (1)

197-223: Ensure pg_upgrade completes before checking update script.
switch-to-configuration can return before services are fully ready; the immediate check for update_extensions.sql can race and flip has_update_script to no. Consider waiting for postgresql.service (or postgresql-migrate.service) after each switch.

🔧 Suggested wait to reduce flakiness

           server.succeed(
             f"{pg15_configuration}/bin/switch-to-configuration test >&2"
           )
+          server.wait_for_unit("postgresql.service")
           test.drop_extension()
           test.install_extension(version)
           server.succeed(
             f"{pg17_configuration}/bin/switch-to-configuration test >&2"
           )
+          server.wait_for_unit("postgresql.service")
           has_update_script = server.succeed(
             "test -f /var/lib/postgresql/update_extensions.sql && echo 'yes' || echo 'no'"
           ).strip() == "yes"

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between aa78cb3 and 423dd3d.

📒 Files selected for processing (11)

• nix/ext/pg_graphql/default.nix
• nix/ext/pg_jsonschema/default.nix
• nix/ext/pg_stat_monitor.nix
• nix/ext/tests/default.nix
• nix/ext/tests/lib.py
• nix/ext/tests/pg_stat_monitor.nix
• nix/ext/tests/pgmq.nix
• nix/ext/tests/pgroonga.nix
• nix/ext/tests/pgsodium.nix
• nix/ext/tests/vault.nix
• nix/ext/versions.json

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: nix-eval / eval

🔇 Additional comments (26)

nix/ext/tests/lib.py (1)

57-58: LGTM - CASCADE ensures clean teardown.

Adding CASCADE to the DROP EXTENSION command is appropriate for test cleanup, ensuring dependent objects (views, functions, etc.) are properly removed between test iterations. This aligns with the multi-version upgrade testing scenarios introduced in this PR.

nix/ext/pg_graphql/default.nix (3)

122-124: LGTM - Centralizing pgVersion improves maintainability.

Extracting pgVersion as a variable and using it consistently throughout the file is a good refactoring that aligns with the same pattern in pg_jsonschema/default.nix.

---

132-162: LGTM - SQL-only packages enable pg_upgrade for unsupported versions.

The buildUnsupported function correctly creates minimal SQL-only extension packages that pg_upgrade needs to reference old extension versions without requiring them to compile against the new PostgreSQL version. The approach of concatenating SQL files with semicolon separators is appropriate for this migration-only use case.

---

163-179: LGTM - Conditional inclusion of SQL-only packages for pg_upgrade.

The logic correctly:

1. Filters versions incompatible with the current pgVersion
2. Skips generating SQL-only packages for PG15 (no older versions to upgrade from)
3. Includes SQL-only packages in the build environment for PG17+ to support pg_upgrade

nix/ext/tests/pgroonga.nix (2)

141-143: LGTM - Configuration path variables.

The explicit system and configuration path variables improve readability and provide clear references for switching between PG15 and PG17 configurations during upgrade testing.

---

196-221: LGTM - Comprehensive pg_upgrade test for all PG15 extension versions.

This test thoroughly validates the upgrade path by:

1. Testing each PG15 extension version individually
2. Handling both scenarios: with update scripts (upgrade to latest) and without (version preserved)
3. Running pg_regress validation after each upgrade

The clean state reset (stopping postgres, removing data) between iterations ensures isolated test runs.

nix/ext/pg_jsonschema/default.nix (2)

78-80: LGTM - Consistent pgVersion usage in preCheck.

The pgVersion variable is used consistently for rsync paths and cargo pgrx initialization, matching the pattern established in pg_graphql/default.nix.

---

135-136: LGTM - Unsupported version handling for pg_upgrade.

The approach correctly:

1. Identifies PG15-only versions that won't compile against PG17
2. Only generates placeholders when building for PG17
3. Copies SQL from the earliest supported version as a migration placeholder

Note: This differs from pg_graphql which concatenates source SQL files. The simpler copy approach here works because pg_jsonschema's SQL structure allows it.

Also applies to: 153-155

nix/ext/versions.json (1)

778-779: LGTM - Marking pg_stat_monitor 1.0 as pg_upgrade incompatible.

The pgUpgradeCompatible: false flag correctly excludes this PG15-only version from pg_upgrade testing. The field is properly consumed in nix/ext/pg_stat_monitor.nix (lines 21, 25-29) to filter out incompatible versions from both the version list and build packages. Version 2.1, which supports both PG15 and PG17, will be used for upgrade validation instead.

nix/ext/tests/pgmq.nix (2)

122-125: LGTM: system-derived pg15/pg17 configuration paths.

This keeps configuration switching consistent with other test files.

---

182-205: LGTM: pg_upgrade coverage across older PG15 extension versions.

The loop validates upgrade behavior and update-script handling in a clear, repeatable way.

nix/ext/tests/vault.nix (2)

149-151: LGTM: consistent pg15/pg17 configuration derivation.

---

206-233: LGTM: pg_upgrade loop validates older-version upgrades and regression coverage.

nix/ext/tests/default.nix (3)

153-155: LGTM: system-based pg15/pg17 configuration paths.

---

212-237: LGTM: pg_upgrade loop for older extension versions.

---

261-264: LGTM: extension list update (pg_partman, postgis).

nix/ext/tests/pg_stat_monitor.nix (2)

30-119: LGTM: dedicated pg_stat_monitor test harness with pg15/pg17 specialisation.

---

230-264: LGTM: conditional pg_regress skip when no update script is generated.

Clear rationale and targeted handling for schema divergence after pg_upgrade.

nix/ext/tests/pgsodium.nix (8)

50-51: LGTM: explicit psql_15/psql_17 bindings.
Keeps reuse consistent across service and migration configuration.

---

71-85: Verify peer auth mapping matches how tests connect.
With local all postgres ... map=postgres and no postgres root postgres entry, any psql -U postgres executed as root will fail. Please confirm the harness uses sudo -u postgres (or similar), or add a mapping if root needs to connect as postgres.

Optional fallback if root must connect as postgres

 identMap = ''
   root root supabase_admin
+  postgres root postgres
   postgres postgres postgres
 '';

---

94-94: LGTM: pg17 specialization uses psql_17.

---

108-109: LGTM: explicit old/new PostgreSQL packages for pg_upgrade.

---

136-151: LGTM: harness wiring with explicit config variables.

---

158-175: LGTM: PG15 upgrade/pg_regress coverage.

---

179-179: LGTM: switch uses pg17_configuration.

---

182-195: LGTM: PG17 regression and last-version checks.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@nix/ext/pg_jsonschema/default.nix`:
- Around line 124-136: The unsupported-version detection is too narrow: replace
the filter that defines unsupportedVersionsItems (currently using
value.postgresql == [ "15" ]) with one that selects entries where the current
pgVersion is not listed (use !(builtins.elem pgVersion value.postgresql)), and
then set unsupportedVersions to lib.attrNames unsupportedVersionsItems (remove
the special-case if pgVersion == "17"). Update the variables
unsupportedVersionsItems and unsupportedVersions accordingly so all package
entries that don't support the current pgVersion are detected.

🧹 Nitpick comments (5)

nix/ext/tests/pgmq.nix (2)

48-48: Avoid duplicating pname in the test name.

Using name = pname avoids drift if the string changes later.

♻️ Proposed diff

-  name = "pgmq";
+  name = pname;

---

182-205: Consider validating functionality after each pg_upgrade iteration.

Right now the loop only checks versioning; adding the same after-create SQL + pg_regress step you already use elsewhere would catch runtime regressions for older versions.

🧪 Proposed diff

           else:
             # Otherwise, the version should match the version from postgresql 15
             test.assert_version_matches(version)
+
+          test.run_sql_file("${../../../ansible/files/postgresql_extension_custom_scripts/pgmq/after-create.sql}")
+          test.check_pg_regress(Path("${psql_17}/lib/pgxs/src/test/regress/pg_regress"), "17", pg_regress_test_name)

nix/ext/pg_graphql/default.nix (1)

132-180: Guard SQL-only packaging against accidental inclusion of update scripts.

buildUnsupported concatenates every sql/*.sql. If upstream includes versioned update scripts, they’d be folded into the base install file, which can skew pg_upgrade results. Consider preferring a prebuilt pg_graphql--${version}.sql when present and otherwise concatenating a deterministic, filtered list. Please verify against the upstream sql/ layout.

🧩 Proposed diff

-      installPhase = ''
-        mkdir -p $out/share/postgresql/extension
-        for file in $src/sql/*.sql; do
-          filename=$(basename "$file")
-          if [[ "$filename" != "load_sql_config.sql" && "$filename" != "load_sql_context.sql" ]]; then
-            cat "$file"
-            echo ";"
-          fi
-        done > $out/share/postgresql/extension/${pname}--${version}.sql
-      '';
+      installPhase = ''
+        mkdir -p $out/share/postgresql/extension
+        if [[ -f "$src/sql/${pname}--${version}.sql" ]]; then
+          cp "$src/sql/${pname}--${version}.sql" \
+            $out/share/postgresql/extension/${pname}--${version}.sql
+        else
+          for file in $(ls $src/sql/*.sql | sort -V); do
+            filename=$(basename "$file")
+            if [[ "$filename" != "load_sql_config.sql" \
+               && "$filename" != "load_sql_context.sql" \
+               && "$filename" != *"--"* ]]; then
+              cat "$file"
+              echo ";"
+            fi
+          done > $out/share/postgresql/extension/${pname}--${version}.sql
+        fi
+      '';

nix/ext/pg_jsonschema/default.nix (2)

153-155: Confirm which SQL version should seed unsupported versions.

lib.head versions copies the oldest supported SQL into every unsupported version. If the goal is to align with the most recent supported schema, consider lib.last versions or a version-specific mapping. Please confirm the intended behavior.

♻️ Alternative if “latest” is intended

-    for v in ${lib.concatStringsSep " " unsupportedVersions}; do
-      cp $out/share/postgresql/extension/${pname}--${lib.head versions}.sql $out/share/postgresql/extension/${pname}--$v.sql
-    done
+    for v in ${lib.concatStringsSep " " unsupportedVersions}; do
+      cp $out/share/postgresql/extension/${pname}--${lib.last versions}.sql $out/share/postgresql/extension/${pname}--$v.sql
+    done

---

95-97: Consider dropping the debug find $out.

It’s noisy and slows logs without affecting outputs.

🧹 Proposed cleanup

-      postInstall = ''
-        find $out
+      postInstall = ''

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@nix/ext/pg_jsonschema/default.nix`:
- Line 96: Remove the leftover debug command `find $out` from the build script
so it no longer prints the entire output tree during builds; locate the `find
$out` invocation (the standalone shell command) in the nix expression and delete
that line (or guard it behind a debug flag if you need it for local debugging)
so build logs are no longer cluttered.