Skip to content

Fix upgrade test: allow function removal#2422

Merged
muhammadshoaib merged 1 commit intoapache:masterfrom
jrgemignani:fix_upgrade_function_removal
May 5, 2026
Merged

Fix upgrade test: allow function removal#2422
muhammadshoaib merged 1 commit intoapache:masterfrom
jrgemignani:fix_upgrade_function_removal

Conversation

@jrgemignani
Copy link
Copy Markdown
Contributor

Fix upgrade test: allow function removal and detect more deficiencies.

The age_upgrade regression test (added in #2364, refined in #2377, #2397, install and a synthetic-initial -> current upgrade. Three gaps surfaced in practice:

  1. Function removal forced permanent C stubs. The synthetic '_initial' install is built from a fixed historical commit. CREATE EXTENSION resolves every CREATE FUNCTION ... AS '$libdir/age', '' via dlsym at install time when check_function_bodies is on (the default). If a developer retires a C entry point in HEAD's age.so, step 10 aborts with "could not find function ... in file age.so" -- even though the immediately-following ALTER EXTENSION UPDATE would DROP that SQL declaration. The only way to keep the test green was to leave a permanent error-raising stub in age.so, and to remember to add a DROP to the upgrade template.

  2. Modifications were under-detected. The function-property-change query did not compare probin or prosrc, so a C function whose symbol was renamed in the upgrade template, or a SQL/plpgsql function whose body changed in either path, slipped through.

  3. Extension membership was not checked. A template that CREATEs an object but never ALTER EXTENSION ADDs it leaves a row in pg_proc/pg_class but no pg_depend deptype='e' link. pg_dump --extension would diverge, but the existing per-catalog diff queries all returned 0 rows.

Changes (regress/sql/age_upgrade.sql + regress/expected/age_upgrade.out):

  • Step 10 wraps the synthetic CREATE EXTENSION in SET check_function_bodies = off; ... RESET check_function_bodies; Symbol resolution is deferred to call time. Step 11's ALTER EXTENSION UPDATE then DROPs any retired functions before any plan can call them. Step 35's fresh CREATE EXTENSION runs at the GUC default, so HEAD's sql/ <-> HEAD's age.so consistency is still enforced on the production install path.

  • Steps 2 and 13 add probin and prosrc to the function snapshot. Step 21 reports probin and prosrc divergences alongside the existing property-change columns.

  • Steps 7b and 18b add an extension-membership snapshot from pg_depend deptype='e' filtered to the AGE extension OID. Every member is labeled by stable identity (regprocedure, regtype, regoperator, opfname+strategy+types, etc.), never by raw OID, so OID drift between fresh and upgrade installs cannot produce false positives. Steps 33a and 33b report MISSING / EXTRA members. Step 34 adds extmembers_match to the summary row.

  • Section-header step ranges updated to include the new sub-steps.

The change is fully self-contained: only regress/sql/age_upgrade.sql and regress/expected/age_upgrade.out are modified. No production C, SQL, build, or test files are touched. All 34 regression tests pass on a clean tree.

Mutation-tested with 8 cases against the unmutated tree: baseline pass; remove-function-with-DROP pass (no stub needed); remove-function-forget- DROP fail; add-function-with-CREATE pass; add-function-forget-CREATE fail; volatility-change-no-template fail; volatility-change-with-CREATE- OR-REPLACE pass; C-symbol-rename-no-template fail. All eight expected outcomes observed.

All 34 regression tests pass.

Co-authored-by: Claude [email protected]

modified: regress/expected/age_upgrade.out
modified: regress/sql/age_upgrade.sql

Fix upgrade test: allow function removal and detect more deficiencies.

The age_upgrade regression test (added in apache#2364, refined in apache#2377, apache#2397,
install and a synthetic-initial -> current upgrade. Three gaps surfaced
in practice:

1. Function removal forced permanent C stubs.
   The synthetic '_initial' install is built from a fixed historical
   commit. CREATE EXTENSION resolves every CREATE FUNCTION ... AS
   '$libdir/age', '<symbol>' via dlsym at install time when
   check_function_bodies is on (the default). If a developer retires a
   C entry point in HEAD's age.so, step 10 aborts with "could not find
   function ... in file age.so" -- even though the immediately-following
   ALTER EXTENSION UPDATE would DROP that SQL declaration. The only way
   to keep the test green was to leave a permanent error-raising stub
   in age.so, and to remember to add a DROP to the upgrade template.

2. Modifications were under-detected.
   The function-property-change query did not compare probin or prosrc,
   so a C function whose symbol was renamed in the upgrade template, or
   a SQL/plpgsql function whose body changed in either path, slipped
   through.

3. Extension membership was not checked.
   A template that CREATEs an object but never ALTER EXTENSION ADDs it
   leaves a row in pg_proc/pg_class but no pg_depend deptype='e' link.
   pg_dump --extension would diverge, but the existing per-catalog diff
   queries all returned 0 rows.

Changes (regress/sql/age_upgrade.sql + regress/expected/age_upgrade.out):

* Step 10 wraps the synthetic CREATE EXTENSION in
  SET check_function_bodies = off; ... RESET check_function_bodies;
  Symbol resolution is deferred to call time. Step 11's ALTER EXTENSION
  UPDATE then DROPs any retired functions before any plan can call them.
  Step 35's fresh CREATE EXTENSION runs at the GUC default, so HEAD's
  sql/ <-> HEAD's age.so consistency is still enforced on the production
  install path.

* Steps 2 and 13 add probin and prosrc to the function snapshot.
  Step 21 reports probin and prosrc divergences alongside the existing
  property-change columns.

* Steps 7b and 18b add an extension-membership snapshot from
  pg_depend deptype='e' filtered to the AGE extension OID. Every member
  is labeled by stable identity (regprocedure, regtype, regoperator,
  opfname+strategy+types, etc.), never by raw OID, so OID drift between
  fresh and upgrade installs cannot produce false positives. Steps 33a
  and 33b report MISSING / EXTRA members. Step 34 adds extmembers_match
  to the summary row.

* Section-header step ranges updated to include the new sub-steps.

The change is fully self-contained: only regress/sql/age_upgrade.sql and
regress/expected/age_upgrade.out are modified. No production C, SQL,
build, or test files are touched. All 34 regression tests pass on a
clean tree.

Mutation-tested with 8 cases against the unmutated tree: baseline pass;
remove-function-with-DROP pass (no stub needed); remove-function-forget-
DROP fail; add-function-with-CREATE pass; add-function-forget-CREATE
fail; volatility-change-no-template fail; volatility-change-with-CREATE-
OR-REPLACE pass; C-symbol-rename-no-template fail. All eight expected
outcomes observed.

All 34 regression tests pass.

Co-authored-by: Claude <[email protected]>

modified:   regress/expected/age_upgrade.out
modified:   regress/sql/age_upgrade.sql
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the age_upgrade regression test to better validate extension upgrade templates by (1) allowing retired C entry points to be removed without leaving permanent stubs, and (2) detecting more catalog/template deficiencies (function binding/body changes and missing extension membership links).

Changes:

  • Disable check_function_bodies only for the synthetic *_initial CREATE EXTENSION so removed C symbols don’t fail the initial install before the upgrade script can drop them.
  • Expand function snapshots/comparisons to include pg_proc.probin and pg_proc.prosrc to catch C symbol renames and SQL/plpgsql body changes.
  • Add pg_depend (deptype='e') extension-membership snapshot and compare fresh vs upgraded membership to detect missing ALTER EXTENSION ... ADD entries.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
regress/sql/age_upgrade.sql Enhances upgrade regression logic: toggles check_function_bodies for synthetic install; adds probin/prosrc and extension-membership snapshots and comparisons.
regress/expected/age_upgrade.out Updates expected output to match the new steps, result columns, and summary fields.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@muhammadshoaib
Copy link
Copy Markdown
Contributor

Looks fine.

@jrgemignani
Copy link
Copy Markdown
Contributor Author

Looks fine.

Looks fine approval? or merge? ;)

@jrgemignani jrgemignani requested a review from muhammadshoaib May 5, 2026 20:33
Copy link
Copy Markdown
Contributor

@muhammadshoaib muhammadshoaib left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine

@muhammadshoaib muhammadshoaib merged commit a1b749a into apache:master May 5, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants