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

From: Alena Rybakina <a(dot)rybakina(at)postgrespro(dot)ru>
To: Andrei Lepikhov <a(dot)lepikhov(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-12 10:51:45
Message-ID: 18584b1b-ca12-480c-9a82-4f43fd08dc62@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 12.02.2024 12:01, Andrei Lepikhov wrote:
> On 12/2/2024 15:55, Alena Rybakina wrote:
>> Hi!
>>
>> I can't unnderstand this part of code:
>>
>> /* Time to generate index paths */
>>
>> MemSet(&clauseset, 0, sizeof(clauseset));
>> match_clauses_to_index(root, list_make1(rinfo1), index, &clauseset);
>>
>> 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"?

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))
...

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.

--
Regards,
Alena Rybakina
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Shlok Kyal 2024-02-12 11:00:35 Add publisher and subscriber to glossary documentation.
Previous Message Amit Kapila 2024-02-12 10:49:33 Re: Synchronizing slots from primary to standby