Converge 0.2.3 update path to fresh install and test all update origins

.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 <origin>--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

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

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