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

From: Alena Rybakina <a(dot)rybakina(at)postgrespro(dot)ru>
To: jian he <jian(dot)universality(at)gmail(dot)com>
Cc: Andrei Lepikhov <a(dot)lepikhov(at)postgrespro(dot)ru>, Robert Haas <robertmhaas(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org, Alexander Korotkov <aekorotkov(at)gmail(dot)com>, 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>
Subject: Re: POC, WIP: OR-clause support for indexes
Date: 2024-01-31 11:10:44
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Hi, thank you for your review and interest in this subject.

On 31.01.2024 13:15, jian he wrote:
> On Wed, Jan 31, 2024 at 10:55 AM jian he<jian(dot)universality(at)gmail(dot)com> wrote:
>> based on my understanding of
>> I think you need move commutator check right after the `if
>> (get_op_rettype(opno) != BOOLOID)` branch
> I was wrong about this part. sorry for the noise.
> I have made some changes (attachment).
> * if the operator expression left or right side type category is
> {array | domain | composite}, then don't do the transformation.
> (i am not 10% sure with composite)

To be honest, I'm not sure about this check, because we check the type
of variable there:

if (!IsA(orqual, OpExpr))
            or_list = lappend(or_list, orqual);
And below:
if (IsA(leftop, Const))
            opno = get_commutator(opno);

            if (!OidIsValid(opno))
                /* Commuter doesn't exist, we can't reverse the order */
                or_list = lappend(or_list, orqual);

            nconst_expr = get_rightop(orqual);
            const_expr = get_leftop(orqual);
        else if (IsA(rightop, Const))
            const_expr = get_rightop(orqual);
            nconst_expr = get_leftop(orqual);
            or_list = lappend(or_list, orqual);

Isn't that enough?

Besides, some of examples (with ARRAY) works fine:

postgres=# CREATE TABLE sal_emp (
    pay_by_quarter  integer[],
    pay_by_quater1 integer[]
postgres=# INSERT INTO sal_emp
    VALUES (
    '{10000, 10000, 10000, 10000}',
postgres=# select * from sal_emp where pay_by_quarter[1] = 10000 or
      pay_by_quarter       | pay_by_quater1
 {10000,10000,10000,10000} | {1,2,3,4}
(1 row)

postgres=# explain select * from sal_emp where pay_by_quarter[1] = 10000
or pay_by_quarter[1]=2;
                          QUERY PLAN
 Seq Scan on sal_emp  (cost=0.00..21.00 rows=9 width=64)
   Filter: (pay_by_quarter[1] = ANY ('{10000,2}'::integer[]))
(2 rows)

> * if the left side of the operator expression node contains volatile
> functions, then don't do the transformation.

I'm also not sure about the volatility check function, because we
perform such a conversion at the parsing stage, and at this stage we
don't have a RelOptInfo variable and especially a RestictInfo such as

Speaking of NextValueExpr, I couldn't find any examples where the
current patch wouldn't work. I wrote one of them below:

postgres=# create table foo (f1 int, f2 int generated always as identity);
postgres=# insert into foo values(1);

postgres=# explain verbose update foo set f1 = 2 where f1=1 or f1=2 ;
                            QUERY PLAN
 Update on  (cost=0.00..38.25 rows=0 width=0)
   ->  Seq Scan on  (cost=0.00..38.25 rows=23 width=10)
         Output: 2, ctid
         Filter: (foo.f1 = ANY ('{1,2}'::integer[]))
(4 rows)

Maybe I missed something. Do you have any examples?

> * some other minor cosmetic changes.
Thank you, I agree with them.

Alena Rybakina
Postgres Professional:
The Russian Postgres Company

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Bharath Rupireddy 2024-01-31 11:36:40 Re: Improve WALRead() to suck data directly from WAL buffers when possible
Previous Message Alvaro Herrera 2024-01-31 10:59:21 Re: CI and test improvements