Date: Sun, 7 Apr 2024 19:38:36 -0500
From: Justin Pryzby <pryzby@telsasoft.com>
To: Alexander Korotkov <aekorotkov@gmail.com>
Cc: Andrei Lepikhov <a.lepikhov@postgrespro.ru>,
	jian he <jian.universality@gmail.com>,
	Alena Rybakina <a.rybakina@postgrespro.ru>,
	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
Message-ID: <ZhM8jH8gsKm5Q-9p@pryzbyj2023>
References: <74e3c8bc-dccd-45e5-ad2d-6a6e96fc5864@postgrespro.ru>
 <CAPpHfdutHt31sdt2rfU=4fsDMWxf6tvtnHARgCzLY2Tf21+fgw@mail.gmail.com>
 <a5762b63-59c0-4721-8755-2ae6a73338ce@postgrespro.ru>
 <CAPpHfdsADL+agHnnjvi_42hNJWQdHK76SVPbNjsqAyHzxOxv8w@mail.gmail.com>
 <b79b5a8f-40d8-4da9-938e-b96585ccb5d5@postgrespro.ru>
 <CAPpHfdtSXxhdv3mLOLjEewGeXJ+Ftfhjqodn1WWuq5JLsKx48g@mail.gmail.com>
 <6d27d752-db0b-4cac-9843-6ba3dd7a1e94@postgrespro.ru>
 <CAPpHfdu4YJa5xCaOQnLzk4qsutRWrJGbcffVuZ=6R=eTAdMLig@mail.gmail.com>
 <f3705bb3-f9b0-4d92-a660-48ebd764d2df@postgrespro.ru>
 <CAPpHfdtGmUHv=E4QWGWHk2QUuJ-x0mTNBssW_Ehv2dcOQ2L66g@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAPpHfdtGmUHv=E4QWGWHk2QUuJ-x0mTNBssW_Ehv2dcOQ2L66g@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/ZhM8jH8gsKm5Q-9p%40pryzbyj2023>
Precedence: bulk

On Mon, Apr 08, 2024 at 01:34:37AM +0300, Alexander Korotkov wrote:
> Hi!
> 
> On Mon, Apr 1, 2024 at 9:38 AM Andrei Lepikhov
> <a.lepikhov@postgrespro.ru> wrote:
> > On 28/3/2024 16:54, Alexander Korotkov wrote:
> > > The current patch has a boolean guc enable_or_transformation.
> > > However, when we have just a few ORs to be transformated, then we
> > > should get less performance gain from the transformation and higher
> > > chances to lose a good bitmap scan plan from that.  When there is a
> > > huge list of ORs to be transformed, then the performance gain is
> > > greater and it is less likely we could lose a good bitmap scan plan.
> > >
> > > What do you think about introducing a GUC threshold value: the minimum
> > > size of list to do OR-to-ANY transformation?
> > > min_list_or_transformation or something.
> > I labelled it or_transformation_limit (see in attachment). Feel free to
> > rename it.
> > It's important to note that the limiting GUC doesn't operate
> > symmetrically for forward, OR -> SAOP, and backward SAOP -> OR
> > operations. In the forward case, it functions as you've proposed.
> > However, in the backward case, we only check whether the feature is
> > enabled or not. This is due to our existing limitation,
> > MAX_SAOP_ARRAY_SIZE, and the fact that we can't match the length of the
> > original OR list with the sizes of the resulting SAOPs. For instance, a
> > lengthy OR list with 100 elements can be transformed into 3 SAOPs, each
> > with a size of around 30 elements.
> > One aspect that requires attention is the potential inefficiency of our
> > OR -> ANY transformation when we have a number of elements less than
> > MAX_SAOP_ARRAY_SIZE. This is because we perform a reverse transformation
> > ANY -> OR at the stage of generating bitmap scans. If the BitmapScan
> > path dominates, we may have done unnecessary work. Is this an occurrence
> > that we should address?
> > But the concern above may just be a point of improvement later: We can
> > add one more strategy to the optimizer: testing each array element as an
> > OR clause; we can also provide a BitmapOr path, where SAOP is covered
> > with a minimal number of partial indexes (likewise, previous version).
> 
> 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.

The sample config file has the wrong default

+#or_to_any_transform_limit = 0

We had a patch to catch this kind of error, but it was closed (which IMO
was itself an error).

-- 
Justin