Hi! Thank you for your feedback.
On Fri, Aug 23, 2024 at 1:23 PM Andrei Lepikhov <lepi...@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
v37-0001-Transform-OR-clauses-to-SAOP-s-during-index-matc.patch
Description: Binary data
v37-0002-Teach-bitmap-path-generation-about-transforming-.patch
Description: Binary data
