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

On Thu, Jan 30, 2025 at 3:23 PM Pavel Borisov <pashkin.elfe@gmail.com> wrote:
> On Tue, 28 Jan 2025 at 12:42, Andrei Lepikhov <lepihov@gmail.com> wrote:
> >
> > On 1/28/25 11:36, Andrei Lepikhov wrote:
> > > On 1/27/25 16:50, Alexander Korotkov wrote:
> > > qsort(matches, n, sizeof(OrArgIndexMatch), or_arg_index_match_cmp);
> > >
> > > To fit an index, the order of elements in the target array of the
> > > `ScalarArrayOpExpr` may change compared to the initial list of OR
> > > expressions. If there are indexes that cover the same set of columns but
> > > in reverse order, this could potentially alter the position of a
> > > Subplan. However, I believe this is a rare case; it is supported by the
> > > initial OR path and should be acceptable.
> > I beg your pardon - I forgot that we've restricted the feature's scope
> > and can't combine OR clauses into ScalarArrayOpExpr if the args list
> > contains references to different columns.
> > So, my note can't be applied here.
> >
> > --
> > regards, Andrei Lepikhov
>
> I've looked at the patch v46-0001
> Looks good to me.
>
> There is a test that demonstrates the behavior change. Maybe some more
> cases like are also worth adding to a test.
>
> +SELECT * FROM bitmap_split_or t1, bitmap_split_or t2 WHERE t1.a=t2.c
> OR (t1.a=t2.b OR t1.a=1);
> +                       QUERY PLAN
> +--------------------------------------------------------
> + Nested Loop
> +   ->  Seq Scan on bitmap_split_or t2
> +   ->  Index Scan using t_a_b_idx on bitmap_split_or t1
> +         Index Cond: (a = ANY (ARRAY[t2.c, t2.b, 1]))
> +(4 rows)
> +
> +EXPLAIN (COSTS OFF)
> +SELECT * FROM bitmap_split_or t1, bitmap_split_or t2 WHERE t1.c=t2.b OR t1.a=1;
> +                  QUERY PLAN
> +----------------------------------------------
> + Nested Loop
> +   Join Filter: ((t1.c = t2.b) OR (t1.a = 1))
> +   ->  Seq Scan on bitmap_split_or t1
> +   ->  Materialize
> +         ->  Seq Scan on bitmap_split_or t2
> +(5 rows)
> +
> +EXPLAIN (COSTS OFF)

Added more tests to join.sql

> Comment
> > *    Also, add any potentially usable join OR clauses to *joinorclauses
> may reflect the change in v46-0001 lappend -> list_append_unique_ptr
> that differs in the processing of equal clauses in the list.

Comments in this function are revised.  I also added detailed
explanation of this change to the commit message.

> Semantics mentioned in the commit message:
> > 2. Make match_join_clauses_to_index() pass OR-clauses to
> >        match_clause_to_index().
> could also be added as comments in the section just before
> match_join_clauses_to_index()

Right, this is addressed too.

> Since d4378c0005e6 comment for match_clause_to_indexcol() I think
> needs change. This could be as a separate commit, not regarding
> current patch v46-0001.
> > * NOTE:  returns NULL if clause is an OR or AND clause; it is the
> > * responsibility of higher-level routines to co

Good catch.  This is added as a separate patch.

> I think the patch can be pushed with possible additions to regression
> test and comments.

OK, thank you!

------
Regards,
Alexander Korotkov
Supabase