MIME-Version: 1.0
References: <567ED6CA.2040504@sigaev.ru> <CAEudQArk002t_C9LGoeJWnw=iGKikt+gSa8Mh==muG00WS=qYA@mail.gmail.com>
 <CACJufxFhQn0awSEcZD7=FHvjx6OLt6PUtuoi0_10pWFVXUEDJw@mail.gmail.com>
 <ded629b2-7885-4be2-b5f5-9426ff47f72c@postgrespro.ru> <11ee9c04-281f-43c8-a0ff-d481c1e01c09@postgrespro.ru>
 <CACJufxGqLA32zTtaJRL2Cacp771k6PJnau7gsax0KRV434Qq7w@mail.gmail.com>
 <54009ba5-4aea-4344-a30d-e3558a59e2a9@postgrespro.ru> <380e5683-cdfd-4220-ab9b-7cccb9d3e480@postgrespro.ru>
 <CACJufxE6eG3ATnTfO9ubs7CjyfgjpCQrSt85oU33EZj00PdcbQ@mail.gmail.com>
 <e394ac5b-7ae2-4f64-a8ff-51fc934bba3e@postgrespro.ru> <d5d3f257-a2d8-4776-9c97-83f98e74636c@postgrespro.ru>
 <CAPpHfdtACtU_XbuNf9mgDBsTH1cN=i+uASXKBX_rvQk6zQ3MdQ@mail.gmail.com>
 <7389d0dd-05d5-41b7-a12d-2e73f939f851@postgrespro.ru> <CAPpHfdsAWGF2VvcszNofc32Sa2pefBpgY1nw2aYV=FTG7Q3wyA@mail.gmail.com>
 <74e3c8bc-dccd-45e5-ad2d-6a6e96fc5864@postgrespro.ru> <CAPpHfdutHt31sdt2rfU=4fsDMWxf6tvtnHARgCzLY2Tf21+fgw@mail.gmail.com>
 <a5762b63-59c0-4721-8755-2ae6a73338ce@postgrespro.ru>
In-Reply-To: <a5762b63-59c0-4721-8755-2ae6a73338ce@postgrespro.ru>
From: Alexander Korotkov <aekorotkov@gmail.com>
Date: Wed, 13 Mar 2024 13:05:41 +0200
Message-ID: <CAPpHfdsADL+agHnnjvi_42hNJWQdHK76SVPbNjsqAyHzxOxv8w@mail.gmail.com>
Subject: Re: POC, WIP: OR-clause support for indexes
To: Andrei Lepikhov <a.lepikhov@postgrespro.ru>
Cc: 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>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CAPpHfdsADL%2BagHnnjvi_42hNJWQdHK76SVPbNjsqAyHzxOxv8w%40mail.gmail.com>
Precedence: bulk

On Wed, Mar 13, 2024 at 7:52=E2=80=AFAM Andrei Lepikhov
<a.lepikhov@postgrespro.ru> wrote:
> On 12/3/2024 22:20, Alexander Korotkov wrote:
> > On Mon, Mar 11, 2024 at 2:43=E2=80=AFPM Andrei Lepikhov
> >> I think you are right. It is probably a better place than any other to
> >> remove duplicates in an array. I just think we should sort and remove
> >> duplicates from entry->consts in one pass. Thus, this optimisation
> >> should be applied to sortable constants.
> >
> > Ok.
> New version of the patch set implemented all we have agreed on for now.
> We can return MAX_SAOP_ARRAY_SIZE constraint and Alena's approach to
> duplicates deletion for non-sortable cases at the end.
> >
> >> Hmm, we already tried to do it at that point. I vaguely recall some
> >> issues caused by this approach. Anyway, it should be done as quickly a=
s
> >> possible to increase the effect of the optimization.
> >
> > I think there were provided quite strong reasons why this shouldn't be
> > implemented at the parse analysis stage [1], [2], [3].  The
> > canonicalize_qual() looks quite appropriate place for that since it
> > does similar transformations.
> Ok. Let's discuss these reasons. In Robert's opinion [1,3], we should do
> the transformation based on the cost model. But in the canonicalize_qual
> routine, we still make the transformation blindly. Moreover, the second
> patch reduces the weight of this reason, doesn't it? Maybe we shouldn't
> think about that as about optimisation but some 'general form of
> expression'?
> Peter [2] worries about the possible transformation outcomes at this
> stage. But remember, we already transform clauses like ROW() IN (...) to
> a series of ORs here, so it is not an issue. Am I wrong?
> Why did we discard the attempt with canonicalize_qual on the previous
> iteration? - The stage of parsing is much more native for building SAOP
> quals. We can reuse make_scalar_array_op and other stuff, for example.
> During the optimisation stage, the only list partitioning machinery
> creates SAOP based on a list of constants. So, in theory, it is possible
> to implement. But do we really need to make the code more complex?

As we currently do OR-to-ANY transformation at the parse stage, the
system catalog (including views, inheritance clauses, partial and
expression indexes, and others) would have a form depending on
enable_or_transformation at the moment of DDL execution.  I think this
is rather wrong.  The enable_or_transformation should be run-time
optimization which affects the resulting query plan, its result
shouldn't be persistent.

Regarding the ROW() IN (...) precedent.

1. AFAICS, this is not exactly an optimization.  This transformation
allows us to perform type matching individually for every value.
Therefore it allows the execute some queries which otherwise would end
up with error.
2. I don't think this is a sample of good design.  This is rather
hack, which is historically here, but we don't want to replicate this
experience.

Given all of the above, I think moving transformation to the
canonicalize_qual() would be the right way to go.

------
Regards,
Alexander Korotkov