diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 75ea629..5b622c5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -106,37 +106,41 @@ jobs: strategy: matrix: include: - # old_pg=10: install an old cat_tools on the oldest PostgreSQL, then + # old_pg=10: install an OLD cat_tools on the oldest PostgreSQL, then # BRIDGE-update to 0.2.3 (the current version) on PG10 BEFORE pg_upgrade # -- modelling a user who updates the extension, then binary-upgrades to # a newer major. # - # old_install SHOULD be 0.2.0: proving that a database installed at the - # OLDEST version can update to 0.2.3 and then survive binary pg_upgrade - # is the whole point of this leg. It is TEMPORARILY pinned to a FRESH - # 0.2.2 because a 0.2.0-origin database cannot yet survive the journey -- - # the shipped 0.2.x update scripts have fresh-vs-update divergences that - # break on newer PostgreSQL. Specifically cat_tools.trigger__parse as - # produced by the update scripts hardcodes `EXECUTE PROCEDURE` (PG11+ - # needs an `EXECUTE (?:PROCEDURE|FUNCTION)` match) and omits the - # empty-args guard, so it errors with "cannot determine type of empty - # array" when the suite parses a trigger (surfaces on PG18). (The - # pg_class_v view divergence is already repaired by 0.2.2->0.2.3.) A - # FRESH 0.2.2 has the corrected trigger__parse and already strips - # relhasoids/relhaspkey (fixed `!= ALL` omit_column), so it is both - # pg_upgrade-safe and free of that divergence. - # - # Restore old_install=0.2.0 once the 0.2.2->0.2.3 repair also fixes - # trigger__parse (and any remaining divergence) -- that work is in - # progress. Meanwhile the 0.2.0-only install and its 0.2.2->0.2.3 view - # rebuild stay covered by extension-update-test on PG10. + # old_install is 0.2.0/0.2.1: the whole point of these legs is proving a + # database installed at the OLDEST versions can update to 0.2.3 and then + # survive binary pg_upgrade to a newer major. 0.2.0/0.2.1 install ONLY on + # PG10 (PG11 added pg_attribute.attmissingval, PG12+ exposes oid in + # SELECT *), so these origins can only start here. The bridge to 0.2.3 is + # what makes the objects pg_upgrade-safe: the shipped 0.2.0->0.2.2 / + # 0.2.1->0.2.2 scripts leave fresh-vs-update divergences (relhasoids/ + # relhaspkey never stripped from _cat_tools.pg_class_v via the no-op + # omit_column; a trigger__parse body that hardcodes `EXECUTE PROCEDURE` + # and omits the empty-args guard), and the 0.2.2->0.2.3 update repairs + # both -- the pg_class_v rebuild strips the dropped columns, and the + # trigger__parse CREATE OR REPLACE matches `EXECUTE (?:PROCEDURE|FUNCTION)` + # with the empty-args guard (PG11+ emits EXECUTE FUNCTION). Without the + # bridge the raw 0.2.0 views/functions reference catalog state removed in + # newer PostgreSQL and break after pg_upgrade (trigger__parse surfaces on + # PG18). - old_pg: "10" new_pg: "11" - old_install: "0.2.2" + old_install: "0.2.0" bridge_to: "0.2.3" - old_pg: "10" new_pg: "18" - old_install: "0.2.2" + old_install: "0.2.0" + bridge_to: "0.2.3" + # 0.2.1-origin: same PG10-only bridge journey, exercising the + # 0.2.1->0.2.2 update script (a different script from 0.2.0->0.2.2) + # feeding the same 0.2.2->0.2.3 repair, then binary pg_upgrade to PG18. + - old_pg: "10" + new_pg: "18" + old_install: "0.2.1" bridge_to: "0.2.3" # old_pg>=11: such users are already on 0.2.2+. A FRESH 0.2.2 install # already strips relhasoids/relhaspkey (fixed `!= ALL` omit_column), so @@ -157,7 +161,10 @@ jobs: new_pg: "18" old_install: "0.2.2" bridge_to: "" - name: 🔄 Binary pg_upgrade ${{ matrix.old_pg }} → ${{ matrix.new_pg }} + # Include old_install in the name so matrix legs that differ only by origin + # version (e.g. the two 10 → 18 legs from 0.2.0 and 0.2.1) get distinct, + # unambiguous check names. + name: 🔄 Binary pg_upgrade ${{ matrix.old_pg }} → ${{ matrix.new_pg }} (from ${{ matrix.old_install }}) runs-on: ubuntu-latest container: pgxn/pgxn-tools env: @@ -275,21 +282,26 @@ jobs: # starting at 0.2.1 exercises the 0.2.1--0.2.2 script directly. update-check # asserts each lands on 0.2.2. # - # The third check exercises the REAL broken path end to end: a 0.2.0 - # install on PG10 leaves relhasoids in _cat_tools.pg_class_v (the buggy - # 0.2.0--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.0--0.2.2 then 0.2.2--0.2.3), firing the - # conditional rebuild that strips relhasoids. On PG12+ relhasoids never + # 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. 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 leg. `$$` is escaped + # 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 cat_tools_rebuild_020 0.2.0 0.2.3 - psql -d cat_tools_rebuild_020 -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 \$\$" + 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 + 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+) # update-scenario creates a real database at 0.2.2, plants + proves the # dependency guard, ALTER EXTENSION UPDATEs to the current version, and @@ -337,3 +349,4 @@ jobs: echo "One or more jobs failed or were cancelled" exit 1 fi +# vi: expandtab ts=2 sw=2 diff --git a/sql/cat_tools--0.2.2--0.2.3.sql.in b/sql/cat_tools--0.2.2--0.2.3.sql.in index 4954cff..b8d8ad1 100644 --- a/sql/cat_tools--0.2.2--0.2.3.sql.in +++ b/sql/cat_tools--0.2.2--0.2.3.sql.in @@ -202,6 +202,7 @@ DROP SCHEMA __cat_tools; CREATE OR REPLACE FUNCTION cat_tools.relation__kind( relkind cat_tools.relation_relkind ) RETURNS cat_tools.relation_type LANGUAGE sql STRICT IMMUTABLE AS $body$ +/* relkind values per pg_class.h. */ SELECT CASE relkind WHEN 'r' THEN 'table' WHEN 'i' THEN 'index' @@ -217,6 +218,7 @@ $body$; CREATE OR REPLACE FUNCTION cat_tools.relation__relkind( kind cat_tools.relation_type ) RETURNS cat_tools.relation_relkind LANGUAGE sql STRICT IMMUTABLE AS $body$ +/* Mapping per pg_class.h, same as relation__kind() above. */ SELECT CASE kind WHEN 'table' THEN 'r' WHEN 'index' THEN 'i' @@ -228,3 +230,136 @@ SELECT CASE kind WHEN 'materialized view' THEN 'm' END::cat_tools.relation_relkind $body$; + +/* + * Converge trigger__parse to the fresh-install body. The published 0.2.0->0.2.2 + * / 0.2.1->0.2.2 update scripts create this function with a body that hardcodes + * ' EXECUTE PROCEDURE ' and omits the empty-args guard; a fresh install has the + * corrected body. PG11+ emits EXECUTE FUNCTION, so match either keyword, and + * guard empty args (otherwise "cannot determine type of empty array" when + * parsing a trigger). CREATE OR REPLACE preserves grants and the comment and has + * no dependents to break, so no DROP is needed; it is a no-op on a fresh 0.2.2 + * (already correct) and a repair on the update path. The signature is identical + * to the fresh 0.2.3 definition, and the body below is copied verbatim from + * sql/cat_tools.sql.in. + */ +CREATE OR REPLACE FUNCTION cat_tools.trigger__parse( + trigger_oid oid + , OUT trigger_table regclass + , OUT timing text + , OUT events text[] + , OUT defer text + , OUT row_statement text + , OUT when_clause text + , OUT trigger_function regprocedure + , OUT function_arguments text[] +) RETURNS record STABLE LANGUAGE plpgsql AS $body$ +DECLARE + r_trigger pg_catalog.pg_trigger; + v_triggerdef text; + v_create_stanza text; + v_on_clause text; + v_execute_clause text; + + v_work text; + v_array text[]; +BEGIN + /* + * Do this first to make sure trigger exists. + * + * TODO: After we no longer support < 9.6, test v_triggerdef for NULL instead + * using the extra block here. + */ + BEGIN + SELECT * INTO STRICT r_trigger FROM pg_catalog.pg_trigger WHERE oid = trigger_oid; + EXCEPTION WHEN no_data_found THEN + RAISE EXCEPTION 'trigger with OID % does not exist', trigger_oid + USING errcode = 'undefined_object' -- 42704 + ; + END; + trigger_table := r_trigger.tgrelid; + trigger_function := r_trigger.tgfoid; + + v_triggerdef := pg_catalog.pg_get_triggerdef(trigger_oid, true); + + v_create_stanza := format( + 'CREATE %sTRIGGER %I ' + , CASE WHEN r_trigger.tgconstraint=0 THEN '' ELSE 'CONSTRAINT ' END + , r_trigger.tgname + ); + -- Strip CREATE [CONSTRAINT] TRIGGER ... off + v_work := replace( v_triggerdef, v_create_stanza, '' ); + + -- Get BEFORE | AFTER | INSTEAD OF + timing := split_part( v_work, ' ', 1 ); + timing := timing || CASE timing WHEN 'INSTEAD' THEN ' OF' ELSE '' END; + + -- Strip off timing clause + v_work := replace( v_work, timing || ' ', '' ); + + -- Get array of events (INSERT, UPDATE [OF column, column], DELETE, TRUNCATE) + v_on_clause := ' ON ' || r_trigger.tgrelid::pg_catalog.regclass || ' '; + v_array := regexp_split_to_array( v_work, v_on_clause ); + events := string_to_array( v_array[1], ' OR ' ); + -- Get everything after ON table_name + v_work := v_array[2]; + RAISE DEBUG 'v_work "%"', v_work; + + -- Strip off FROM referenced_table if we have it + IF r_trigger.tgconstrrelid<>0 THEN + v_work := replace( + v_work + , 'FROM ' || r_trigger.tgconstrrelid::pg_catalog.regclass || ' ' + , '' + ); + END IF; + RAISE DEBUG 'v_work "%"', v_work; + + /* + * 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 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]; + -- Get everything prior to EXECUTE PROCEDURE ... + v_work := v_array[1]; + RAISE DEBUG 'v_work "%"', v_work; + + row_statement := (regexp_matches( v_work, 'FOR EACH (ROW|STATEMENT)' ))[1]; + + -- Get [ NOT DEFERRABLE | [ DEFERRABLE ] { INITIALLY IMMEDIATE | INITIALLY DEFERRED } ] + v_array := regexp_split_to_array( v_work, 'FOR EACH (ROW|STATEMENT)' ); + RAISE DEBUG 'v_work = "%", v_array = "%"', v_work, v_array; + defer := rtrim(v_array[1]); + + IF r_trigger.tgqual IS NOT NULL THEN + when_clause := rtrim( + (regexp_split_to_array( v_array[2], E' WHEN \\(' ))[2] + , ')' + ); + END IF; + + RAISE DEBUG +$$v_create_stanza = "%" + v_on_clause = "%" + v_execute_clause = "%"$$ + , v_create_stanza + , v_on_clause + , v_execute_clause + ; + + RETURN; +END +$body$; + +-- vi: expandtab ts=2 sw=2 diff --git a/sql/cat_tools.sql.in b/sql/cat_tools.sql.in index 49b03b6..cc583b7 100644 --- a/sql/cat_tools.sql.in +++ b/sql/cat_tools.sql.in @@ -646,7 +646,7 @@ SELECT __cat_tools.create_function( , 'relkind cat_tools.relation_relkind' , 'cat_tools.relation_type LANGUAGE sql STRICT IMMUTABLE' , $body$ -/* Per pg_class.h: relkind 'c' = composite type, 'f' = foreign table, 'm' = materialized view. */ +/* relkind values per pg_class.h. */ SELECT CASE relkind WHEN 'r' THEN 'table' WHEN 'i' THEN 'index' @@ -1381,11 +1381,13 @@ BEGIN END IF; RAISE DEBUG 'v_work "%"', v_work; - -- 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. + /* + * 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 CASE