From: Andrei Lepikhov <a.lepikhov@postgrespro.ru>
To: Tom Lane <tgl@sss.pgh.pa.us>
To: Alexander Korotkov <aekorotkov@gmail.com>
Cc: Richard Guo <guofenglinux@gmail.com>
Cc: Alexander Lakhin <exclusion@gmail.com>
Cc: Gregory Stark (as CFM) <stark.cfm@gmail.com>
Cc: Michał Kłeczek <michal@kleczek.org>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Removing unneeded self joins
Date: Fri, 3 May 2024 10:19:58 +0700
Message-ID: <75de38d9-005d-4995-9ecf-a155844dcbcd@postgrespro.ru> (raw)
In-Reply-To: <2422119.1714691974@sss.pgh.pa.us>
References: <64486b0b-0404-e39e-322d-0801154901f3@postgrespro.ru>
<8800ccf2-ae82-461c-92a1-d4c0a22f8cf2@postgrespro.ru>
<CAPpHfdvJ3jBQaJQKrf69K++kXi+Tvzn6TXO-DKXj0KVJjvMebg@mail.gmail.com>
<1bd80083-a13e-4e9f-a7ce-73c3faab521c@postgrespro.ru>
<CAPpHfdsz8E085YF2VoJe2+h4YAShsh7p--t3kQfQTntiCy63Xg@mail.gmail.com>
<922623af-872e-4cdc-9238-129b18bee42d@postgrespro.ru>
<CAPpHfduoAHTp6O7Q8PDe_w=30v0n_06pGe5M65iR4DYcfaYnRw@mail.gmail.com>
<0ed2cf02-e655-6ec7-a4fe-52fd8d572f65@gmail.com>
<CAPpHfdsHWbdPVzUdmoKv3dpo8RHJOa=LQR-ZAT+89uvktMs9XA@mail.gmail.com>
<735b5cd4-568d-fbf7-ad8c-dcfb4c3af86e@gmail.com>
<CAPpHfdssga9CBQQ6s32t1Xb2Js2WuPfiporbAse334CJiyBb1Q@mail.gmail.com>
<81! 00af4a-d0aa-450d-a09f-541f26b04ac2@postgrespro.ru>
<CAPpHfdvGVcud2i00pEVH2_GS09c8UsDh6P-SEOLd6YMGOxiNEw@mail.gmail.com>
<CAMbWs49Q8g1LPVCeNHYv-Y-gFo826ertrg6nYNC9LL=8=zHP3g@mail.gmail.com>
<CAPpHfdsUiwbGLbBwtdL+UCT5GCUVEU+SqQaaZjtcje3znU2JgQ@mail.gmail.com>
<2422119.1714691974@sss.pgh.pa.us>
On 5/3/24 06:19, Tom Lane wrote:
> Alexander Korotkov <aekorotkov@gmail.com> writes:
>> I would appreciate your review of this patchset, and review from Andrei as well.
>
> I hate to say this ... but if we're still finding bugs this
> basic in SJE, isn't it time to give up on it for v17?
>
> I might feel better about it if there were any reason to think
> these were the last major bugs. But you have already committed
> around twenty separate fixes for the original SJE patch, and
> now here you come with several more; so it doesn't seem like
> the defect rate has slowed materially. There can be no doubt
> whatever that the original patch was far from commit-ready.
>
> I think we should revert SJE for v17 and do a thorough design
> review before trying again in v18.
I need to say I don't see any evidence of bad design.
I think this feature follows the example of 2489d76 [1], 1349d27 [2],
and partitionwise join features — we get some issues from time to time,
but these strengths and frequencies are significantly reduced.
First and foremost, this feature is highly isolated: like the PWJ
feature, you can just disable (not enable?) SJE and it guarantees you
will avoid the problems.
Secondly, this feature reflects the design decisions the optimiser has
made before. It raises some questions: Do we really control the
consistency of our paths and the plan tree? Maybe we hide our
misunderstanding of its logic by extensively copying expression trees,
sometimes without fundamental necessity. Perhaps the optimiser needs
some abstraction layers or reconstruction to reduce the quickly growing
complexity.
A good example here is [1]. IMO, the new promising feature it has
introduced isn't worth the complexity it added to the planner.
SJE, much like OR <-> ANY transformation, introduces a fresh perspective
into the planner: if we encounter a complex, redundant query, it may be
more beneficial to invest in simplifying the internal query
representation rather than adding new optimisations that will grapple
with this complexity.
Also, SJE raised questions I've never seen before, like: Could we
control the consistency of the PlannerInfo by changing something in the
logic?
Considering the current state, I don't see any concrete outcomes or
evidence that a redesign of the feature will lead us to a new path.
However, I believe the presence of SJE in the core could potentially
trigger improvements in the planner. As a result, I vote to stay with
the feature. But remember, as an author, I'm not entirely objective, so
let's wait for alternative opinions.
[1] Make Vars be outer-join-aware
[2] Improve performance of ORDER BY / DISTINCT aggregates
--
regards,
Andrei Lepikhov
Postgres Professional
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: a.lepikhov@postgrespro.ru, tgl@sss.pgh.pa.us, aekorotkov@gmail.com, guofenglinux@gmail.com, exclusion@gmail.com, stark.cfm@gmail.com, michal@kleczek.org, pgsql-hackers@lists.postgresql.org
Subject: Re: Removing unneeded self joins
In-Reply-To: <75de38d9-005d-4995-9ecf-a155844dcbcd@postgrespro.ru>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
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