Thread: pgsql: Refactor ChangeVarNodesExtended() using the custom callback

Refactor ChangeVarNodesExtended() using the custom callback

fc069a3a6319 implemented Self-Join Elimination (SJE) and put related logic
to ChangeVarNodes_walker().  This commit provides refactoring to remove the
SJE-related logic from ChangeVarNodes_walker() but adds a custom callback to
ChangeVarNodesExtended(), which has a chance to process a node before
ChangeVarNodes_walker().  Passing this callback to ChangeVarNodesExtended()
allows SJE-related node handling to be kept within the analyzejoins.c.

Reported-by: Richard Guo <guofenglinux@gmail.com>
Discussion: https://postgr.es/m/CAMbWs49PE3CvnV8vrQ0Dr%3DHqgZZmX0tdNbzVNJxqc8yg-8kDQQ%40mail.gmail.com
Author: Andrei Lepikhov <lepihov@gmail.com>
Author: Alexander Korotkov <aekorotkov@gmail.com>

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/ab42d643c14509cf1345588f55d798284b11a91e

Modified Files
--------------
src/backend/optimizer/plan/analyzejoins.c | 160 +++++++++++++++++++++++++++---
src/backend/rewrite/rewriteManip.c        | 138 ++++++--------------------
src/include/rewrite/rewriteManip.h        |  17 +++-
src/tools/pgindent/typedefs.list          |   1 +
4 files changed, 192 insertions(+), 124 deletions(-)

Alexander,

> Refactor ChangeVarNodesExtended() using the custom callback
>
> fc069a3a6319 implemented Self-Join Elimination (SJE) and put related logic
> to ChangeVarNodes_walker().  This commit provides refactoring to remove the
> SJE-related logic from ChangeVarNodes_walker() but adds a custom callback to
> ChangeVarNodesExtended(), which has a chance to process a node before
> ChangeVarNodes_walker().  Passing this callback to ChangeVarNodesExtended()
> allows SJE-related node handling to be kept within the analyzejoins.c.

I believe this was pushed by mistake.

-- 
Best regards,
Aleksander Alekseev

On Wed, May 7, 2025 at 11:47 AM Aleksander Alekseev
<aleksander@timescale.com> wrote:
> > Refactor ChangeVarNodesExtended() using the custom callback
> >
> > fc069a3a6319 implemented Self-Join Elimination (SJE) and put related logic
> > to ChangeVarNodes_walker().  This commit provides refactoring to remove the
> > SJE-related logic from ChangeVarNodes_walker() but adds a custom callback to
> > ChangeVarNodesExtended(), which has a chance to process a node before
> > ChangeVarNodes_walker().  Passing this callback to ChangeVarNodesExtended()
> > allows SJE-related node handling to be kept within the analyzejoins.c.
>
> I believe this was pushed by mistake.

Why it should be mistake this time?
At least, this time I managed to wait till the end of release freeze.

------
Regards,
Alexander Korotkov
Supabase

Hi,

> > I believe this was pushed by mistake.
>
> Why it should be mistake this time?
> At least, this time I managed to wait till the end of release freeze.

Perhaps I misunderstand something in the process. How one will tag
REL_18_BETA2 or fork REL_18_STABLE considering that the `master`
branch contains this commit or others?

-- 
Best regards,
Aleksander Alekseev

Hi,

> OK, now I get your point: you think this should go to the next major
> release.  But no, it's not.  It's intended to eventually land into
> REL_18_STABLE, REL_18_BETA2 etc.  It addresses one of PG18 Open Items:
> Richard Guo found the way SJE changes ChangeVarNodes() unsatisfiable,
> and this commit is intended to fix that.  Also, note that it is not
> the only PG18 Open Item left after beta1, and fixing Open Items
> usually requires changes besides docs and whitespaces.

Now I get it. From the commit message it looked like a regular
refactoring. I didn't realize it fixes an open item.

Many thanks!

-- 
Best regards,
Aleksander Alekseev