Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Sep 24, 2025 at 6:03 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I think a better idea is to keep a list of just the subplan
>> names that we've assigned so far. That has a far clearer
>> charter, plus it can be updated immediately by choose_plan_name()
>> instead of relying on the caller to do the right thing later.
>> I coded this up, and was rather surprised to find that it changed
>> some regression outputs. On investigation, that's because
>> build_minmax_path() was actually doing the wrong thing later:
>> it was putting the wrong root into allroots, so that "minmax_1"
>> never became assigned and could be re-used later.
> Ooph, that's embarrassing. I think the reason that I ended up making a
> list of the roots themselves rather than the strings is that I was
> thinking that everything in this data structure would need to be a
> node, and I didn't want to cons up String nodes for every list
> element. Then later I marked that structure member read_write_ignore
> and never stopped to think that maybe then we didn't need nodes at
> all. So, in short, I think this is a great solution and thanks a ton
> for putting in the legwork to figure it out.
I think if we do decide later that the list-of-names needs to be
Nodes, then converting them to String nodes is a perfectly fine
solution. As you remark elsewhere, none of this is going to be
more than microscopic compared to the overall cost of planning
a subplan. But for right now, yeah, we don't need it.
Anyway, it seems the only remaining issue is what name
pull_up_simple_subquery should give to its transient root clone.
I don't actually believe that it matters, so if you're content
with re-using the parent name, let's just roll with that until
someone complains.
regards, tom lane
