From: Nikolay Shaplov <dhyan@nataraj.su>
To: Alena Rybakina <a.rybakina@postgrespro.ru>,
 Alexander Korotkov <aekorotkov@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org,
 Andrei Lepikhov <a.lepikhov@postgrespro.ru>,
 jian he <jian.universality@gmail.com>, Robert Haas <robertmhaas@gmail.com>,
 pgsql-hackers@postgresql.org, Peter Geoghegan <pg@bowt.ie>,
 "Finnerty, Jim" <jfinnert@amazon.com>, Marcos Pegoraro <marcos@f10.com.br>,
 teodor@sigaev.ru, Tomas Vondra <tomas.vondra@enterprisedb.com>,
 Peter Eisentraut <peter@eisentraut.org>, Ranier Vilela <ranier.vf@gmail.com>
Subject: Re: POC, WIP: OR-clause support for indexes
Date: Sun, 21 Jul 2024 11:17:01 +0300
Message-ID: <8969055.VV5PYv0bhD@thinkpad-pgpro>
Organization: Postgres Professional
In-Reply-To:
 <CAPpHfdu1AZK91T8Yy4VYNYO+TEX9dSFWK59sRB6K9eM=zEjHOA@mail.gmail.com>
References:
 <567ED6CA.2040504@sigaev.ru>
 <dee1fedc-03ee-468e-943f-b8cbd685c583@postgrespro.ru>
 <CAPpHfdu1AZK91T8Yy4VYNYO+TEX9dSFWK59sRB6K9eM=zEjHOA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; boundary="nextPart6573450.K2JlShyGXD";
 micalg="pgp-sha512"; protocol="application/pgp-signature"
Archived-At: <https://www.postgresql.org/message-id/8969055.VV5PYv0bhD%40thinkpad-pgpro>
Precedence: bulk

--nextPart6573450.K2JlShyGXD
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="UTF-8"; protected-headers="v1"
From: Nikolay Shaplov <dhyan@nataraj.su>
Subject: Re: POC, WIP: OR-clause support for indexes
Date: Sun, 21 Jul 2024 11:17:01 +0300
Message-ID: <8969055.VV5PYv0bhD@thinkpad-pgpro>
Organization: Postgres Professional
MIME-Version: 1.0

=D0=92 =D0=BF=D0=B8=D1=81=D1=8C=D0=BC=D0=B5 =D0=BE=D1=82 =D1=81=D1=80=D0=B5=
=D0=B4=D0=B0, 17 =D0=B8=D1=8E=D0=BB=D1=8F 2024 =D0=B3. 22:36:19 MSK =D0=BF=
=D0=BE=D0=BB=D1=8C=D0=B7=D0=BE=D0=B2=D0=B0=D1=82=D0=B5=D0=BB=D1=8C Alexande=
r=20
Korotkov =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB:

Hi All!

I am continue reading the patch, now it's newer version

=46irst main question:

As far a I can get, the entry point for OR->ANY convertation have been move=
d=20
to match_clause_to_indexcol funtion, that checks if some restriction can us=
e=20
index for performance.

The thing I do not understand what match_clause_to_indexcol actually receiv=
ed=20
as arguments. Should this be set of expressions  with OR in between grouped=
 by=20
one of the expression argument?

If not I do not understand how this ever should work.

The rest is about code readability

> +	if (bms_is_member(index->rel->relid, rinfo->right_relids))
> +		return NULL;

This check it totally not obvious for person who is not deep into postgres=
=20
code. There should go comment explaining what are we checking for, and why =
it=20
does not suit our purposes


> +	foreach(lc, orclause->args)
> +	{
Being no great expert in postgres code, I am confused what are we iterating=
 on=20
here? Two arguments of OR statement? (a>1) OR (b>2) those in brackets? Or=20
what? Comment explaining that would be a great help here.


> +if (sub_rinfo->is_pushed_down !=3D rinfo->is_pushed_down ||
> +	sub_rinfo->is_clone !=3D rinfo->is_clone ||
> +	sub_rinfo->security_level !=3D rinfo->security_level ||
> +	!bms_equal(sub_rinfo->required_relids, rinfo->required_relids) ||
> +	!bms_equal(sub_rinfo->incompatible_relids, rinfo-=20
incompatible_relids) ||
> +	!bms_equal(sub_rinfo->outer_relids, rinfo->outer_relids))
> +	{=20

This check it totally mind-blowing... What in the name of existence is goin=
g=20
on here?

I would suggest  to split these checks into parts (compiler optimizer shoul=
d=20
take care about overhead)  and give each part a sane explanation.

=2D-=20
Nikolay Shaplov aka Nataraj
=46uzzing Engineer at Postgres Professional
Matrix IM: @dhyan:nataraj.su

--nextPart6573450.K2JlShyGXD
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: This is a digitally signed message part.
Content-Transfer-Encoding: 7Bit

-----BEGIN PGP SIGNATURE-----

iQEzBAABCgAdFiEE+sk3ebqQKlezKOi8PMbfuIHAGpgFAmacw/0ACgkQPMbfuIHA
Gpj0WQf9GNBcfLqeR8KMZEtdIgeU6lwzvlvvw7PgYjA0YjW8U61w3mx+2FrZ/Aau
VM5lVqqlPeXZ2H6VOuxTFreCOnPqnzKIaGiWkJNPUvFw/WRUmZBh9GVGb7etYmSw
TToco6cUmI5TIT0QZM+SI0DMDKL9EoaACT0EWPIcauE7gSliNc75oe/mEfbCTDQZ
97WjOR7JetefpyGSttvPrWdJe97PR7Q45B8mt5RfeR80J35zbRMVEbknciffwFtq
y6o3fnY/gMe6kXMerfaW8TN5QCe7Py3tJ6HJ7BHLC0vxBKu+yFLH7oESdlAM0jDJ
WoH4NQLZ9cvCUtq9iQsTY2PauYo1Zg==
=0IeD
-----END PGP SIGNATURE-----

--nextPart6573450.K2JlShyGXD--