Re: Removing unneeded self joins

On Thu, Jul 4, 2024 at 11:04 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:
>
> On Thu, Jul 4, 2024 at 5:15 AM jian he <jian.universality@gmail.com> wrote:
> > in remove_self_join_rel, i have
> > ```ChangeVarNodes((Node *) root->parse, toRemove->relid, toKeep->relid, 0);```
> > which will change the joinlist(RangeTblRef)  from (1,2)  to (2,2).
> > Immediately after this call, I wrote a function (restore_rangetblref)
> > to restore the joinlist as original (1,2).
> > then remove_rel_from_joinlist won't error out.
> > see remove_self_join_rel, restore_rangetblref.
>
> Thank you, now this is clear.  Could we add additional parameters to
> ChangeVarNodes() instead of adding a new function which reverts part
> of changes.
>

I didn't dare to. we have 42 occurrences of ChangeVarNodes.
adding a parameter to it only for one location seems not intuitive.

Now I have tried.
changing to
`ChangeVarNodes(Node *node, int rt_index, int new_index, int
sublevels_up, bool change_RangeTblRef)`

/* Replace varno in all the query structures */
ChangeVarNodes((Node *) root->parse, toRemove->relid, toKeep->relid, 0, false);
```

it seems to work, pass the regression test.
```ChangeVarNodes((Node *) root->parse, toRemove->relid,
toKeep->relid, 0, false);```
is in remove_self_join_rel, remove_self_joins_one_group,
remove_self_joins_recurse.
all other places are ```ChangeVarNodes((Node *) root->parse,
toRemove->relid, toKeep->relid, 0, true);```
so ChangeVarNodes add a parameter will only influence the SJE feature.


I also tried, save to a temp list, but it did not work.
original_fromlist = list_copy_deep(root->parse->jointree->fromlist);
/* Replace varno in all the query structures */
ChangeVarNodes((Node *) root->parse, toRemove->relid, toKeep->relid, 0);
root->parse->jointree->fromlist = list_copy(original_fromlist);