Fix psql \d's query for identifying parent triggers.

The original coding (from c33869cc3) failed with "more than one row
returned by a subquery used as an expression" if there were unrelated
triggers of the same tgname on parent partitioned tables.  (That's
possible because statement-level triggers don't get inherited.)  Fix
by applying LIMIT 1 after sorting the candidates by inheritance level.

Also, wrap the subquery in a CASE so that we don't have to execute it at
all when the trigger is visibly non-inherited.  Aside from saving some
cycles, this avoids the need for a confusing and undocumented NULLIF().

While here, tweak the format of the emitted query to look a bit
nicer for "psql -E", and add some explanation of this subquery,
because it badly needs it.

Report and patch by Justin Pryzby (with some editing by me).
Back-patch to v13 where the faulty code came in.

Discussion: https://postgr.es/m/20211217154356.GJ17618@telsasoft.com

diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 48348750ee38363ff4c83e3e9638aceff1d6a9d1..db767d78f03f19dc4fee8afbad3bb82d9e8c5854 100644 (file)
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3250,22 +3250,45 @@ describeOneTableDetails(const char *schemaname,
        printfPQExpBuffer(&buf,
                          "SELECT t.tgname, "
                          "pg_catalog.pg_get_triggerdef(t.oid%s), "
-                         "t.tgenabled, %s, %s\n"
-                         "FROM pg_catalog.pg_trigger t\n"
-                         "WHERE t.tgrelid = '%s' AND ",
+                         "t.tgenabled, %s,\n",
                          (pset.sversion >= 90000 ? ", true" : ""),
                          (pset.sversion >= 90000 ? "t.tgisinternal" :
                           pset.sversion >= 80300 ?
                           "t.tgconstraint <> 0 AS tgisinternal" :
-                          "false AS tgisinternal"),
-                         (pset.sversion >= 130000 ?
-                          "(SELECT (NULLIF(a.relid, t.tgrelid))::pg_catalog.regclass"
-                          " FROM pg_catalog.pg_trigger AS u, "
-                          "      pg_catalog.pg_partition_ancestors(t.tgrelid) AS a"
-                          " WHERE u.tgname = t.tgname AND u.tgrelid = a.relid"
-                          "       AND u.tgparentid = 0) AS parent" :
-                          "NULL AS parent"),
+                          "false AS tgisinternal"));
+
+       /*
+        * Detect whether each trigger is inherited, and if so, get the name
+        * of the topmost table it's inherited from.  We have no easy way to
+        * do that pre-v13, for lack of the tgparentid column.  Even with
+        * tgparentid, a straightforward search for the topmost parent would
+        * require a recursive CTE, which seems unduly expensive.  We cheat a
+        * bit by assuming parent triggers will match by tgname; then, joining
+        * with pg_partition_ancestors() allows the planner to make use of
+        * pg_trigger_tgrelid_tgname_index if it wishes.  We ensure we find
+        * the correct topmost parent by stopping at the first-in-partition-
+        * ancestry-order trigger that has tgparentid = 0.  (There might be
+        * unrelated, non-inherited triggers with the same name further up the
+        * stack, so this is important.)
+        */
+       if (pset.sversion >= 130000)
+           appendPQExpBufferStr(&buf,
+                                "  CASE WHEN t.tgparentid != 0 THEN\n"
+                                "    (SELECT u.tgrelid::pg_catalog.regclass\n"
+                                "     FROM pg_catalog.pg_trigger AS u,\n"
+                                "          pg_catalog.pg_partition_ancestors(t.tgrelid) WITH ORDINALITY AS a(relid, depth)\n"
+                                "     WHERE u.tgname = t.tgname AND u.tgrelid = a.relid\n"
+                                "           AND u.tgparentid = 0\n"
+                                "     ORDER BY a.depth LIMIT 1)\n"
+                                "  END AS parent\n");
+       else
+           appendPQExpBufferStr(&buf, "  NULL AS parent\n");
+
+       appendPQExpBuffer(&buf,
+                         "FROM pg_catalog.pg_trigger t\n"
+                         "WHERE t.tgrelid = '%s' AND ",
                          oid);
+
        if (pset.sversion >= 110000)
            appendPQExpBufferStr(&buf, "(NOT t.tgisinternal OR (t.tgisinternal AND t.tgenabled = 'D') \n"
                                 "    OR EXISTS (SELECT 1 FROM pg_catalog.pg_depend WHERE objid = t.oid \n"

diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out
index 5254447cf8ebefb1e0f54c210ff0568d7e8445df..1d9a25f22df8eb08e37b79704ba65bf4bad56d92 100644 (file)
--- a/src/test/regress/expected/triggers.out
+++ b/src/test/regress/expected/triggers.out
@@ -2122,6 +2122,20 @@ Triggers:
 alter table trigpart attach partition trigpart3 FOR VALUES FROM (2000) to (3000); -- fail
 ERROR:  trigger "trg1" for relation "trigpart3" already exists
 drop table trigpart3;
+-- check display of unrelated triggers
+create trigger samename after delete on trigpart execute function trigger_nothing();
+create trigger samename after delete on trigpart1 execute function trigger_nothing();
+\d trigpart1
+             Table "public.trigpart1"
+ Column |  Type   | Collation | Nullable | Default 
+--------+---------+-----------+----------+---------
+ a      | integer |           |          | 
+ b      | integer |           |          | 
+Partition of: trigpart FOR VALUES FROM (0) TO (1000)
+Triggers:
+    samename AFTER DELETE ON trigpart1 FOR EACH STATEMENT EXECUTE FUNCTION trigger_nothing()
+    trg1 AFTER INSERT ON trigpart1 FOR EACH ROW EXECUTE FUNCTION trigger_nothing(), ON TABLE trigpart
+
 drop table trigpart;
 drop function trigger_nothing();
 --

diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql
index 7b73ee20a1b0de8ea614938d510a0c3b3d10921a..2f4516e04427dc1532a47fb656780098246698cd 100644 (file)
--- a/src/test/regress/sql/triggers.sql
+++ b/src/test/regress/sql/triggers.sql
@@ -1428,6 +1428,11 @@ create trigger trg1 after insert on trigpart3 for each row execute procedure tri
 alter table trigpart attach partition trigpart3 FOR VALUES FROM (2000) to (3000); -- fail
 drop table trigpart3;
 
+-- check display of unrelated triggers
+create trigger samename after delete on trigpart execute function trigger_nothing();
+create trigger samename after delete on trigpart1 execute function trigger_nothing();
+\d trigpart1
+
 drop table trigpart;
 drop function trigger_nothing();