On 11.07.2023 16:29, Ranier Vilela wrote:
> Em ter., 11 de jul. de 2023 às 09:29, Alena Rybakina
> <lena(dot)ribackina(at)yandex(dot)ru> escreveu:
>
> Hi!
>
> On 10.07.2023 15:15, Ranier Vilela wrote:
>> Em seg., 10 de jul. de 2023 às 09:03, Ranier Vilela
>> <ranier(dot)vf(at)gmail(dot)com> escreveu:
>>
>> 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.
>>
> Sorry, I fixed it.
>>
>> 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;
>> + }
>>
> Agreed.
>
> You missed in removing the declaration
> - bool const_is_left = true;
Yes, thank you. I fixed it.
> .
>
>>
>> 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))
>>
> Is it safe? Maybe we should first make sure that it can be checked
> on RECORDOID at all?
>
> Yes it's safe, because && connector.
> But you can leave as is in v5.
>
Added it.
--
Regards,
Alena Rybakina
Postgres Professional