Re: Removing unneeded self joins

From: Andrei Lepikhov <a(dot)lepikhov(at)postgrespro(dot)ru>
To: Alexander Korotkov <aekorotkov(at)gmail(dot)com>
Cc: "Gregory Stark (as CFM)" <stark(dot)cfm(at)gmail(dot)com>, Michał Kłeczek <michal(at)kleczek(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "a(dot)rybakina" <a(dot)rybakina(at)postgrespro(dot)ru>
Subject: Re: Removing unneeded self joins
Date: 2023-10-13 02:55:13
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 12/10/2023 18:32, Alexander Korotkov wrote:
> On Thu, Oct 5, 2023 at 12:17 PM Andrei Lepikhov
>> We have almost the results we wanted to have. But in the last explain
>> you can see that nothing happened with the OR clause. We should use the
>> expression mutator instead of walker to handle such clauses. But It
>> doesn't process the RestrictInfo node ... I'm inclined to put a solution
>> of this issue off for a while.
> OK. I think it doesn't worth to eliminate IS NULL quals with this
> complexity (at least at this stage of work).

Yeah. I think It would be meaningful in the case of replacing also
nested x IS NOT NULL with nothing. But it requires using a mutator
instead of the walker and may be done more accurately next time.

> I made improvements over the code. Mostly new comments, grammar
> corrections of existing comments and small refactoring.


> Also, I found that the suggestion from David Rowley [1] to qsort
> array of relations to faster find duplicates is still unaddressed.
> I've implemented it. That helps to evade quadratic complexity with
> large number of relations.

I see. The thread is too long so far, thanks for the catch.

> Also I've incorporated improvements from Alena Rybakina except one for
> skipping SJ removal when no SJ quals is found. It's not yet clear for
> me if this check fix some cases. But at least optimization got skipped
> in some useful cases (as you can see in regression tests).

Agree. I wouldn't say I like it too. But also, I suggest skipping some
unnecessary assertions proposed in that patch:
Assert(toKeep->relid != -1); - quite strange. Why -1? Why not all the
negative numbers, at least?
Assert(is_opclause(orinfo->clause)); - above we skip clauses with
rinfo->mergeopfamilies == NIL. Each mergejoinable clause is already
checked as is_opclause.
All these changes (see in the attachment) are optional.

Andrey Lepikhov
Postgres Professional

Attachment Content-Type Size
minor-changes-v44.diff text/plain 1.7 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Nathan Bossart 2023-10-13 03:07:47 Re: [dynahash] do not refill the hashkey after hash_search
Previous Message Vik Fearing 2023-10-13 02:41:24 Re: PostgreSQL domains and NOT NULL constraint