Re: POC, WIP: OR-clause support for indexes

From: Andrei Lepikhov <a(dot)lepikhov(at)postgrespro(dot)ru>
To: Alena Rybakina <a(dot)rybakina(at)postgrespro(dot)ru>
Cc: Alexander Korotkov <aekorotkov(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org, Peter Geoghegan <pg(at)bowt(dot)ie>, "Finnerty, Jim" <jfinnert(at)amazon(dot)com>, Marcos Pegoraro <marcos(at)f10(dot)com(dot)br>, teodor(at)sigaev(dot)ru, Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>, Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>, Peter Eisentraut <peter(at)eisentraut(dot)org>, jian he <jian(dot)universality(at)gmail(dot)com>
Subject: Re: POC, WIP: OR-clause support for indexes
Date: 2024-02-13 10:43:21
Message-ID: d3e4eb41-5863-4248-86ee-5b779231ecb7@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 12/2/2024 17:51, Alena Rybakina wrote:
> On 12.02.2024 12:01, Andrei Lepikhov wrote:
>> On 12/2/2024 15:55, Alena Rybakina wrote:
>>> As I understand it, match_clauses_to_index is necessary if you have a
>>> RestrictInfo (rinfo1) variable, so maybe we should run it after the
>>> make_restrictonfo procedure, otherwise calling it, I think, is useless.
>> I think you must explain your note in more detail. Before this call,
>> we already called make_restrictinfo() and built rinfo1, haven't we?
>>
> I got it, I think, I was confused by the “else“ block when we can
> process the index, otherwise we move on to the next element.
>
> I think maybe “else“ block of creating restrictinfo with the indexpaths
> list creation should be moved to a separate function or just remove "else"?
IMO, it is a matter of taste. But if you are really confused, maybe it
will make understanding for someone else simpler. So, changed.
> I think we need to check that rinfo->clause is not empty, because if it
> is we can miss calling build_paths_for_OR function. We should add it there:
>
> restriction_is_saop_clause(RestrictInfo *restrictinfo)
> {
>     if (IsA(restrictinfo->clause, ScalarArrayOpExpr))
I wonder if we should add here assertion, not NULL check. In what case
we could get NULL clause here? But added for surety.

> By the way, I think we need to add a check that the clauseset is not
> empty (if (!clauseset.nonempty)) otherwise we could get an error. The
> same check I noticed in build_paths_for_OR function.
I don't. Feel free to provide counterexample.

--
regards,
Andrei Lepikhov
Postgres Professional

Attachment Content-Type Size
0002-Teach-generate_bitmap_or_paths-to-build-BitmapOr-pat-20240213.patch text/plain 32.2 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Shlok Kyal 2024-02-13 10:48:33 Re: Add publisher and subscriber to glossary documentation.
Previous Message Andrei Lepikhov 2024-02-13 10:03:20 Re: POC, WIP: OR-clause support for indexes