From: | Alena Rybakina <a(dot)rybakina(at)postgrespro(dot)ru> |
---|---|
To: | Alexander Korotkov <aekorotkov(at)gmail(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andrei Lepikhov <lepihov(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Richard Guo <guofenglinux(at)gmail(dot)com>, Alexander Lakhin <exclusion(at)gmail(dot)com>, "Gregory Stark (as CFM)" <stark(dot)cfm(at)gmail(dot)com>, Michał Kłeczek <michal(at)kleczek(dot)org>, Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> |
Subject: | Re: Removing unneeded self joins |
Date: | 2025-02-11 15:31:33 |
Message-ID: | d6319b0c-b186-472f-8b64-41dfa41e7b5f@postgrespro.ru |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi! Thank you for the work with this subject, I think it is really
important.
On 10.02.2025 22:58, Alexander Korotkov wrote:
> Hi!
>
> On Mon, Feb 10, 2025 at 7:19 AM Andrei Lepikhov <lepihov(at)gmail(dot)com> wrote:
>> On 9/2/2025 18:41, Alexander Korotkov wrote:
>>> Regarding adjust_relid_set() and replace_relid(). I think they are
>>> now strictly equivalent, except for the case then old relid is given
>>> and not found. In this case adjust_relid_set() returns the original
>>> relids while replace_relid() returns a copy. The behavior of
>>> adjust_relid_set() appears more desirable as we don't need extra
>>> copying when no modification is done. So, I've replaced all
>>> replace_relid() with adjust_relid_set().
>> Ok, I glanced into it, and it makes sense to merge these routines.
>> I think the comment to adjust_relid_set() should be arranged, too. See
>> the attachment for a short variant of such modification.
>>> Also, I did some grammar correction to your new comment in tests.
>> Thanks!
> I've further revised adjust_relid_set() header comment.
>
> Looking back to the work done since previous attempt to commit this to
> pg17, I can highlight following.
> 1) We're now using more of existing infrastructure including
> adjust_relid_set() and ChangeVarNodes(). The most of complexity is
> still there though.
> 2) We've checked few ways to further simplify this patch. But yet the
> current way still feels to be best possible.
> 3) For sure, several bugs were fixed.
>
> I think we could give it another chance for pg18 after some further
> polishing (at least commit message still needs to be revised). Any
> thoughts on this? Tom?
>
I didn't find any mistakes, I just have a refactoring remark. I think
the part where we add non-redundant expressions with the
binfo_candidates, jinfo_candidates
check can be moved to a separate function, otherwise the code is very
repetitive in this place. I did it and attached diff file
--
Regards,
Alena Rybakina
Postgres Professional
Attachment | Content-Type | Size |
---|---|---|
self-join-removal.diff | text/x-patch | 3.9 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Melanie Plageman | 2025-02-11 15:35:07 | Re: Eagerly scan all-visible pages to amortize aggressive vacuum |
Previous Message | Tomas Vondra | 2025-02-11 15:29:16 | Re: should we have a fast-path planning for OLTP starjoins? |