Re: POC, WIP: OR-clause support for indexes - Mailing list pgsql-hackers
From | Alena Rybakina |
---|---|
Subject | Re: POC, WIP: OR-clause support for indexes |
Date | |
Msg-id | 7aed2a84-41a5-450f-9630-514338172017@postgrespro.ru Whole thread Raw |
In response to | POC, WIP: OR-clause support for indexes (Teodor Sigaev <teodor@sigaev.ru>) |
Responses |
Re: POC, WIP: OR-clause support for indexes
|
List | pgsql-hackers |
Hi! To be fair, I fixed this before [0] by selecting the appropriate group of "or" expressions to transform them to "ANY" expression and then checking for compatibility with the index column. maybe we should try this too? I can think about it. [0] https://www.postgresql.org/message-id/531fc0ab-371e-4235-97e3-dd2d077b6995%40postgrespro.ru On 23.08.2024 15:58, Alexander Korotkov wrote: > Hi! > > Thank you for your feedback. > > On Fri, Aug 23, 2024 at 1:23 PM Andrei Lepikhov <lepihov@gmail.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 -- Regards, Alena Rybakina Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
pgsql-hackers by date: