On Thu, Jul 4, 2024 at 11:40 AM jian he <jian.universal...@gmail.com> wrote: > On Thu, Jul 4, 2024 at 11:04 AM Alexander Korotkov <aekorot...@gmail.com> > wrote: > > > > On Thu, Jul 4, 2024 at 5:15 AM jian he <jian.universal...@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.
Good. But I think it's not necessary to to replace function signature in all the 42 occurrences. This will make our patch unnecessarily conflict with others. Instead we can have two functions ChangeVarNodes(original function signature) and ChangeVarNodesExtended(extended function signature). Then existing occurrences can still use ChangeVarNodes(), which will be just shortcut for ChangeVarNodesExtended(). ------ Regards, Alexander Korotkov Supabase
