public inbox for [email protected]  
help / color / mirror / Atom feed
From: Alena Rybakina <[email protected]>
To: Alexander Korotkov <[email protected]>
Cc: Andrei Lepikhov <[email protected]>
Cc: jian he <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: [email protected]
Cc: Peter Geoghegan <[email protected]>
Cc: Finnerty, Jim <[email protected]>
Cc: Marcos Pegoraro <[email protected]>
Cc: [email protected]
Cc: Tomas Vondra <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: Ranier Vilela <[email protected]>
Subject: Re: POC, WIP: OR-clause support for indexes
Date: Mon, 17 Jun 2024 13:33:49 +0300
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAPpHfduah1PLzajBJFDmp7+MZuaWYpie2p+GsV0r03fcGghQ-g@mail.gmail.com>
References: <[email protected]>
	<[email protected]>
	<CAPpHfdtACtU_XbuNf9mgDBsTH1cN=i+uASXKBX_rvQk6zQ3MdQ@mail.gmail.com>
	<[email protected]>
	<CAPpHfdsAWGF2VvcszNofc32Sa2pefBpgY1nw2aYV=FTG7Q3wyA@mail.gmail.com>
	<[email protected]>
	<CAPpHfdutHt31sdt2rfU=4fsDMWxf6tvtnHARgCzLY2Tf21+fgw@mail.gmail.com>
	<[email protected]>
	<CAPpHfdsADL+agHnnjvi_42hNJWQdHK76SVPbNjsqAyHzxOxv8w@mail.gmail.com>
	<[email protected]>
	<CAPpHfdtSXxhdv3mLOLjEewGeXJ+Ftfhjqodn1WWuq5JLsKx48g@mail.gmail.com>
	<[email protected]>
	<CAPpHfdu4YJa5xCaOQnLzk4qsutRWrJGbcffVuZ=6R=eTAdMLig@mail.gmail.com>
	<[email protected]>
	<CAPpHfdtGmUHv=E4QWGWHk2QUuJ-x0mTNBssW_Ehv2dcOQ2L66g@mail.gmail.com>
	<CAPpHfduah1PLzajBJFDmp7+MZuaWYpie2p+GsV0r03fcGghQ-g@mail.gmail.com>

Hi, thank you for your work with this subject!

On 14.06.2024 15:00, Alexander Korotkov wrote:
> On Mon, Apr 8, 2024 at 1:34 AM Alexander Korotkov<[email protected]>  wrote:
>> I've revised the patch.  Did some beautification, improvements for
>> documentation, commit messages etc.
>>
>> I've pushed the 0001 patch without 0002.  I think 0001 is good by
>> itself given that there is the or_to_any_transform_limit GUC option.
>> The more similar OR clauses are here the more likely grouping them
>> into SOAP will be a win.  But I've changed the default value to 5.
>> This will make it less invasive and affect only queries with obvious
>> repeating patterns.  That also reduced the changes in the regression
>> tests expected outputs.
>>
>> Regarding 0002, it seems questionable since it could cause a planning
>> slowdown for SAOP's with large arrays.  Also, it might reduce the win
>> of transformation made by 0001.  So, I think we should skip it for
>> now.
> The patch has been reverted from pg17.  Let me propose a new version
> for pg18 based on the valuable feedback from Tom Lane [1][2].
>
>   * The transformation is moved to the stage of adding restrictinfos to
> the base relation (in particular add_base_clause_to_rel()).  This
> leads to interesting consequences.  While this allows IndexScans to
> use transformed clauses, BitmapScans and SeqScans seem unaffected.
> Therefore, I wasn't able to find a planning regression.
>   * As soon as there is no planning regression anymore, I've removed
> or_to_any_transform_limit GUC, which was a source of critics.
>   * Now, not only Consts allowed in the SAOP's list, but also Params.
>   * The criticized hash based on expression jumbling has been removed.
> Now, the plain list is used instead.
>   * OrClauseGroup now gets a legal node tag.  That allows to mix it in
> the list with other nodes without hacks.
>
> I think this patch shouldn't be as good as before for optimizing
> performance of large OR lists, given that BitmapScans and SeqScans
> still deal with ORs.  However, it allows IndexScans to handle more,
> doesn't seem to cause planning regression and therefore introduce no
> extra GUC.  Overall, this seems like a good compromise.
>
> This patch could use some polishing, but I'd like to first hear some
> feedback on general design.
>
> Links
> 1.https://www.postgresql.org/message-id/3604469.1712628736%40sss.pgh.pa.us
> 2.https://www.postgresql.org/message-id/3649287.1712642139%40sss.pgh.pa.us

Inoticedthat7librarieshave 
beenaddedtosrc/backend/optimizer/plan/initsplan.c,andas faras 
Iremember,TomLanehas alreadyexpresseddoubtsaboutthe 
approachthatrequiresaddinga largenumberof libraries[0], but I'm afraid 
I'm out of ideas about alternative approach.

In addition,Icheckedthe fixinthe 
previouscasesthatyouwroteearlier[1]andnoticedthatSeqScancontinuesto 
generate,unfortunately,withoutconvertingexpressions:

with patch:

create table test as (select (random()*10)::int x, (random()*1000) y 
from generate_series(1,1000000) i); create index test_x_1_y on test (y) 
where x = 1; create index test_x_2_y on test (y) where x = 2; vacuum 
analyze test; SELECT 1000000 CREATE INDEX CREATE INDEX VACUUM 
alena@postgres=# explain select * from test where (x = 1 or x = 2) and y 
= 100; QUERY PLAN 
-------------------------------------------------------------------------- 
Gather (cost=1000.00..12690.10 rows=1 width=12) Workers Planned: 2 -> 
Parallel Seq Scan on test (cost=0.00..11690.00 rows=1 width=12) Filter: 
(((x = 1) OR (x = 2)) AND (y = '100'::double precision)) (4 rows) 
alena@postgres=# set enable_seqscan =off; SET alena@postgres=# explain 
select * from test where (x = 1 or x = 2) and y = 100; QUERY PLAN 
------------------------------------------------------------------------- 
Seq Scan on test (cost=10000000000.00..10000020440.00 rows=1 width=12) 
Filter: (((x = 1) OR (x = 2)) AND (y = '100'::double precision)) (2 rows)

without patch:

-------------------------------------------------------------------------------------------------------------- 
Bitmap Heap Scan on test (cost=8.60..12.62 rows=1 width=12) Recheck 
Cond: (((y = '100'::double precision) AND (x = 1)) OR ((y = 
'100'::double precision) AND (x = 2))) -> BitmapOr (cost=8.60..8.60 
rows=1 width=0) -> Bitmap Index Scan on test_x_1_y (cost=0.00..4.30 
rows=1 width=0) Index Cond: (y = '100'::double precision) -> Bitmap 
Index Scan on test_x_2_y (cost=0.00..4.30 rows=1 width=0) Index Cond: (y 
= '100'::double precision) (7 rows)

[0] https://www.postgresql.org/message-id/3604469.1712628736%40sss.pgh.pa.us

[1] 
https://www.postgresql.org/message-id/CAPpHfduJtO0s9E%3DSHUTzrCD88BH0eik0UNog1_q3XBF2wLmH6g%40mail.g... 


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


view thread (41+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: POC, WIP: OR-clause support for indexes
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox