Re: POC, WIP: OR-clause support for indexes

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