public inbox for [email protected]
help / color / mirror / Atom feedFrom: Alexander Korotkov <[email protected]>
To: Alena Rybakina <[email protected]>
Cc: Nikolay Shaplov <[email protected]>
Cc: [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: Wed, 17 Jul 2024 22:36:19 +0300
Message-ID: <CAPpHfdu1AZK91T8Yy4VYNYO+TEX9dSFWK59sRB6K9eM=zEjHOA@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<9736220.CDJkKcVGEf@thinkpad-pgpro>
<2193851.QkHrqEjB74@thinkpad-pgpro>
<[email protected]>
<[email protected]>
<CAPpHfdvhWE5pArZhgJeLViLx3-A3rxEREZvfkTj3E=h7q-Bx9w@mail.gmail.com>
<[email protected]>
Hi, Alena!
On Wed, Jul 17, 2024 at 3:53 PM Alena Rybakina <[email protected]>
wrote:
> On 17.07.2024 03:03, Alexander Korotkov wrote:
> > Hi, Alena!
> >
> > On Thu, Jul 11, 2024 at 7:17 PM Alena Rybakina
> > <[email protected]> wrote:
> >> I have finished patch and processed almost your suggestions (from [0],
> [1], [2]). It remains only to add tests where the conversion should work,
> but I will add this in the next version.
> >>
> >> [0]
> https://www.postgresql.org/message-id/3381819.e9J7NaK4W3%40thinkpad-pgpro
> >>
> >> [1]
> https://www.postgresql.org/message-id/9736220.CDJkKcVGEf%40thinkpad-pgpro
> >>
> >> [2]
> https://www.postgresql.org/message-id/2193851.QkHrqEjB74%40thinkpad-pgpro
> > I dare making another revision of this patch. In this version I moved
> > the transformation to match_clause_to_indexcol(). Therefore, this
> > allows to successfully construct index scans with SAOP, but has no
> > degradation in generation of bitmap scans which I observed in [1] and
> > [2]. BTW, I found that my description in [2] lacks of t_b_c_idx index
> > definition. Sorry for that.
> >
> > Given that now we're doing OR-to-ANY transformation solely to match an
> > index we don't need complex analysis of OR-list, which potentially
> > could take quadratic time. Instead, we're trying to match every OR
> > element to an index and quit immediately on failure.
>
> Yes I see that. I will look at this in detail, but so far I have not
> found any unpleasant side effects indicating that the patch should be
> moved to another place and this is very good)
>
> The only thing that worries me so far is that most likely we will need
> to analyze the changes in rinfo and distribute them to others places
> where links about them are used.
> But I need to look at this in more detail separately before discussing it.
>
I'm not sure if would need to distribute changes of RestrictInfo's, because
we're modifying anything in-place. Instead we create a new RestrictInfo
for IndexOptInfo. I think this is what Robert proposed at [1]. The side
effect of this I yet see is redundancy of clauses in [2] test case.
QUERY PLAN
----------------------------------------------------------------------------------------------------------------------------------------
Bitmap Heap Scan on t (cost=19.70..26.93 rows=5001 width=12)
Recheck Cond: (((b = 1) AND (b = ANY ('{1,2}'::integer[])) AND (c = 2))
OR ((a = 1) AND (b = 2) AND (b = ANY ('{1,2}'::integer[]))))
Filter: ((a = 1) AND (c = 2))
-> BitmapOr (cost=19.70..19.70 rows=2 width=0)
-> Bitmap Index Scan on t_b_c_idx (cost=0.00..8.60 rows=1
width=0)
Index Cond: ((b = 1) AND (b = ANY ('{1,2}'::integer[])) AND
(c = 2))
-> Bitmap Index Scan on t_a_b_idx (cost=0.00..8.60 rows=1
width=0)
Index Cond: ((a = 1) AND (b = 2) AND (b = ANY
('{1,2}'::integer[])))
(8 rows)
You can see that index conds and recheck conds contain both SAOP clauses
and equality clauses. I this this happens because bitmap scan planning
code doesn't understands equivalency of original and transformed
RestrictInfo's. I'm not yet sure what to do about this. We probably need
to teach bitmap scan planning code to understand this equivalency. Or,
otherwise, just allow this redundancy given that this is quite rare case I
believe.
> Yes, I am ready to agree that there was no degradation in tests [1] and
> [2]. But just in case, I will do a review to rule out any other problems.
> > I'd like to head a feedback on the new place to apply the
> > transformation. It looks like significant simplification for me and
> > the way to go.
> >
> > Also, I have addressed some of notes by Robert Haas [3]. In v27 we
> > don't use expression evaluation, but directly construct an array
> > constant when possible. Also we don't transform operator id to string
> > and back, but directly construct SAOP instead.
> >
> > Links.
> > 1.
> https://www.postgresql.org/message-id/CAPpHfduJtO0s9E%3DSHUTzrCD88BH0eik0UNog1_q3XBF2wLmH6g%40mail.g...
> > 2.
> https://www.postgresql.org/message-id/CAPpHfdtSXxhdv3mLOLjEewGeXJ%2BFtfhjqodn1WWuq5JLsKx48g%40mail.g...
> > 3.
> https://www.postgresql.org/message-id/CA%2BTgmobu0DUFCTF28DuAi975mEc4xYqX3xyt8RA0WbnyrYg%2BFw%40mail...
>
> Thanks for your effort and any help is welcome)
>
> Yesterday I finished a big project in my work and now I'm ready to
> continue working on this thread. I'll write the results one of these days.
>
Great, thank you. I would appreciate your further work on this patch.
Apart from general feedback on approach, the last patch requires comments,
code beautification etc.
Links.
1.
https://www.postgresql.org/message-id/CA%2BTgmoarYLO6PL%2BFEnXJ6A-57KsVsotpvHnB771M-wXQOGNy9w%40mail...
2.
https://www.postgresql.org/message-id/CAPpHfdtSXxhdv3mLOLjEewGeXJ%2BFtfhjqodn1WWuq5JLsKx48g%40mail.g...
------
Regards,
Alexander Korotkov
Supabase
view thread (13+ messages) latest in thread
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], [email protected], [email protected]
Subject: Re: POC, WIP: OR-clause support for indexes
In-Reply-To: <CAPpHfdu1AZK91T8Yy4VYNYO+TEX9dSFWK59sRB6K9eM=zEjHOA@mail.gmail.com>
* 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