Richard Guo <guofenglinux@gmail.com> writes:
>> This feels more like a band-aid though. I think the more principled
>> fix would be a PHV cache in add_nullingrels_if_needed, so that we
>> don't generate identical PHVs with different IDs, like what rv_cache
>> does in pullup_replace_vars_callback. But since
>> flatten_join_alias_vars is called many times per query level, the
>> cache would have to live in PlannerInfo, which seems too invasive to
>> back-patch.
I agree with that sounding more principled, but I wonder if we should
think bigger than just tweaking add_nullingrels_if_needed: if we're
desirous of de-duplicating PHVs, why not do that across the board,
for every place that makes PHVs? So we'd mechanize this in
make_placeholder_expr's assignment of phid rather than somewhere else.
> The problem with it is that it breaks ABI and can't be back-patched.
> Given that there have been no field reports since v16, maybe we can
> fix it on master only?
Maybe. In the past, when we needed a new struct field, we've sometimes
decided that adding it at the end of the struct in the back branches
would be adequately ABI-compatible. I think we might be able to get
away with that here too, although I'm a bit worried whether any
extensions might be doing makeNode(PlannerInfo).
regards, tom lane
