Re: plan shape work

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Richard Guo <guofenglinux(at)gmail(dot)com>, Alexandra Wang <alexandra(dot)wang(dot)oss(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "bruce(at)momjian(dot)us" <bruce(at)momjian(dot)us>, lepihov(at)gmail(dot)com
Subject: Re: plan shape work
Date: 2025-09-15 21:07:47
Message-ID: CA+TgmoY6Pf9wyP13ZZjNyFz+xCeqfkzRe7G8LY5V0uFhFuLT0Q@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Sep 15, 2025 at 3:43 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > Yes, I wondered if it should actually look more like this:
> > Degenerate Scan on blah
> > Degenerate Join on blah, blah, blah
> > Degenerate Aggregate
> > MinMaxAggregate Result
> > So getting rid of Replaces: altogether.
>
> Yeah, that would be pretty tempting if we were working in a green
> field. I think it might be too much change though. Also, from a
> developer's standpoint it's better if what EXPLAIN prints agrees
> with what the node types are internally.

Well, I was just bringing it up because you seemed to think that the
Replaces: line was not necessarily where we would have put it in a
green field. I am not sure about where we would have put it, but I
think in a green field we wouldn't have it at all, and I actually
think the above is worth considering. I think people would get used to
it pretty fast and that it might be more clear than "Result", which
doesn't really mean a whole lot. However, if you or others don't like
that and you want to just move the Replaces line slightly higher in
the output, I mean, I don't mind that, I just don't know that it's
really a material difference.

> I do suggest adding the above as a regression test.

Makes sense.

> Uh ... this example does not involve an RTE_RESULT does it?
> I see a regular RTE_RELATION RTE for pgbench_accounts, which
> your code duly prints.
>
> Looking at the code, there are just three places (all in
> prepjointree.c) that create RTE_RESULT RTEs. Two of them
> are building the RTE from scratch, and they both do
> rte->eref = makeAlias("*RESULT*", NIL);
> However the third one (pull_up_constant_function) is changing
> a pulled-up RTE_FUNCTION into RTE_RESULT, and it doesn't do
> anything to the RTE's eref. While that makes your argument
> nominally true, I'd be inclined to argue that this was an oversight
> and it should have changed the alias/eref fields to look like other
> RTE_RESULTs. (I've not investigated, but I wonder what your
> patch prints for such cases.)

Will investigate.

> + * relids identifies the relation for which this Result node is generating the
> + * tuples. When subplan is not NULL, it should be empty: this node is not
> + * generating anything in that case, just acting on tuples generated by the
> + * subplan. Otherwise, it contains the relids of the planner relation that
> + * the Result represents.

OK.

--
Robert Haas
EDB: http://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2025-09-15 21:12:38 Re: Identifying function-lookup failures due to argument name mismatches
Previous Message Robert Haas 2025-09-15 21:02:43 Re: Identifying function-lookup failures due to argument name mismatches