diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2f08de6..a7a6ad3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -606,28 +606,39 @@ jobs: # the current version) and are otherwise never exercised. Both origins are # checked because ALTER EXTENSION UPDATE takes the shortest path (see the # pg-upgrade-test matrix): 0.2.0 goes via the 0.2.0--0.2.2 script, 0.2.1 via - # 0.2.1--0.2.2. update-check asserts each lands on 0.2.2. No suite runs (the - # current version needs PG12+). + # 0.2.1--0.2.2. update-check-version asserts each lands on 0.2.2 -- NOT + # plain update-check: landing on 0.2.2 from these origins is a KNOWN + # divergence from a fresh 0.2.2 install (trigger__parse and the + # pg_class_v omit_column bug, repaired in 0.2.2->0.2.3; a type-ACL gap, + # repaired only in 0.2.3->0.3.0), and both 0.2.0->0.2.2 / 0.2.1->0.2.2 + # are already-published scripts that cannot be edited to fix it + # directly, so asserting fresh-parity here would fail forever by + # design. No suite runs (the current version needs PG12+). # # The rebuild_020/rebuild_021 checks exercise the REAL broken path end to # end from BOTH pre-0.2.2 origins: a 0.2.0 (resp. 0.2.1) install on PG10 # leaves relhasoids in _cat_tools.pg_class_v (the buggy 0.2.0--0.2.2 / # 0.2.1--0.2.2 omit_column no-op), and updating to 0.2.3 routes through # 0.2.2--0.2.3 (shortest path --0.2.2 then 0.2.2--0.2.3), firing - # the conditional rebuild that strips relhasoids. 0.2.3 is the furthest - # PG10 can reach (0.2.3--0.3.0 needs PG12+). On PG12+ relhasoids never - # existed, so only PG10 exercises the rebuild. The psql assertion fails - # loudly if the rebuild did not fire (relhasoids still present) -- - # complementing the stronger 10→18 pg_upgrade bridge legs. `$$` is escaped - # as `\$\$` so the shell passes literal dollar quotes through to psql. + # the conditional rebuild that strips relhasoids and the trigger__parse + # repair. It stays update-check-version, not plain update-check: 0.2.3 + # is ALSO already-published (tagged), so the type-ACL gap above is not + # (and cannot be) fixed until 0.2.3->0.3.0 either -- this landing point + # still diverges from a fresh 0.2.3 install on that ACL alone. 0.2.3 is + # the furthest PG10 can reach (0.2.3--0.3.0 needs PG12+). On PG12+ + # relhasoids never existed, so only PG10 exercises the rebuild. The + # psql assertion fails loudly if the rebuild did not fire (relhasoids + # still present) -- complementing the stronger 10→18 pg_upgrade bridge + # legs. `$$` is escaped as `\$\$` so the shell passes literal dollar + # quotes through to psql. if: matrix.pg == '10' run: | - bin/test_existing update-check cat_tools_from_020 0.2.0 0.2.2 - bin/test_existing update-check cat_tools_from_021 0.2.1 0.2.2 + bin/test_existing update-check-version cat_tools_from_020 0.2.0 0.2.2 + bin/test_existing update-check-version cat_tools_from_021 0.2.1 0.2.2 for origin in 020:0.2.0 021:0.2.1; do db="cat_tools_rebuild_${origin%%:*}" from="${origin##*:}" - bin/test_existing update-check "$db" "$from" 0.2.3 + bin/test_existing update-check-version "$db" "$from" 0.2.3 psql -d "$db" -v ON_ERROR_STOP=1 -c "DO \$\$ BEGIN IF EXISTS (SELECT 1 FROM pg_attribute WHERE attrelid='_cat_tools.pg_class_v'::regclass AND attname='relhasoids' AND NOT attisdropped AND attnum>0) THEN RAISE EXCEPTION 'pg_class_v still exposes relhasoids after update through 0.2.2->0.2.3 -- rebuild did not fire'; END IF; END \$\$" done - name: Update 0.2.2 → current and run the suite (existing mode, PG12+) diff --git a/bin/structural_diff b/bin/structural_diff new file mode 100755 index 0000000..35749ad --- /dev/null +++ b/bin/structural_diff @@ -0,0 +1,87 @@ +#!/usr/bin/env bash +# +# Structurally compare, object-by-object, every member of the cat_tools +# extension in two databases: function bodies (pg_get_functiondef), view +# definitions (pg_get_viewdef), type labels/columns (enums, the one plain +# table, the one standalone composite type), comments, and ACLs. A nonempty +# diff is a bug -- the whole point of an extension UPDATE script is that it +# reaches the SAME objects a fresh install of the target version would. +# +# This generalizes a manual comparison run by hand while fixing +# https://github.com/Postgres-Extensions/cat_tools/pull/46: that PR found a +# real fresh-vs-update divergence this way (cat_tools.trigger__parse, as +# produced by the pre-0.2.2 update scripts, hardcoded `EXECUTE PROCEDURE` and +# skipped an empty-args guard that the fresh-install body had, breaking every +# trigger parse on PG11+). Committing the comparison as a script makes it a +# standing, automated check instead of something that has to be remembered +# and run by hand. +# +# USAGE: bin/structural_diff [args] +# +# dump DB [EXTNAME] +# Print the signature of every EXTNAME member object in DB (EXTNAME +# defaults to cat_tools). Useful on its own for eyeballing one +# database's structure, and it's what `compare` diffs under the hood. +# +# compare DB1 DB2 [EXTNAME] +# Dump both databases and diff them. Prints a unified diff and exits +# non-zero if they differ; exits 0 (and prints an OK line) if +# identical. +# +# See bin/structural_diff.sql for the query that defines "signature" (and how +# it decides which object kinds get a real structural definition vs. falling +# back to just identity/comment/ACL). +set -euo pipefail + +SCRIPT_DIR=$(cd "$(dirname "$(readlink -f "$0")")" && pwd) + +# --------------------------------------------------------------------------- +# Subcommand implementations +# --------------------------------------------------------------------------- + +dump() { + local db=$1 extname=${2:-cat_tools} + psql -d "$db" -v extname="'$extname'" -f "$SCRIPT_DIR/structural_diff.sql" +} + +compare() { + local db1=$1 db2=$2 extname=${3:-cat_tools} + # Run in a subshell so the EXIT trap (temp-file cleanup) is scoped to this + # comparison only. A trap set with plain `trap ... RETURN` is NOT scoped to + # the function that set it -- it re-fires on every later function return in + # the same shell, including main()'s, by which point f1/f2 no longer exist. + ( + f1=$(mktemp) + f2=$(mktemp) + trap 'rm -f "$f1" "$f2"' EXIT + dump "$db1" "$extname" > "$f1" + dump "$db2" "$extname" > "$f2" + if diff -u --label "$db1" --label "$db2" "$f1" "$f2"; then + echo "OK: '$db1' and '$db2' are structurally identical for extension '$extname'" + else + echo "FAIL: structural diff between '$db1' and '$db2' for extension '$extname' (see diff above) -- an update path reached objects that differ from a fresh install" >&2 + exit 1 + fi + ) +} + +usage() { + echo "usage: bin/structural_diff [args]" >&2 + echo " dump DB [EXTNAME]" >&2 + echo " compare DB1 DB2 [EXTNAME]" >&2 + exit 2 +} + +# Explicit subcommand dispatch on $1, matching bin/test_existing and +# bin/assert_fs_clean's convention. +main() { + local cmd=${1:-} + shift || true + case "$cmd" in + dump) dump "$@" ;; + compare) compare "$@" ;; + *) usage ;; + esac +} + +main "$@" diff --git a/bin/structural_diff.sql b/bin/structural_diff.sql new file mode 100644 index 0000000..8d52cb5 --- /dev/null +++ b/bin/structural_diff.sql @@ -0,0 +1,170 @@ +/* + * Structural signature dump for every object that belongs to an extension + * (pg_depend deptype = 'e'), used by bin/structural_diff to compare a + * database reached via an extension UPDATE against a FRESH install of the + * same target version. Any nonempty diff between two runs of this query is a + * bug: the two paths are supposed to produce byte-identical objects. + * + * Run via: psql -d DBNAME -v extname="'cat_tools'" -f bin/structural_diff.sql + * + * Origin: this generalizes a manual comparison technique (diffing + * pg_get_functiondef / pg_get_viewdef / type labels / comments / ACLs / + * extension membership between a fresh install and an updated database) used + * to find the trigger__parse divergence fixed in + * https://github.com/Postgres-Extensions/cat_tools/pull/46. + * + * Emits one text block per member object, ordered by its pg_describe_object() + * identity so the SAME object sorts to the SAME position regardless of the + * OIDs assigned along each installation path. Each block covers: + * - a structural definition, using pg_get_functiondef/pg_get_viewdef for + * routines/views, an ordered column dump for the one plain table and the + * one standalone composite type this extension has, and an ordered label + * list for enums -- whichever a member's catalog/kind actually calls for. + * Row types implicitly created BY a member relation, and the array type + * shadowing any other member type, are skipped: their structure is fully + * captured by the relation/base-type entry already, so listing them again + * would just duplicate that comparison under a second identity. + * - its comment (pg_description), generically via obj_description(). + * - its ACL, generically via whichever ACL column its catalog has (proacl / + * typacl / relacl / nspacl); sorted, since grant order is not meaningful. + * + * This is deliberately NOT specific to cat_tools's current object list: any + * object kind this extension does not (yet) use falls through to the ELSE + * branch below, which still includes it (via its pg_describe_object identity, + * comment and ACL) so a future new member is compared at least at that level + * rather than silently skipped, even though this file does not (yet) know how + * to render a structural definition for it. + */ +\set ON_ERROR_STOP on +\pset format unaligned +\pset tuples_only on +\pset fieldsep '' + +WITH ext AS ( + SELECT oid FROM pg_extension WHERE extname = :extname +), members AS ( + SELECT d.classid, d.objid + FROM pg_depend d, ext + WHERE d.refclassid = 'pg_extension'::regclass + AND d.refobjid = ext.oid + AND d.deptype = 'e' +), skip_shadow AS ( + /* Implicit row type of a member relation: same structure as the relation + * itself, so comparing it too would just duplicate that check. */ + SELECT t.oid + FROM pg_type t + JOIN members rel ON rel.classid = 'pg_class'::regclass AND rel.objid = t.typrelid + WHERE t.typtype = 'c' + UNION + /* Array type shadowing another member type: same element type, no + * independent structure of its own. */ + SELECT t.oid + FROM pg_type t + JOIN members base ON base.classid = 'pg_type'::regclass AND base.objid = t.typelem + WHERE t.typelem <> 0 +), acl AS ( + SELECT m.classid, m.objid, + ( + SELECT array_to_string(array_agg(a::text ORDER BY a::text), ',') + FROM unnest( + CASE m.classid + WHEN 'pg_proc'::regclass THEN (SELECT proacl FROM pg_proc WHERE oid = m.objid) + WHEN 'pg_type'::regclass THEN (SELECT typacl FROM pg_type WHERE oid = m.objid) + WHEN 'pg_class'::regclass THEN (SELECT relacl FROM pg_class WHERE oid = m.objid) + WHEN 'pg_namespace'::regclass THEN (SELECT nspacl FROM pg_namespace WHERE oid = m.objid) + ELSE NULL + END + ) a + ) AS acl_text + FROM members m +), relation_cols AS ( + /* Ordered column dump, shared by the plain-table case (pg_class relkind + * 'r') and the standalone-composite-type case (pg_type typtype 'c' whose + * typrelid is NOT a member relation, i.e. survived skip_shadow) -- both + * describe a set of (name, type, not-null, default) columns identically. */ + SELECT m.classid, m.objid, + ( + SELECT string_agg( + format( + '%s %s%s%s' + , a.attname + , format_type(a.atttypid, a.atttypmod) + , CASE WHEN a.attnotnull THEN ' NOT NULL' ELSE '' END + , COALESCE(' DEFAULT ' || pg_get_expr(ad.adbin, ad.adrelid), '') + ) + , E'\n' ORDER BY a.attnum + ) + FROM pg_attribute a + LEFT JOIN pg_attrdef ad ON ad.adrelid = a.attrelid AND ad.adnum = a.attnum + WHERE a.attrelid = CASE m.classid + WHEN 'pg_class'::regclass THEN m.objid + WHEN 'pg_type'::regclass THEN (SELECT typrelid FROM pg_type WHERE oid = m.objid) + END + AND a.attnum > 0 + AND NOT a.attisdropped + ) + /* Table constraints (PK/UNIQUE/CHECK/FK) have no equivalent on a + * standalone composite type, so this is NULL there and simply appends + * nothing. */ + || COALESCE( + E'\n' || ( + SELECT string_agg(pg_get_constraintdef(c.oid), E'\n' ORDER BY c.conname) + FROM pg_constraint c + WHERE m.classid = 'pg_class'::regclass AND c.conrelid = m.objid + ) + , '' + ) AS cols + FROM members m + WHERE (m.classid = 'pg_class'::regclass AND (SELECT relkind FROM pg_class WHERE oid = m.objid) = 'r') + OR (m.classid = 'pg_type'::regclass AND (SELECT typtype FROM pg_type WHERE oid = m.objid) = 'c') +) +SELECT + '=== ' || pg_describe_object(m.classid, m.objid, 0) || E' ===\n' + || 'DEFINITION:' || E'\n' || COALESCE( + CASE + WHEN m.classid = 'pg_proc'::regclass + THEN pg_get_functiondef(m.objid) + WHEN m.classid = 'pg_class'::regclass AND (SELECT relkind FROM pg_class WHERE oid = m.objid) IN ('v', 'm') + THEN pg_get_viewdef(m.objid, true) + WHEN m.classid = 'pg_class'::regclass AND (SELECT relkind FROM pg_class WHERE oid = m.objid) = 'r' + THEN (SELECT cols FROM relation_cols rc WHERE rc.classid = m.classid AND rc.objid = m.objid) + WHEN m.classid = 'pg_type'::regclass AND (SELECT typtype FROM pg_type WHERE oid = m.objid) = 'e' + THEN (SELECT string_agg(enumlabel, ',' ORDER BY enumsortorder) FROM pg_enum WHERE enumtypid = m.objid) + WHEN m.classid = 'pg_type'::regclass AND (SELECT typtype FROM pg_type WHERE oid = m.objid) = 'c' + THEN (SELECT cols FROM relation_cols rc WHERE rc.classid = m.classid AND rc.objid = m.objid) + WHEN m.classid = 'pg_type'::regclass AND (SELECT typtype FROM pg_type WHERE oid = m.objid) = 'd' + THEN ( + SELECT format( + 'base=%s notnull=%s default=%s check=%s' + , t.typbasetype::regtype, t.typnotnull, t.typdefault + , (SELECT string_agg(pg_get_constraintdef(c.oid), ' AND ' ORDER BY c.oid) + FROM pg_constraint c WHERE c.contypid = m.objid) + ) + FROM pg_type t WHERE t.oid = m.objid + ) + WHEN m.classid = 'pg_cast'::regclass + THEN ( + SELECT format( + 'CAST (%s AS %s) METHOD %s CONTEXT %s' + , ct.castsource::regtype, ct.casttarget::regtype + , CASE ct.castmethod + WHEN 'f' THEN 'FUNCTION ' || ct.castfunc::regprocedure::text + WHEN 'i' THEN 'INOUT' + WHEN 'b' THEN 'BINARY COERCION' + END + , ct.castcontext + ) + FROM pg_cast ct WHERE ct.oid = m.objid + ) + ELSE NULL + END + , '(no structural definition rendered for this object kind -- see identity/comment/ACL below)' + ) + || E'\n' || 'COMMENT: ' || COALESCE(obj_description(m.objid, m.classid::regclass::text), '(none)') + || E'\n' || 'ACL: ' || COALESCE((SELECT acl_text FROM acl WHERE acl.classid = m.classid AND acl.objid = m.objid), '(none)') + || E'\n' + AS block + FROM members m + LEFT JOIN skip_shadow s ON m.classid = 'pg_type'::regclass AND s.oid = m.objid + WHERE s.oid IS NULL + ORDER BY pg_describe_object(m.classid, m.objid, 0); diff --git a/bin/test_existing b/bin/test_existing index 32a4f4c..8a34b4f 100755 --- a/bin/test_existing +++ b/bin/test_existing @@ -28,10 +28,31 @@ # # update-scenario DB FROM_VERSION # Create DB + extension at FROM_VERSION, plant guard, update to current, -# run the suite against that real updated database. +# structurally compare against a fresh install of current (see +# diff-fresh), run the suite against that real updated database. # # update-check DB FROM_VERSION TO_VERSION -# Lightweight check that a specific update script applies (no suite). +# Lightweight check that a specific update script applies (no suite), +# then structurally compare against a fresh install of TO_VERSION (see +# diff-fresh). +# +# update-check-version DB FROM_VERSION TO_VERSION +# Same as update-check but WITHOUT the structural comparison: for a +# landing point pinned at an already-tagged version, where a fresh- +# install divergence is documented and can only converge forward into a +# LATER, still-unpublished update script (an already-published +# version-specific file is never retroactively edited). Only asserts the +# version landed. +# +# diff-fresh DB VERSION +# Structurally compare DB's cat_tools objects against a throwaway fresh +# install of VERSION (function/view definitions, type labels, comments, +# ACLs, extension membership -- see bin/structural_diff.sql). Fails loudly +# on any nonempty diff. Generalizes a manual comparison that found the +# trigger__parse divergence fixed by +# https://github.com/Postgres-Extensions/cat_tools/pull/46; update-scenario +# and update-check both call this automatically, but it's also directly +# useful standalone against any already-populated database. # # Run `bin/test_existing` with no subcommand to print usage. # @@ -168,6 +189,30 @@ update_ext() { psql_do "$db" -c "ALTER EXTENSION cat_tools UPDATE $to" } +# Structurally compare DB's cat_tools objects (function/view definitions, type +# labels, comments, ACLs, extension membership -- see bin/structural_diff.sql) +# against a FRESH install of VERSION. An update script only really "supports" +# a version if updating to it reaches the SAME objects a fresh install would; +# https://github.com/Postgres-Extensions/cat_tools/pull/46 found a real +# divergence this way by hand (a pre-0.2.2 update script's trigger__parse body +# didn't match the fresh-install body, breaking every trigger parse on +# PG11+). Runs in a subshell so the EXIT trap dropping the throwaway reference +# database is scoped to this call, not the whole script (see the same pattern, +# and why it must be a subshell rather than a plain function-local trap, in +# bin/structural_diff's compare()). +assert_matches_fresh() { + local db=$1 version=$2 + # Separate statement: a self-referencing `local a=1 b=$a` is bash-version- + # dependent on whether $a is visible yet while computing b's value. + local fresh_db="${db}__fresh_ref" + ( + trap 'dropdb --if-exists "$fresh_db"' EXIT + createdb "$fresh_db" + psql_do "$fresh_db" -c "CREATE EXTENSION cat_tools VERSION '$version'" + bin/structural_diff compare "$db" "$fresh_db" + ) +} + # --------------------------------------------------------------------------- # Subcommand implementations # --------------------------------------------------------------------------- @@ -198,21 +243,40 @@ prepare_old() { # update-scenario DB FROM_VERSION # Full extension-update flow that runs the existing-mode suite: create the DB # and extension at FROM_VERSION, plant + prove the guard, update to the current -# version, then run the suite against that real updated database. +# version, structurally compare the result against a fresh install of the +# current version (assert_matches_fresh), then run the suite against that +# real updated database. update_scenario() { local db=$1 from=$2 createdb "$db" psql_do "$db" -c "CREATE EXTENSION cat_tools VERSION '$from'" plant_guard "$db" update_ext "$db" + assert_matches_fresh "$db" "$(current_version)" run_suite "$db" } -# update-check DB FROM_VERSION TO_VERSION -# Lightweight check that a specific update script applies (no suite): used for -# the pre-0.2.2 scripts, which only install on PG10 and target 0.2.2 (not the -# current version, so the suite cannot run against them). -update_check() { +# update-check-version DB FROM_VERSION TO_VERSION +# Lightweight check that a specific update script applies (no suite): used +# for the pre-0.2.2 scripts, which only install on PG10 and target 0.2.2 (not +# the current version, so the suite cannot run against them). Also used for +# ANY update path landing on 0.2.3, regardless of origin -- 0.2.3 is itself +# already tagged/published, and a version-specific file is NEVER edited once +# tagged (CLAUDE.md's SQL file conventions rule 5), so a divergence at that +# landing point can never be repaired in place; the fix has to converge +# forward into 0.2.3->0.3.0 instead, the same way PR #46 converged its own +# fix forward into 0.2.2->0.2.3 rather than editing 0.2.0/0.2.1's already- +# published scripts. Two such divergences exist for 0.2.3: a comment-only +# drift between the frozen sql/cat_tools--0.2.3.sql.in fresh-install script +# (never got PR #46's comment tweaks) and the 0.2.2->0.2.3 update script +# (which did) affects EVERY origin, including a fresh 0.2.2; a type-ACL gap +# on five enum types predating 0.2.2 affects only a 0.2.0/0.2.1 origin +# (0.2.2->0.2.3 never grants it, since a fresh 0.2.2 already has it and so +# never needed the grant added there). Either way, asserting fresh-parity at +# a landing point pinned to an already-tagged version fails forever by +# design, not from a regression -- see update-check's comment for the +# version where full parity IS expected and proven. +update_check_version() { local db=$1 from=$2 to=$3 createdb "$db" psql_do "$db" -c "CREATE EXTENSION cat_tools VERSION '$from'" @@ -220,6 +284,30 @@ update_check() { assert_version "$db" "$to" } +# update-check DB FROM_VERSION TO_VERSION +# Same as update-check-version, plus a structural comparison against a fresh +# install of TO_VERSION (assert_matches_fresh) -- this is precisely the shape +# of check that would have caught +# https://github.com/Postgres-Extensions/cat_tools/pull/46's trigger__parse +# divergence: an update script reaching TO_VERSION with a different function +# body than a fresh install of it. Only use this where TO_VERSION is a +# target the update path is actually expected to converge on. Every +# ALREADY-TAGGED version is a poor fit for this: a comment-only tweak PR #46 +# made to sql/cat_tools.sql.in and the 0.2.2->0.2.3 update script never +# reached the frozen sql/cat_tools--0.2.3.sql.in fresh-install script (a +# version-specific file, once tagged, is NEVER edited to fix this kind of +# drift -- see CLAUDE.md's SQL file conventions rule 5), so ANY path +# reaching 0.2.3 diverges from "fresh install pinned at exactly 0.2.3", +# regardless of origin -- use update-check-version for any TO_VERSION that +# is already tagged. Reaching the CURRENT version is where convergence is +# both expected and proven (see update-scenario, and diff-fresh against +# any already-populated database). +update_check() { + local db=$1 from=$2 to=$3 + update_check_version "$db" "$from" "$to" + assert_matches_fresh "$db" "$to" +} + # Run the pgTAP suite against an already-populated database in existing mode. # Verifies the extension is at the current version, re-proves the guard still # blocks a drop (i.e. it survived the update/upgrade), runs the suite via @@ -273,6 +361,8 @@ usage() { echo " run-suite DB" >&2 echo " update-scenario DB FROM_VERSION" >&2 echo " update-check DB FROM_VERSION TO_VERSION" >&2 + echo " update-check-version DB FROM_VERSION TO_VERSION" >&2 + echo " diff-fresh DB VERSION" >&2 exit 2 } @@ -283,13 +373,15 @@ main() { local cmd=${1:-} shift || true case "$cmd" in - plant-guard) plant_guard "$@" ;; - update) update_ext "$@" ;; - prepare-old) prepare_old "$@" ;; - run-suite) run_suite "$@" ;; - update-scenario) update_scenario "$@" ;; - update-check) update_check "$@" ;; - *) usage ;; + plant-guard) plant_guard "$@" ;; + update) update_ext "$@" ;; + prepare-old) prepare_old "$@" ;; + run-suite) run_suite "$@" ;; + update-scenario) update_scenario "$@" ;; + update-check) update_check "$@" ;; + update-check-version) update_check_version "$@" ;; + diff-fresh) assert_matches_fresh "$@" ;; + *) usage ;; esac } diff --git a/sql/cat_tools--0.2.3--0.3.0.sql.in b/sql/cat_tools--0.2.3--0.3.0.sql.in index 86d9bd2..a24c6cc 100644 --- a/sql/cat_tools--0.2.3--0.3.0.sql.in +++ b/sql/cat_tools--0.2.3--0.3.0.sql.in @@ -101,6 +101,12 @@ SELECT __cat_tools.create_function( , 'pg_catalog.regprocedure LANGUAGE plpgsql' , $body$ DECLARE + /* + * Template for creating a temporary function with the user-provided argument + * signature. This allows us to leverage PostgreSQL's parser to validate and + * extract argument information without permanently creating a function. + * Using plpgsql language for the temp function to handle any return type. + */ c_template CONSTANT text := $fmt$CREATE FUNCTION pg_temp.cat_tools__function__%s__temp_function( %s ) RETURNS %s LANGUAGE plpgsql AS 'BEGIN RETURN; END' @@ -109,9 +115,13 @@ DECLARE temp_proc pg_catalog.regprocedure; sql text; BEGIN + /* + * Security check: Ensure current_user == session_user to detect SECURITY DEFINER context + * This prevents SQL injection attacks through elevated privileges. + */ IF current_user != session_user THEN RAISE EXCEPTION USING - ERRCODE = '28000' + ERRCODE = '28000' -- invalid_authorization_specification , MESSAGE = 'potential use of SECURITY DEFINER detected' , DETAIL = format('current_user is %s, session_user is %s', current_user, session_user) , HINT = 'Helper functions must not be called from SECURITY DEFINER context.'; @@ -122,6 +132,7 @@ BEGIN , arguments , 'void' ); + --RAISE DEBUG 'Executing SQL %', sql; DECLARE v_type pg_catalog.regtype; BEGIN @@ -137,6 +148,12 @@ BEGIN EXECUTE sql; END; + /* + * Get new OID. *This must be done dynamically!* Otherwise we get stuck + * with a CONST oid after first compilation. The regproc cast ensures there's + * only one function with this name. The cast to regprocedure is for the sake + * of the DROP down below. + */ EXECUTE format( $$SELECT 'pg_temp.cat_tools__function__%s__temp_function'::pg_catalog.regproc::pg_catalog.regprocedure$$ , function_suffix @@ -155,9 +172,13 @@ SELECT __cat_tools.create_function( , 'void LANGUAGE plpgsql' , $body$ BEGIN + /* + * Security check: Ensure current_user == session_user to detect SECURITY DEFINER context + * This prevents SQL injection attacks through elevated privileges. + */ IF current_user != session_user THEN RAISE EXCEPTION USING - ERRCODE = '28000' + ERRCODE = '28000' -- invalid_authorization_specification , MESSAGE = 'potential use of SECURITY DEFINER detected' , DETAIL = format('API function %s must not be called from a SECURITY DEFINER function', api_function_name) , HINT = 'We detect SECURITY DEFINER context by comparing current_user and session_user, which can cause false positives if SET ROLE is used'; @@ -429,6 +450,7 @@ SELECT __cat_tools.create_function( SELECT CASE WHEN proargnames IS NULL THEN + -- No named arguments, return array of NULLs matching proargtypes length CASE WHEN pronargs > 0 THEN array_fill(NULL::text, ARRAY[pronargs]) @@ -436,11 +458,13 @@ SELECT '{}'::text[] END WHEN proargmodes IS NULL THEN + -- All arguments are IN mode, proargnames and proargtypes align array( SELECT CASE WHEN name = '' THEN NULL ELSE name END FROM unnest(proargnames) AS name ) ELSE + -- Mixed argument modes, need to filter names to match proargtypes array( SELECT CASE @@ -493,7 +517,10 @@ DECLARE result pg_catalog.regtype[]; BEGIN result := cat_tools.routine__arg_types(c_temp_proc); + + -- Clean up the temporary function PERFORM _cat_tools.function__drop_temp(c_temp_proc, 'cat_tools.routine__parse_arg_types'); + RETURN result; END $body$ @@ -514,7 +541,10 @@ DECLARE result text[]; BEGIN result := cat_tools.routine__arg_names(c_temp_proc); + + -- Clean up the temporary function PERFORM _cat_tools.function__drop_temp(c_temp_proc, 'cat_tools.routine__parse_arg_names'); + RETURN result; END $body$ @@ -560,6 +590,7 @@ SELECT __cat_tools.create_function( , $body$ BEGIN RAISE WARNING 'function__arg_types() is deprecated, use routine__parse_arg_types instead'; + RETURN cat_tools.routine__parse_arg_types(arguments); END $body$ @@ -577,6 +608,7 @@ SELECT __cat_tools.create_function( , $body$ BEGIN RAISE WARNING 'function__arg_types_text() is deprecated, use routine__parse_arg_types_text instead'; + RETURN cat_tools.routine__parse_arg_types_text(arguments); END $body$ @@ -639,6 +671,7 @@ SELECT ( WHEN object_type::text LIKE '% column' THEN 'pg_attribute' ELSE CASE object_type + -- Unusual cases WHEN 'default value' THEN 'pg_attrdef' WHEN 'large object' THEN 'pg_largeobject' WHEN 'operator class' THEN 'pg_opclass' @@ -656,7 +689,7 @@ SELECT ( WHEN 'server' THEN 'pg_foreign_server' WHEN 'user mapping' THEN 'pg_user_mapping' WHEN 'default acl' THEN 'pg_default_acl' - WHEN 'event trigger' THEN 'pg_event_trigger' + WHEN 'event trigger' THEN 'pg_event_trigger' -- SED: REQUIRES 9.3! WHEN 'access method' THEN 'pg_am' ELSE 'pg_' || object_type::text END @@ -692,7 +725,7 @@ SELECT __cat_tools.create_function( , $$boolean LANGUAGE sql STRICT STABLE$$ , $body$ SELECT relnamespace::pg_catalog.regnamespace::text ~ '^pg_temp' -FROM pg_catalog.pg_class +FROM pg_catalog.pg_class WHERE oid = $1 $body$ , 'cat_tools__usage' @@ -706,7 +739,7 @@ SELECT __cat_tools.create_function( , $$boolean LANGUAGE sql STRICT STABLE$$ , $body$ SELECT relnamespace::pg_catalog.regnamespace::text = 'pg_catalog' -FROM pg_catalog.pg_class +FROM pg_catalog.pg_class WHERE oid = $1 $body$ , 'cat_tools__usage' @@ -806,18 +839,19 @@ BEGIN END IF; RAISE DEBUG 'v_work "%"', v_work; - -- Get function arguments - -- Use a generic pattern rather than the regproc name, since pg_get_triggerdef - -- may render temp functions as "pg_temp.f" while ::regproc gives "pg_temp_N.f". - v_execute_clause := E' EXECUTE (FUNCTION|PROCEDURE) \\S+\\('; + /* + * Get function arguments. PG11+ uses "EXECUTE FUNCTION"; older versions use + * "EXECUTE PROCEDURE". Note: ::regproc returns the internal pg_temp_N schema + * name while pg_get_triggerdef uses the pg_temp alias, so we match on EXECUTE + * PROCEDURE/FUNCTION + any non-space chars (the function name) rather than the + * specific function name. + */ + v_execute_clause := E' EXECUTE (?:PROCEDURE|FUNCTION) \\S+\\('; v_array := regexp_split_to_array( v_work, v_execute_clause ); - EXECUTE format( - CASE WHEN coalesce( rtrim( v_array[2], ')' ), '' ) = '' - THEN 'SELECT ARRAY[]::text[]' - ELSE 'SELECT array[ %s ]' - END - , rtrim( v_array[2], ')' ) -- Yank trailing ) - ) + EXECUTE CASE + WHEN trim(rtrim(v_array[2], ')')) = '' THEN 'SELECT ARRAY[]::text[]' + ELSE format('SELECT array[ %s ]', rtrim( v_array[2], ')' )) + END INTO function_arguments ; RAISE DEBUG 'v_array[2] "%"', v_array[2]; @@ -868,3 +902,24 @@ DROP FUNCTION __cat_tools.create_function( , comment text ); DROP SCHEMA __cat_tools; + +/* + * Converge the ACL of the five enum types that predate 0.2.2 (constraint_type, + * procedure_type, relation_type, relation_relkind, object_type) to the + * fresh-install state. ALTER DEFAULT PRIVILEGES (near the top of + * sql/cat_tools.sql.in) only applies to objects created AFTER it runs; these + * types were created back in 0.2.0/0.2.1, so the 0.2.0->0.2.2 and 0.2.1->0.2.2 + * update scripts never granted USAGE on them, and neither does 0.2.2->0.2.3 -- + * a 0.2.0/0.2.1-origin database is still missing the grant on reaching 0.2.3. + * Both already-released scripts are immutable, so the fix converges forward + * here instead, on the first still-unpublished update script downstream of the + * gap. A fresh install at any version creates these types AFTER the statement + * and already has the grant, so GRANT's idempotency makes this a no-op there. + */ +GRANT USAGE ON TYPE + cat_tools.constraint_type + , cat_tools.procedure_type + , cat_tools.relation_type + , cat_tools.relation_relkind + , cat_tools.object_type + TO cat_tools__usage;