Обсуждение: \d with triggers: more than one row returned by a subquery used as an expression

I want to mention that the 2nd problem I mentioned here is still broken.
https://www.postgresql.org/message-id/20210717010259.GU20208@telsasoft.com

It happens if non-inheritted triggers on child and parent have the same name.

On Fri, Jul 16, 2021 at 08:02:59PM -0500, Justin Pryzby wrote:
> On Fri, Jul 16, 2021 at 06:01:12PM -0400, Alvaro Herrera wrote:
> > On 2021-Jul-16, Justin Pryzby wrote:
> > > CREATE TABLE p(i int) PARTITION BY RANGE(i);
> > > CREATE TABLE p1 PARTITION OF p FOR VALUES FROM (1)TO(2);
> > > CREATE FUNCTION foo() returns trigger LANGUAGE plpgsql AS $$begin end$$;
> > > CREATE TRIGGER x AFTER DELETE ON p1 EXECUTE FUNCTION foo();
> > > CREATE TRIGGER x AFTER DELETE ON p EXECUTE FUNCTION foo();
> > 
> > Hmm, interesting -- those statement triggers are not cloned, so what is
> > going on here is just that the psql query to show them is tripping on
> > its shoelaces ... I'll try to find a fix.
> > 
> > I *think* the problem is that the query matches triggers by name and
> > parent/child relationship; we're missing to ignore triggers by tgtype.
> > It's not great design that tgtype is a bitmask of unrelated flags ...
> 
> I see it's the subquery Amit wrote and proposed here:
> https://www.postgresql.org/message-id/CA+HiwqEiMe0tCOoPOwjQrdH5fxnZccMR7oeW=f9FmgszJQbgFg@mail.gmail.com
> 
> .. and I realize that I've accidentally succeeded in breaking what I first
> attempted to break 15 months ago:
> 
> On Mon, Apr 20, 2020 at 02:57:40PM -0500, Justin Pryzby wrote:
> > I'm happy to see that this doesn't require a recursive cte, at least.
> > I was trying to think how to break it by returning multiple results or results
> > out of order, but I think that can't happen.
> 
> If you assume that pg_partition_ancestors returns its results in order, I think
> you can fix it by adding LIMIT 1.  Otherwise I think you need a recursive CTE,
> as I'd feared.
> 
> Note also that I'd sent a patch to add newlines, to make psql -E look pretty.
> v6-0001-fixups-c33869cc3bfc42bce822251f2fa1a2a346f86cc5.patch 

Justin Pryzby <pryzby@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.GU20208@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.  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?

            regards, tom lane

On Mon, Jan 17, 2022 at 05:02:00PM -0500, Tom Lane wrote:
> Justin Pryzby <pryzby@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.GU20208@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

Justin Pryzby <pryzby@telsasoft.com> writes:
> On Mon, Jan 17, 2022 at 05:02:00PM -0500, Tom Lane wrote:
>> 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().

Uh, what?  tgparentid is a trigger OID, not a relation OID.

> Is there any reason why WITH ORDINALITY can't work ?
> This is passing the smoke test.

How hard did you try to break it?  It still seems to me that
this can be fooled by an unrelated trigger with the same tgname.

            regards, tom lane

I wrote:
> Justin Pryzby <pryzby@telsasoft.com> writes:
>> Is there any reason why WITH ORDINALITY can't work ?
>> This is passing the smoke test.

> How hard did you try to break it?  It still seems to me that
> this can be fooled by an unrelated trigger with the same tgname.

Hmm ... no, it does work, because we'll stop at the first trigger
with tgparentid = 0, so unrelated triggers further up the partition stack
don't matter.  But this definitely requires commentary.  (And I'm
not too happy with burying such a complicated thing inside a conditional
inside a printf, either.)  Will see about cleaning it up.

            regards, tom lane