Re: Some problems regarding the self-join elimination code

On Sun, Apr 27, 2025 at 2:02 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
>
> On Fri, Apr 11, 2025 at 5:46 PM Andrei Lepikhov <lepihov@gmail.com> wrote:
> > On 4/10/25 14:39, Andrei Lepikhov wrote:
> > > On 4/10/25 13:36, Alexander Korotkov wrote:
> > >> On Wed, Apr 9, 2025 at 10:39 AM Andrei Lepikhov <lepihov@gmail.com>
> > >> wrote:
> > >>> It seems we are coming to the conclusion that join removal optimisation
> > >>> may do something out of ChangeVarNodes resposibility. Before further
> > >>> complicating of this function code I would like to know opinion of Tom,
> > >>> who initially proposed [1] to use this routine. May be better a) return
> > >>> to more specialised change_relid / sje_walker machinery or b) move
> > >>> ChangeVarNodes out of rewriteManip and make it multi-purpose routine,
> > >>> allowing to transform expression that may happen after a Var node
> > >>> change?
> > >>
> > >> What about adding a callback to ChangeVarNodes_context that would
> > >> called for each RestrictInfo after changing varnodes itself?  SJE
> > >> could use a callback that replaces OpExpr with NullTest when needed.
> > > I think it is doable, of course. Just looking forward a little, it may
> > > need more complication in the future (SJE definitely should be widened
> > > to partitioned tables) and it may be simpler to have two different
> > > routines for two different stages of planning.
> > To provide some food for thought, here is a draft in attachment which
> > addresses both issues: RestrictInfo relid replacement and move
> > SJE-specific code out of the ChangeVarNodes routine (callback approach).
>
> Thank you, Andrei.  I've put it all together.
> 0001 Fixes material bugs in ChangeVarNodes_walker() including regression test
> 0002 Puts back comments which got accidentally removed
> 0003 Refactors ChangeVarNodesExtended() with custom user-defined callback
>
> I'm going to further work on improvement of these patches.

Sorry, I accidentally sent some messages off-list.  So, now 0001 and
0002 are pushed.

I've revised the remaining refactoring patch.  I've made a callback an
additional callback, but not the replacement to the walker.  It looks
better for me now.  Also, I've written some comments and the commit
message.  Any thoughts?

------
Regards,
Alexander Korotkov
Supabase