Em ter., 11 de jul. de 2023 às 09:29, Alena Rybakina <
lena.riback...@yandex.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...@gmail.com>
> escreveu:
>
>> Hi Alena,
>>
>> Em seg., 10 de jul. de 2023 às 05:38, Alena Rybakina <
>> lena.riback...@yandex.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;
.
>
>> 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.
best regards,
Ranier Vilela
