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

Hi!

On Thu, Aug 15, 2024 at 10:13 PM Alena Rybakina
<a(dot)rybakina(at)postgrespro(dot)ru> wrote:
> On 07.08.2024 04:11, Alexander Korotkov wrote:
> > On Mon, Aug 5, 2024 at 11:24 PM Alena Rybakina
> > <a(dot)rybakina(at)postgrespro(dot)ru> wrote:
> >> Ok, thank you for your work)
> >>
> >> I think we can leave only the two added libraries in the first patch,
> >> others are superfluous.
> > Thank you.
> > I also have fixed some grammar issues.
>
> While reviewing the patch, I can't understand one part of the code where
> we check the comparability of restrictinfos.
>
> /* RestrictInfo parameters dmust match parent */
> if (subRinfo->is_pushed_down != rinfo->is_pushed_down ||
> subRinfo->is_clone != rinfo->is_clone ||
> subRinfo->security_level != rinfo->security_level ||
> !bms_equal(subRinfo->required_relids,
> rinfo->required_relids) ||
> !bms_equal(subRinfo->incompatible_relids,
> rinfo->incompatible_relids) ||
> !bms_equal(subRinfo->outer_relids, rinfo->outer_relids))
> return NULL;
>
> I didn't find a place in the optimizer where required_relids,
> incompatible_relids and outer_relids become different. Each
> make_restrictinfo function takes arguments from
> parent data.
>
> I disabled this check and the regression tests passed. This code is
> needed for security verification， may I clarify?

Thank you for pointing this. I've rechecked the life cycle of those
parameters. make_restrictinfo() makes them initially equal (except
required_relids which might be narrower for sub-clauses). The later
changes like adjust_appendrel_attrs_mutator() applies equally for the
both parent and children.

So, I've turned this into assert check.

> In the last patch I corrected the libraries - one of them was not in
> alphabetical order.

Thank you!

Also, I convert the check you've introduced in the previous message to
op_in_opfamily(), and introduced collation check similar to
match_opclause_to_indexcol().

------
Regards,
Alexander Korotkov
Supabase