Re: Some problems regarding the self-join elimination code

On 4/4/25 09:37, Richard Guo wrote:
> On Wed, Apr 2, 2025 at 10:26 PM Richard Guo <guofenglinux@gmail.com> wrote:
> With more exposure to the changes in ChangeVarNodes(), I have some
> more concerns.  As I understand it, ChangeVarNodes() is designed to
> update the varno fields of Var nodes in the given tree that belong to
> the specific relation; neat and clear.  However, now, within this
> function, we also create new NullTest quals for SJE, which doesn't
> seem like something this function should be handling.  It makes this
> function look a bit like a kludge.
To be precise, inside the function we replace old clause with the 
NullTest, because relid replacement action made it degenerate. It seems 
essential to me and I think it may be ok to add a comment at the top of 
ChangeVarNodes, describing this minor optimisation.

> Additionally, I have a minor suggestion regarding the new boolean
> parameter, "change_RangeTblRef".  AFAIU, by convention, we typically
> use flags to control the behavior of walker functions, like in
> pull_var_clause.  While it's fine to use a boolean parameter here
> given that we currently only have one flag, for the sake of future
> extensibility, I think using flags might be a better choice.
That was exactly why I wasn't happy with replacing the change_relid 
routine with ChangeVarNodes.
But variant with a flag seems non-trivial to implement. Do you have any 
proposal about the code?

-- 
regards, Andrei Lepikhov