Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 9007d4e

Browse files
committed
Fix psql \d's query for identifying parent triggers.
The original coding (from c33869c) 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
1 parent 839f963 commit 9007d4e

File tree

3 files changed

+49
-8
lines changed

3 files changed

+49
-8
lines changed

src/bin/psql/describe.c

+30-8
Original file line numberDiff line numberDiff line change
@@ -2990,16 +2990,38 @@ describeOneTableDetails(const char *schemaname,
29902990
printfPQExpBuffer(&buf,
29912991
"SELECT t.tgname, "
29922992
"pg_catalog.pg_get_triggerdef(t.oid, true), "
2993-
"t.tgenabled, t.tgisinternal, %s\n"
2993+
"t.tgenabled, t.tgisinternal,\n");
2994+
2995+
/*
2996+
* Detect whether each trigger is inherited, and if so, get the name
2997+
* of the topmost table it's inherited from. We have no easy way to
2998+
* do that pre-v13, for lack of the tgparentid column. Even with
2999+
* tgparentid, a straightforward search for the topmost parent would
3000+
* require a recursive CTE, which seems unduly expensive. We cheat a
3001+
* bit by assuming parent triggers will match by tgname; then, joining
3002+
* with pg_partition_ancestors() allows the planner to make use of
3003+
* pg_trigger_tgrelid_tgname_index if it wishes. We ensure we find
3004+
* the correct topmost parent by stopping at the first-in-partition-
3005+
* ancestry-order trigger that has tgparentid = 0. (There might be
3006+
* unrelated, non-inherited triggers with the same name further up the
3007+
* stack, so this is important.)
3008+
*/
3009+
if (pset.sversion >= 130000)
3010+
appendPQExpBufferStr(&buf,
3011+
" CASE WHEN t.tgparentid != 0 THEN\n"
3012+
" (SELECT u.tgrelid::pg_catalog.regclass\n"
3013+
" FROM pg_catalog.pg_trigger AS u,\n"
3014+
" pg_catalog.pg_partition_ancestors(t.tgrelid) WITH ORDINALITY AS a(relid, depth)\n"
3015+
" WHERE u.tgname = t.tgname AND u.tgrelid = a.relid\n"
3016+
" AND u.tgparentid = 0\n"
3017+
" ORDER BY a.depth LIMIT 1)\n"
3018+
" END AS parent\n");
3019+
else
3020+
appendPQExpBufferStr(&buf, " NULL AS parent\n");
3021+
3022+
appendPQExpBuffer(&buf,
29943023
"FROM pg_catalog.pg_trigger t\n"
29953024
"WHERE t.tgrelid = '%s' AND ",
2996-
(pset.sversion >= 130000 ?
2997-
"(SELECT (NULLIF(a.relid, t.tgrelid))::pg_catalog.regclass"
2998-
" FROM pg_catalog.pg_trigger AS u, "
2999-
" pg_catalog.pg_partition_ancestors(t.tgrelid) AS a"
3000-
" WHERE u.tgname = t.tgname AND u.tgrelid = a.relid"
3001-
" AND u.tgparentid = 0) AS parent" :
3002-
"NULL AS parent"),
30033025
oid);
30043026

30053027
/*

src/test/regress/expected/triggers.out

+14
Original file line numberDiff line numberDiff line change
@@ -2122,6 +2122,20 @@ Triggers:
21222122
alter table trigpart attach partition trigpart3 FOR VALUES FROM (2000) to (3000); -- fail
21232123
ERROR: trigger "trg1" for relation "trigpart3" already exists
21242124
drop table trigpart3;
2125+
-- check display of unrelated triggers
2126+
create trigger samename after delete on trigpart execute function trigger_nothing();
2127+
create trigger samename after delete on trigpart1 execute function trigger_nothing();
2128+
\d trigpart1
2129+
Table "public.trigpart1"
2130+
Column | Type | Collation | Nullable | Default
2131+
--------+---------+-----------+----------+---------
2132+
a | integer | | |
2133+
b | integer | | |
2134+
Partition of: trigpart FOR VALUES FROM (0) TO (1000)
2135+
Triggers:
2136+
samename AFTER DELETE ON trigpart1 FOR EACH STATEMENT EXECUTE FUNCTION trigger_nothing()
2137+
trg1 AFTER INSERT ON trigpart1 FOR EACH ROW EXECUTE FUNCTION trigger_nothing(), ON TABLE trigpart
2138+
21252139
drop table trigpart;
21262140
drop function trigger_nothing();
21272141
--

src/test/regress/sql/triggers.sql

+5
Original file line numberDiff line numberDiff line change
@@ -1428,6 +1428,11 @@ create trigger trg1 after insert on trigpart3 for each row execute procedure tri
14281428
alter table trigpart attach partition trigpart3 FOR VALUES FROM (2000) to (3000); -- fail
14291429
drop table trigpart3;
14301430

1431+
-- check display of unrelated triggers
1432+
create trigger samename after delete on trigpart execute function trigger_nothing();
1433+
create trigger samename after delete on trigpart1 execute function trigger_nothing();
1434+
\d trigpart1
1435+
14311436
drop table trigpart;
14321437
drop function trigger_nothing();
14331438

0 commit comments

Comments
 (0)