From: | Alexander Korotkov <aekorotkov(at)gmail(dot)com> |
---|---|
To: | Andrei Lepikhov <lepihov(at)gmail(dot)com> |
Cc: | Alena Rybakina <a(dot)rybakina(at)postgrespro(dot)ru>, Nikolay Shaplov <dhyan(at)nataraj(dot)su>, pgsql-hackers(at)lists(dot)postgresql(dot)org, jian he <jian(dot)universality(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org, Peter Geoghegan <pg(at)bowt(dot)ie>, Marcos Pegoraro <marcos(at)f10(dot)com(dot)br>, teodor(at)sigaev(dot)ru, Peter Eisentraut <peter(at)eisentraut(dot)org>, Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> |
Subject: | Re: POC, WIP: OR-clause support for indexes |
Date: | 2024-08-23 12:58:46 |
Message-ID: | CAPpHfdvF864n=Lzmjd2XBi9TwboZvrhRtLSt2hCP+JVUv6XKzg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi!
Thank you for your feedback.
On Fri, Aug 23, 2024 at 1:23 PM Andrei Lepikhov <lepihov(at)gmail(dot)com> wrote:
>
> On 21/8/2024 16:52, Alexander Korotkov wrote:
> >> /* Only operator clauses scan match */
> >> Should it be:
> >> /* Only operator clauses can match */
> >> ?
> >
> > Corrected, thanks.
> I found one more: /* Only operator clauses scan match */ - in the
> second patch.
> Also I propose:
> - “might match to the index as whole” -> “might match the index as a whole“
> - Group similar OR-arguments intro dedicated RestrictInfos -> ‘into’
Fixed.
> >> The second one:
> >> When creating IndexClause, we assign the original and derived clauses to
> >> the new, containing transformed array. But logically, we should set the
> >> clause with a list of ORs as the original. Why did you do so?
> >
> > I actually didn't notice that. Corrected to set the OR clause as the
> > original. That change turned recheck to use original OR clauses,
> > probably better this way. Also, that change spotted misuse of
> > RestrictInfo.clause and RestrictInfo.orclause in the second patch.
> > Corrected this too.
> New findings:
> =============
>
> 1)
> if (list_length(clause->args) != 2)
> return NULL;
> I guess, above we can 'continue' the process.
>
> 2) Calling the match_index_to_operand in three nested cycles you could
> break the search on first successful match, couldn't it? At least, the
> comment "just stop with first matching index key" say so.
Fixed.
> 3) I finally found the limit of this feature: the case of two partial
> indexes on the same column. Look at the example below:
>
> SET enable_indexscan = 'off';
> SET enable_seqscan = 'off';
> DROP TABLE IF EXISTS test CASCADE;
> CREATE TABLE test (x int);
> INSERT INTO test (x) SELECT * FROM generate_series(1,100);
> CREATE INDEX ON test (x) WHERE x < 80;
> CREATE INDEX ON test (x) WHERE x > 80;
> VACUUM ANALYZE test;
> EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF)
> SELECT * FROM test WHERE x=1 OR x = 79;
> EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF)
> SELECT * FROM test WHERE x=91 OR x = 81;
> EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF)
> SELECT * FROM test WHERE x=1 OR x = 81 OR x = 83;
>
> The last query doesn't group clauses into two indexes. The reason is in
> match_index_to_operand which classifies all 'x=' to one class. I'm not
> sure because of overhead, but it may be resolved by using
> predicate_implied_by to partial indexes.
Yes, this is the conscious limitation of my patch: to consider similar
OR arguments altogether and one-by-one, not in arbitrary groups. The
important thing here is that we still generating BitmapOR patch as we
do without the patch. So, there is no regression. I would leave this
as is to not make this feature too complicated. This could be improved
in future though.
------
Regards,
Alexander Korotkov
Supabase
Attachment | Content-Type | Size |
---|---|---|
v37-0001-Transform-OR-clauses-to-SAOP-s-during-index-matc.patch | application/octet-stream | 41.6 KB |
v37-0002-Teach-bitmap-path-generation-about-transforming-.patch | application/octet-stream | 23.0 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Eisentraut | 2024-08-23 13:02:11 | Re: replace magic num in struct cachedesc with CATCACHE_MAXKEYS |
Previous Message | Ashutosh Bapat | 2024-08-23 12:57:48 | Re: PG_TEST_EXTRA and meson |