Re: plan shape work

On Tue, Sep 9, 2025 at 4:37 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Maybe. I kinda feel that actually redesigning plan trees is outside
> the scope of this project, but maybe we should consider it.

Yeah, I would rather not go there, all things being equal. We could
also consider displaying things differently even if the underlying
node type is the same. ExplainNode already has examples where
pname/sname are set to something other than the node tag, e.g. we set
EXPLAIN designation based on (((ModifyTable *) plan)->operation) or
agg->aggstrategy. I'm not sure if this is the right way to go, but
it's worth a thought.

> I think though that you might be underestimating the amount of
> commonality. For instance, we might need a projecting node on top of
> a subquery-in-FROM subplan, but that node would still have to bear
> a relid --- the relid of the RTE_SUBQUERY RTE in the upper query,
> not that of any RTE in the subquery, but nonetheless it's a relid.
>
> Another variant of this is that that RTE_SUBQUERY relid would normally
> be borne by a SubqueryScan plan node, but if we elide the SubqueryScan
> because it isn't doing anything useful, where shall we put that relid?
> If we don't store it anywhere then we will not be able to reconstruct
> correct join relids for the upper plan level.

I don't quite understand how these two scenarios are different, but I
have found it critical to distinguish problems that happen at setrefs
time from problems that happen during main planning. If we're talking
about feeding information from one planning cycle forward to the next,
we need be able to look at the plan tree and understand what the
pre-setrefs state of affairs was, because when the next planning cycle
happens, the decisions we want to influence are happening pre-setrefs.
The later patches in this patch series deal with exactly this problem:
0004 and 0005 make it possible to match up an RTI from the flattened
range table that pops out of one planning cycle with a specific
subroot and RTI relative to that subroot during the following cycle;
and 0006 and 0007 arrange to leave a breadcrumb trail in cases where
setrefs-time processing deletes plan nodes. The setrefs-time elision
of SubqueryScan nodes is recorded by the mechanism in 0006.

I went back and studied 0001 some more today in reference to your
comments about classifying Result nodes. 0001 already loosely
classifies Result nodes as either "gating" result nodes (that have a
subplan) or "simple" result notes (that don't). "Gating" result notes
happen for target-list projection and/or to apply a one-time filter. I
don't think the reasons for gating nodes need to be recorded anywhere;
either the tlist matches the underlying node or it doesn't, and either
resconstantqual contains something or not. "Simple" result have more
interesting reasons for existing:

1. MinMaxAgg placeholders
2. Degenerate grouping
3. No-FROM-clause cases (these go through create_group_result_plan
like the previous case, but are arguably distinct)
4. Relations proven empty

There's a sort of hybrid case when we want a gating result node but
the underlying node is a simple result. In that case, the patch builds
a new simple result that is similar to the existing one but with the
gating result's target list and one-time filter. It seems OK to me to
forget all about the gating result node and its reason for existence
in this case and just consider ourselves to have updated the
underlying Result node. Otherwise, you'd have to consider that a
Result node might have multiple reasons for existence: whatever caused
the "simple" Result note to get created, plus possibly projection or
one-time filtering.

Now, looking at (1)-(4) above, (3) is actually a special case of (4):

robert.haas=# explain (range_table) select 1;
QUERY PLAN
------------------------------------------
Result (cost=0.00..0.01 rows=1 width=4)
RTIs: 1
RTI 1 (result):
Eref: "*RESULT*" ()
(4 rows)

So the reason why the patch set feels justified in printing "Replaces:
Aggregate" when there are no relids is because we must have case (1)
or (2) from the above list. But it does seem fragile. Not only can we
confuse (1) and (2), but also, only the top-level grouping rel
necessarily has empty relids. We already have child grouping rels that
have relid sets, and I suspect Richard's pending work on eager
aggregation will introduce more of them. This patch won't be able to
distinguish those from case (4).

So maybe what we want for Result reasons is something like
RESULT_REPLACES_BASEREL, RESULT_REPLACES_JOINREL,
RESULT_REPLACES_GROUPING_REL, RESULT_IMPLEMENTS_MINMAX_AGGREGATE?
That's a bit verbose; shorter alternatives welcome. The first two
could be merged, since the cardinality of the relid set should
distinguish them. Or it could be more like RESULT_REPLACES_SCAN,
RESULT_REPLACES_JOIN, RESULT_REPLACES_AGGREGATE,
RESULT_IMPLEMENTS_MINMAX_AGGREGATE, to more closely match what we
would presumably show in the EXPLAIN output.

Thoughts?

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