On 10/17/24 03:39, Alexander Korotkov wrote:
> On Wed, Oct 16, 2024 at 7:22 AM Andrei Lepikhov <lepihov@gmail.com> wrote:
>> On 10/12/24 21:25, Alexander Korotkov wrote:
>>> I forgot to specify (COSTS OFF) for EXPLAINs in regression tests. Fixed in v42.
>> I've passed through the patch set.
>>
>> Let me put aside the v42-0003 patch—it looks debatable, and I need time
>> to analyse the change in regression tests caused by this patch.
>
> Yes, 0003 patch is for illustration purposes for now. I will not keep
> rebasing it. We can pick it later when main patches are committed.
Got it. I will save it into the TODO list.
>
>> Comments look much better according to my current language level. Ideas
>> with fast exits also look profitable and are worth an additional
>> 'matched' variable.
>>
>> So, in general, it is ok. I think only one place with
>> inner_other_clauses can be improved. Maybe it will be enough to create
>> this list only once, outside 'foreach(j, groupedArgs)' cycle? Also, the
>> comment on the necessity of this operation was unclear to me. See the
>> attachment for my modest attempt at improving it.
>
> Thank you, I've integrated your patch with minor edits from me.
Thanks, I'm not sure about necessity to check NIL value of a list
(list_free also do it), but I'm ok with the edits.
--
regards, Andrei Lepikhov