On Mon, Jan 17, 2022 at 05:02:00PM -0500, Tom Lane wrote: > Justin Pryzby <pry...@telsasoft.com> writes: > > On Fri, Dec 17, 2021 at 09:43:56AM -0600, Justin Pryzby wrote: > >> I want to mention that the 2nd problem I mentioned here is still broken. > >> https://www.postgresql.org/message-id/20210717010259.gu20...@telsasoft.com > >> It happens if non-inheritted triggers on child and parent have the same > >> name. > > > This is the fix I was proposing > > It depends on pg_partition_ancestors() to return its partitions in order: > > this partition => parent => ... => root. > > I don't think that works at all. I might be willing to accept the > assumption about pg_partition_ancestors()'s behavior, but you're also > making an assumption about how the output of pg_partition_ancestors() > is joined to "pg_trigger AS u", and I really don't think that's safe.
> ISTM the real problem is the assumption that only related triggers could > share a tgname, which evidently isn't true. I think this query needs to > actually match on tgparentid, rather than taking shortcuts. I don't think that should be needed - tgparentid should match pg_partition_ancestors(). > If we don't > want to use a recursive CTE, maybe we could define it as only looking up to > the immediate parent, rather than necessarily finding the root? I think that defeats the intent of c33869cc3. Is there any reason why WITH ORDINALITY can't work ? This is passing the smoke test. -- Justin
>From 99f8ee921258d9b2e75299e69826004fda3b8919 Mon Sep 17 00:00:00 2001 From: Justin Pryzby <pryz...@telsasoft.com> Date: Fri, 3 Apr 2020 22:43:26 -0500 Subject: [PATCH 1/2] psql: Add newlines and spacing in trigger query for \d cosmetic change to c33869cc3 to make the output of psql -E look pretty. --- src/bin/psql/describe.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c index 61cec118aca..ed9f320b015 100644 --- a/src/bin/psql/describe.c +++ b/src/bin/psql/describe.c @@ -2993,12 +2993,12 @@ describeOneTableDetails(const char *schemaname, "t.tgenabled, t.tgisinternal, %s\n" "FROM pg_catalog.pg_trigger t\n" "WHERE t.tgrelid = '%s' AND ", - (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" : + (pset.sversion >= 130000 ? "\n" + " (SELECT (NULLIF(a.relid, t.tgrelid))::pg_catalog.regclass\n" + " FROM pg_catalog.pg_trigger AS u,\n" + " pg_catalog.pg_partition_ancestors(t.tgrelid) AS a\n" + " WHERE u.tgname = t.tgname AND u.tgrelid = a.relid\n" + " AND u.tgparentid = 0) AS parent" : "NULL AS parent"), oid); -- 2.17.1
>From d866624abbd523afdbc3a539f14c935c42fc345f Mon Sep 17 00:00:00 2001 From: Justin Pryzby <pryz...@telsasoft.com> Date: Fri, 16 Jul 2021 19:57:00 -0500 Subject: [PATCH 2/2] psql: fix \d for statement triggers with same name on partition and its parent This depends on pg_partition_ancestors() to return its partitions in order: this partition => parent => ... => root. 20210716193319.gs20...@telsasoft.com --- src/bin/psql/describe.c | 4 ++-- src/test/regress/expected/triggers.out | 13 +++++++++++++ src/test/regress/sql/triggers.sql | 4 ++++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c index ed9f320b015..8787849e8be 100644 --- a/src/bin/psql/describe.c +++ b/src/bin/psql/describe.c @@ -2996,9 +2996,9 @@ describeOneTableDetails(const char *schemaname, (pset.sversion >= 130000 ? "\n" " (SELECT (NULLIF(a.relid, t.tgrelid))::pg_catalog.regclass\n" " FROM pg_catalog.pg_trigger AS u,\n" - " pg_catalog.pg_partition_ancestors(t.tgrelid) AS a\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) AS parent" : + " AND u.tgparentid = 0 ORDER BY depth LIMIT 1) AS parent" : "NULL AS parent"), oid); diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out index 5c0e7c2b79e..9278cc07237 100644 --- a/src/test/regress/expected/triggers.out +++ b/src/test/regress/expected/triggers.out @@ -2122,6 +2122,19 @@ 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; +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 9cb15c21dc3..5e2197e10e2 100644 --- a/src/test/regress/sql/triggers.sql +++ b/src/test/regress/sql/triggers.sql @@ -1428,6 +1428,10 @@ 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; +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(); -- 2.17.1
