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

Hi Alena,

Em seg., 10 de jul. de 2023 às 05:38, Alena Rybakina <
lena(dot)ribackina(at)yandex(dot)ru> escreveu:

> I agreed with the changes. Thank you for your work.
>
> I updated patch and added you to the authors.
>
> I specified Ranier Vilela as a reviewer.
>
Is a good habit when post a new version of the patch, name it v1, v2,
v3,etc.
Makes it easy to follow development and references on the thread.

Regarding the last patch.
1. I think that variable const_is_left is not necessary.
You can stick with:
+ if (IsA(get_leftop(orqual), Const))
+ nconst_expr =get_rightop(orqual);
+ const_expr = get_leftop(orqual) ;
+ else if (IsA(get_rightop(orqual), Const))
+ nconst_expr =get_leftop(orqual);
+ const_expr = get_rightop(orqual) ;
+ else
+ {
+ or_list = lappend(or_list, orqual);
+ continue;
+ }

2. Test scalar_type != RECORDOID is more cheaper,
mainly if OidIsValid were a function, we knows that is a macro.
+ if (scalar_type != RECORDOID && OidIsValid(scalar_type))

3. Sorry about wrong tip about array_type, but if really necessary,
better use it.
+ newa->element_typeid = scalar_type;
+ newa->array_typeid = array_type;

4. Is a good habit, call free last, to avoid somebody accidentally using it.
+ or_list = lappend(or_list, gentry->expr);
+ list_free(gentry->consts);
+ continue;

5. list_make1(makeString((char *) "=")
Is an invariant?
We can store in a variable and keep out the loop?

Keep up the good work.

best regards,
Ranier Vilela