public inbox for [email protected]
help / color / mirror / Atom feedFrom: Justin Pryzby <[email protected]>
To: Tom Lane <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: [email protected]
Cc: Amit Langote <[email protected]>
Subject: Re: \d with triggers: more than one row returned by a subquery used as an expression
Date: Mon, 17 Jan 2022 18:08:24 -0600
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<[email protected]>
On Mon, Jan 17, 2022 at 05:02:00PM -0500, Tom Lane wrote:
> Justin Pryzby <[email protected]> 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/[email protected]
> >> 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
Attachments:
[text/x-diff] 0001-psql-Add-newlines-and-spacing-in-trigger-query-for-d.patch (1.5K, ../[email protected]/2-0001-psql-Add-newlines-and-spacing-in-trigger-query-for-d.patch)
download | inline diff:
From 99f8ee921258d9b2e75299e69826004fda3b8919 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <[email protected]>
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
[text/x-diff] 0002-psql-fix-d-for-statement-triggers-with-same-name-on-.patch (3.2K, ../[email protected]/3-0002-psql-fix-d-for-statement-triggers-with-same-name-on-.patch)
download | inline diff:
From d866624abbd523afdbc3a539f14c935c42fc345f Mon Sep 17 00:00:00 2001
From: Justin Pryzby <[email protected]>
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.
[email protected]
---
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
view thread (4+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: \d with triggers: more than one row returned by a subquery used as an expression
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox