pg.ddx.io pgsql-committers@postgresql.org mailing list archive
help / color / mirror / Atom feedpgsql: Refactor ChangeVarNodesExtended() using the custom callback
13+ messages / 4 participants
[nested] [flat]
Commits from this thread:
2025-05-03 2782f3b845 Revert "Refactor ChangeVarNodesExtended() using the custom callback"
* pgsql: Refactor ChangeVarNodesExtended() using the custom callback
@ 2025-05-03 19:32 Alexander Korotkov <akorotkov@postgresql.org>
0 siblings, 1 reply; 13+ messages in thread
From: Alexander Korotkov @ 2025-05-03 19:32 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
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/250a718aadad68793e82103282247556a46a3cfc
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(-)
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: pgsql: Refactor ChangeVarNodesExtended() using the custom callback
@ 2025-05-03 19:34 Tom Lane <tgl@sss.pgh.pa.us>
parent: Alexander Korotkov <akorotkov@postgresql.org>
0 siblings, 1 reply; 13+ messages in thread
From: Tom Lane @ 2025-05-03 19:34 UTC (permalink / raw)
To: Alexander Korotkov <akorotkov@postgresql.org>; +Cc: pgsql-committers@lists.postgresql.org
Alexander Korotkov <akorotkov@postgresql.org> writes:
> Refactor ChangeVarNodesExtended() using the custom callback
You are aware that we are in release freeze, no?
regards, tom lane
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: pgsql: Refactor ChangeVarNodesExtended() using the custom callback
@ 2025-05-03 19:41 Alexander Korotkov <aekorotkov@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 13+ messages in thread
From: Alexander Korotkov @ 2025-05-03 19:41 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Alexander Korotkov <akorotkov@postgresql.org>; pgsql-committers@lists.postgresql.org
On Sat, May 3, 2025 at 10:34 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Alexander Korotkov <akorotkov@postgresql.org> writes:
> > Refactor ChangeVarNodesExtended() using the custom callback
>
> You are aware that we are in release freeze, no?
I missed that. Should I revert this commit now?
------
Regards,
Alexander Korotkov
Supabase
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: pgsql: Refactor ChangeVarNodesExtended() using the custom callback
@ 2025-05-03 20:06 Tom Lane <tgl@sss.pgh.pa.us>
parent: Alexander Korotkov <aekorotkov@gmail.com>
0 siblings, 1 reply; 13+ messages in thread
From: Tom Lane @ 2025-05-03 20:06 UTC (permalink / raw)
To: Alexander Korotkov <aekorotkov@gmail.com>; +Cc: Alexander Korotkov <akorotkov@postgresql.org>; pgsql-committers@lists.postgresql.org
Alexander Korotkov <aekorotkov@gmail.com> writes:
> On Sat, May 3, 2025 at 10:34 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> You are aware that we are in release freeze, no?
> I missed that. Should I revert this commit now?
Yes, I think so. If you feel it's critical enough to go in on a
release weekend, you should ask pgsql-release@ for an exception.
regards, tom lane
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: pgsql: Refactor ChangeVarNodesExtended() using the custom callback
@ 2025-05-03 20:09 Alexander Korotkov <aekorotkov@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 13+ messages in thread
From: Alexander Korotkov @ 2025-05-03 20:09 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Alexander Korotkov <akorotkov@postgresql.org>; pgsql-committers@lists.postgresql.org
On Sat, May 3, 2025 at 11:06 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Alexander Korotkov <aekorotkov@gmail.com> writes:
> > On Sat, May 3, 2025 at 10:34 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> You are aware that we are in release freeze, no?
>
> > I missed that. Should I revert this commit now?
>
> Yes, I think so. If you feel it's critical enough to go in on a
> release weekend, you should ask pgsql-release@ for an exception.
I don't think it's critical enough. Reverted.
My apologies for breaking the rules.
------
Regards,
Alexander Korotkov
Supabase
^ permalink raw reply [nested|flat] 13+ messages in thread
* pgsql: Refactor ChangeVarNodesExtended() using the custom callback
@ 2025-05-07 08:15 Alexander Korotkov <akorotkov@postgresql.org>
0 siblings, 1 reply; 13+ messages in thread
From: Alexander Korotkov @ 2025-05-07 08:15 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
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(-)
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: pgsql: Refactor ChangeVarNodesExtended() using the custom callback
@ 2025-05-07 08:47 Aleksander Alekseev <aleksander@timescale.com>
parent: Alexander Korotkov <akorotkov@postgresql.org>
0 siblings, 1 reply; 13+ messages in thread
From: Aleksander Alekseev @ 2025-05-07 08:47 UTC (permalink / raw)
To: Alexander Korotkov <akorotkov@postgresql.org>; +Cc: pgsql-committers@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: pgsql: Refactor ChangeVarNodesExtended() using the custom callback
@ 2025-05-07 08:51 Alexander Korotkov <aekorotkov@gmail.com>
parent: Aleksander Alekseev <aleksander@timescale.com>
0 siblings, 1 reply; 13+ messages in thread
From: Alexander Korotkov @ 2025-05-07 08:51 UTC (permalink / raw)
To: Aleksander Alekseev <aleksander@timescale.com>; +Cc: Alexander Korotkov <akorotkov@postgresql.org>; pgsql-committers@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: pgsql: Refactor ChangeVarNodesExtended() using the custom callback
@ 2025-05-07 08:59 Aleksander Alekseev <aleksander@timescale.com>
parent: Alexander Korotkov <aekorotkov@gmail.com>
0 siblings, 1 reply; 13+ messages in thread
From: Aleksander Alekseev @ 2025-05-07 08:59 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org; +Cc: Alexander Korotkov <aekorotkov@gmail.com>; Alexander Korotkov <akorotkov@postgresql.org>
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
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: pgsql: Refactor ChangeVarNodesExtended() using the custom callback
@ 2025-05-07 09:33 Alexander Korotkov <aekorotkov@gmail.com>
parent: Aleksander Alekseev <aleksander@timescale.com>
0 siblings, 1 reply; 13+ messages in thread
From: Alexander Korotkov @ 2025-05-07 09:33 UTC (permalink / raw)
To: Aleksander Alekseev <aleksander@timescale.com>; +Cc: pgsql-committers@lists.postgresql.org, Alexander Korotkov <akorotkov@postgresql.org>
Hi, Aleksander!
On Wed, May 7, 2025 at 11:59 AM Aleksander Alekseev
<aleksander@timescale.com> wrote:
> > > 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?
How can this or other commits on master branch prevent creating new
tag REL_18_BETA2 or forking REL_18_STABLE from master? Perhaps, this
commit is not the only one on master branch after tag REL_18_BETA1.
------
Regards,
Alexander Korotkov
Supabase
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: pgsql: Refactor ChangeVarNodesExtended() using the custom callback
@ 2025-05-07 09:47 Aleksander Alekseev <aleksander@timescale.com>
parent: Alexander Korotkov <aekorotkov@gmail.com>
0 siblings, 1 reply; 13+ messages in thread
From: Aleksander Alekseev @ 2025-05-07 09:47 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org; +Cc: Alexander Korotkov <akorotkov@postgresql.org>; Alexander Korotkov <aekorotkov@gmail.com>
Hi,
> How can this or other commits on master branch prevent creating new
> tag REL_18_BETA2 or forking REL_18_STABLE from master? Perhaps, this
> commit is not the only one on master branch after tag REL_18_BETA1.
Right, but I see people fixing whitespaces and documentation for PG18
in the `master` branch right now. My humble understanding is that
`master` is basically REL_18_STABLE at the moment.
If one wants to fork REL_18_STABLE for instance, either the branch
will include your change (which we probably don't want), or one will
have to manually revert it, or create a fork from BETA1 and
cherry-pick all the necessary changes from `master`.
This is the reason why I suspected this was committed by mistake.
--
Best regards,
Aleksander Alekseev
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: pgsql: Refactor ChangeVarNodesExtended() using the custom callback
@ 2025-05-07 10:25 Alexander Korotkov <aekorotkov@gmail.com>
parent: Aleksander Alekseev <aleksander@timescale.com>
0 siblings, 1 reply; 13+ messages in thread
From: Alexander Korotkov @ 2025-05-07 10:25 UTC (permalink / raw)
To: Aleksander Alekseev <aleksander@timescale.com>; +Cc: pgsql-committers@lists.postgresql.org, Alexander Korotkov <akorotkov@postgresql.org>
On Wed, May 7, 2025 at 12:47 PM Aleksander Alekseev
<aleksander@timescale.com> wrote:
> > How can this or other commits on master branch prevent creating new
> > tag REL_18_BETA2 or forking REL_18_STABLE from master? Perhaps, this
> > commit is not the only one on master branch after tag REL_18_BETA1.
>
> Right, but I see people fixing whitespaces and documentation for PG18
> in the `master` branch right now. My humble understanding is that
> `master` is basically REL_18_STABLE at the moment.
>
> If one wants to fork REL_18_STABLE for instance, either the branch
> will include your change (which we probably don't want), or one will
> have to manually revert it, or create a fork from BETA1 and
> cherry-pick all the necessary changes from `master`.
>
> This is the reason why I suspected this was committed by mistake.
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.
This commit changes ABI, but this should be OK. We do preserve ABI
only for stable releases.
------
Regards,
Alexander Korotkov
Supabase
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: pgsql: Refactor ChangeVarNodesExtended() using the custom callback
@ 2025-05-07 10:35 Aleksander Alekseev <aleksander@timescale.com>
parent: Alexander Korotkov <aekorotkov@gmail.com>
0 siblings, 0 replies; 13+ messages in thread
From: Aleksander Alekseev @ 2025-05-07 10:35 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org; +Cc: Alexander Korotkov <aekorotkov@gmail.com>; Alexander Korotkov <akorotkov@postgresql.org>
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
^ permalink raw reply [nested|flat] 13+ messages in thread
end of thread, other threads:[~2025-05-07 10:35 UTC | newest]
Thread overview: 13+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-05-03 19:32 pgsql: Refactor ChangeVarNodesExtended() using the custom callback Alexander Korotkov <akorotkov@postgresql.org>
2025-05-03 19:34 ` Tom Lane <tgl@sss.pgh.pa.us>
2025-05-03 19:41 ` Alexander Korotkov <aekorotkov@gmail.com>
2025-05-03 20:06 ` Tom Lane <tgl@sss.pgh.pa.us>
2025-05-03 20:09 ` Alexander Korotkov <aekorotkov@gmail.com>
2025-05-07 08:15 pgsql: Refactor ChangeVarNodesExtended() using the custom callback Alexander Korotkov <akorotkov@postgresql.org>
2025-05-07 08:47 ` Aleksander Alekseev <aleksander@timescale.com>
2025-05-07 08:51 ` Alexander Korotkov <aekorotkov@gmail.com>
2025-05-07 08:59 ` Aleksander Alekseev <aleksander@timescale.com>
2025-05-07 09:33 ` Alexander Korotkov <aekorotkov@gmail.com>
2025-05-07 09:47 ` Aleksander Alekseev <aleksander@timescale.com>
2025-05-07 10:25 ` Alexander Korotkov <aekorotkov@gmail.com>
2025-05-07 10:35 ` Aleksander Alekseev <aleksander@timescale.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox