[prev in list] [next in list] [prev in thread] [next in thread]
List: postgresql-hackers
Subject: Re: POC, WIP: OR-clause support for indexes
From: Alena Rybakina <a.rybakina () postgrespro ! ru>
Date: 2024-01-31 11:10:44
Message-ID: a155849d-7e74-4700-8c55-8a945d42935e () postgrespro ! ru
[Download RAW message or body]
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.universality@gmail.com> wrote:
>> based on my understanding of
>> https://www.postgresql.org/docs/current/xoper-optimization.html#XOPER-COMMUTATOR
>> 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);
continue;
}
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);
continue;
}
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);
}
else
{
or_list = lappend(or_list, orqual);
continue;
}
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[]
);
CREATE TABLE
postgres=# INSERT INTO sal_emp
VALUES (
'{10000, 10000, 10000, 10000}',
'{1,2,3,4}');
INSERT 0 1
postgres=# select * from sal_emp where pay_by_quarter[1] = 10000 or
pay_by_quarter[1]=2;
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
PathTarget.
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);
CREATE TABLE
postgres=# insert into foo values(1);
INSERT 0 1
postgres=# explain verbose update foo set f1 = 2 where f1=1 or f1=2 ;
QUERY PLAN
-------------------------------------------------------------------
Update on public.foo (cost=0.00..38.25 rows=0 width=0)
-> Seq Scan on public.foo (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.
--
Regards,
Alena Rybakina
Postgres Professional:http://www.postgrespro.com
The Russian Postgres Company
[Attachment #3 (text/html)]
<!DOCTYPE html>
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<div class="moz-cite-prefix">Hi, thank you for your review and
interest in this subject.
</div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix">On 31.01.2024 13:15, jian he wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CACJufxFrZS07oBHMk1_c8P3A84VZ3ysXiZV8NeU6gAnvu+HsVA@mail.gmail.com">
<pre class="moz-quote-pre" wrap="">On Wed, Jan 31, 2024 at 10:55 AM jian he <a \
class="moz-txt-link-rfc2396E" \
href="mailto:jian.universality@gmail.com"><jian.universality@gmail.com></a> \
wrote: </pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">
based on my understanding of
<a class="moz-txt-link-freetext" \
href="https://www.postgresql.org/docs/current/xoper-optimization.html#XOPER-COMMUTATOR \
">https://www.postgresql.org/docs/current/xoper-optimization.html#XOPER-COMMUTATOR</a>
I think you need move commutator check right after the `if
(get_op_rettype(opno) != BOOLOID)` branch
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">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)</pre>
</blockquote>
<p>To be honest, I'm not sure about this check, because we check the
type of variable there:</p>
<font face="monospace">if (!IsA(orqual, OpExpr))<br>
{<br>
or_list = lappend(or_list, orqual);<br>
continue;<br>
}<br>
And below:<br>
if (IsA(leftop, Const))<br>
{<br>
opno = get_commutator(opno);<br>
<br>
if (!OidIsValid(opno))<br>
{<br>
/* Commuter doesn't exist, we can't reverse the
order */<br>
or_list = lappend(or_list, orqual);<br>
continue;<br>
}<br>
<br>
nconst_expr = get_rightop(orqual);<br>
const_expr = get_leftop(orqual);<br>
}<br>
else if (IsA(rightop, Const))<br>
{<br>
const_expr = get_rightop(orqual);<br>
nconst_expr = get_leftop(orqual);<br>
}<br>
else<br>
{<br>
or_list = lappend(or_list, orqual);<br>
continue;<br>
}</font>
<p>Isn't that enough?<br>
</p>
<p></p>
<p>Besides, some of examples (with ARRAY) works fine:</p>
<p><font face="monospace">postgres=# CREATE TABLE sal_emp (<br>
pay_by_quarter integer[],<br>
pay_by_quater1 integer[]<br>
);<br>
CREATE TABLE<br>
postgres=# INSERT INTO sal_emp<br>
VALUES ( <br>
'{10000, 10000, 10000, 10000}',<br>
'{1,2,3,4}');<br>
INSERT 0 1<br>
postgres=# select * from sal_emp where pay_by_quarter[1] = 10000
or pay_by_quarter[1]=2;<br>
pay_by_quarter | pay_by_quater1 <br>
---------------------------+----------------<br>
{10000,10000,10000,10000} | {1,2,3,4}<br>
(1 row)<br>
<br>
postgres=# explain select * from sal_emp where pay_by_quarter[1]
= 10000 or pay_by_quarter[1]=2;<br>
QUERY PLAN <br>
--------------------------------------------------------------<br>
Seq Scan on sal_emp (cost=0.00..21.00 rows=9 width=64)<br>
Filter: (pay_by_quarter[1] = ANY ('{10000,2}'::integer[]))<br>
(2 rows)</font></p>
<blockquote type="cite"
cite="mid:CACJufxFrZS07oBHMk1_c8P3A84VZ3ysXiZV8NeU6gAnvu+HsVA@mail.gmail.com">
<pre class="moz-quote-pre" wrap="">* if the left side of the operator \
expression node contains volatile functions, then don't do the transformation.</pre>
</blockquote>
<p>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 PathTarget.<br>
</p>
<p>Speaking of <span style="white-space: pre;">NextValueExpr, I couldn't find any \
examples where the current patch wouldn't work. I wrote one of them below</span><span \
style="white-space: pre;">:</span></p> <p><font face="monospace">postgres=# create \
table foo (f1 int, f2 int generated always as identity);<br>
CREATE TABLE<br>
postgres=# insert into foo values(1);<br>
INSERT 0 1<br>
</font> <font face="monospace"><br>
postgres=# explain verbose update foo set f1 = 2 where f1=1 or
f1=2 ;<br>
QUERY
PLAN <br>
-------------------------------------------------------------------<br>
Update on public.foo (cost=0.00..38.25 rows=0 width=0)<br>
-> Seq Scan on public.foo (cost=0.00..38.25 rows=23
width=10)<br>
Output: 2, ctid<br>
Filter: (foo.f1 = ANY ('{1,2}'::integer[]))<br>
(4 rows)</font></p>
<p>Maybe I missed something. Do you have any examples?</p>
<blockquote type="cite"
cite="mid:CACJufxFrZS07oBHMk1_c8P3A84VZ3ysXiZV8NeU6gAnvu+HsVA@mail.gmail.com">
<pre class="moz-quote-pre" wrap="">* some other minor cosmetic changes.
</pre>
</blockquote>
Thank you, I agree with them.<br>
<pre class="moz-signature" cols="72">--
Regards,
Alena Rybakina
Postgres Professional: <a class="moz-txt-link-freetext" \
href="http://www.postgrespro.com">http://www.postgrespro.com</a> The Russian Postgres \
Company</pre> </body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic