On 29/10/2023 22:56, Tom Lane wrote:
> Alexander Korotkov <aekorotkov@gmail.com> writes:
>> I made some beautification of the patch by Andrei. I also removed the
>> part which changes the target list for estimate_num_groups(). Any
>> objections to pushing this?
>
> It seems moderately likely that this will break as much as it fixes.
>
> I've not studied the original patch enough to understand why you need
> to be playing strange games with tree mutation rules, but I suspect
> that this is band-aiding over some rather fundamentally bad code.
Thank you for the feedback.
I need to confess that it is not clear why keeping two links on the same
query in a consistent state is a bad idea. If it really is, it would be
better for future development to explain the reason as a comment in the
code (because the current problematic procedure was introduced only in [1]).
The doubt I had had about SJE was the cleaning procedure. We are afraid
of hidden corner cases when deleting relid can be placed elsewhere, or a
new feature could add more places for this relid. But as an opposite
reason, we have a better understanding of the planner, a kind of
self-check of hidden potential bugs. And for me personally, the latter
is a strong reason. Also, as I see, in the e9a20e4 you wrote the code
keeping in mind the same idea.
[1] Make Vars be outer-join-aware
--
regards,
Andrei Lepikhov
Postgres Professional