On Wed, Sep 24, 2025 at 8:27 AM Robert Haas <robertmhaas@gmail.com> wrote:
> Cool. Let's focus on 0004 then, and possibly 0005 since it's somewhat
> related and you seem to have an idea that there could be a better way
> of solving that problem. That's not necessarily to say that 0005 would
> get committed this CF, unless we happen to agree vigorously on
> something, but if there's a way to work around needing 0005 or if it
> needs to be redone in some other form, it would be good to have some
> idea around that sooner rather than later.
Here's a new patch set. 0004 is now 0001 and similarly all other patch
numbers are -3, since the old 0001 and 0002 were committed together
and the 0003 is abandoned. I made the following changes to
old-0004/new-0001:
- I rewrote the commit message. I'm not really sure this is any
clearer about the motivation for this patch, but I tried. Suggestions
appreciated.
- CI was complaining about a warning from sublinktype_to_string, which
I've tried to suppress by adding a dummy return to the end of the
function.
- You (Tom) complained about the lack of const on
sublinktype_to_string, so this version has been const-ified. The const
bled into the arguments to choose_plan_name() and subquery_planner(),
and into the plan_name structure members within PlannerInfo and
SubPlan. I don't know if this is the right thing to do, so feel free
to set me straight.
- You (Tom) also asked why not print InitPlan/SubPlan wherever we
refer to subplans, so this version restores that behavior. I did that
by putting logic to print the InitPlan or SubPlan prefix in
ruleutils.c, while not including InitPlan or SubPlan in the SubPlan's
plan_name field any more. The reason for this is that the purpose of
the patch set is to assign names before planning, and the decision as
to whether something is an InitPlan or a SubPlan is made after
planning, so keeping "InitPlan" or "SubPlan" in the actual plan name
undermines the whole point of the patch. I would argue that this is
actually better on philosophical grounds, because I think that the
fact that something is an expression rather than an EXISTS clause or
whatever is part of the identify of the resulting object, whereas I
think whether something is an InitPlan or a SubPlan is mostly
interesting as a performance characteristic rather than as a definer
of identity. I don't necessarily expect this position to be accepted
without debate, but I prefer the EXPLAIN output with the patch to the
pre-patch output.
The remaining patches are simply rebased and are only included here in
case we want to discuss how they could be
better/different/unnecessary, especially what's now 0002, rather than
because I'm looking to get something committed right away.
Thanks,
--
Robert Haas
EDB: http://www.enterprisedb.com
