Обсуждение: Minor comment update in execPartition.c

Поиск
Список
Период
Сортировка

Minor comment update in execPartition.c

От
Etsuro Fujita
Дата:
Hi,

Here is a comment for ExecInitPartitionInfo:

 296  * ExecInitPartitionInfo
 297  *      Initialize ResultRelInfo and other information for a
partition if not
 298  *      already done

I think we should remove the words "if not already done" from that
comment because 1) that function is called if the partition wasn't
already initialized and 2) that function assumes that.  Attached is a
small patch for removing the words.

Best regards,
Etsuro Fujita

Вложения

Re: Minor comment update in execPartition.c

От
Amit Langote
Дата:
Fujita-san,

On 2018/04/24 20:14, Etsuro Fujita wrote:
> Hi,
> 
> Here is a comment for ExecInitPartitionInfo:
> 
>  296  * ExecInitPartitionInfo
>  297  *      Initialize ResultRelInfo and other information for a
> partition if not
>  298  *      already done
> 
> I think we should remove the words "if not already done" from that
> comment because 1) that function is called if the partition wasn't
> already initialized and 2) that function assumes that.  Attached is a
> small patch for removing the words.

Thanks, sounds fine.  I think those words remain from earlier versions of
the patch committed as edd44738bc8 [1], where there used to be a fast-path
return if the ResultRelInfo was already non-NULL for the passed in index.
 I remember you suggested making it so that we call ExecInitPartitionInfo
only if needed [2].  After making the changes as suggested, the "if not
already done" became redundant.

Thanks,
Amit

[1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=edd44738bc8

[2] https://www.postgresql.org/message-id/5A7443DF.1010408%40lab.ntt.co.jp



Re: Minor comment update in execPartition.c

От
Alvaro Herrera
Дата:
Amit Langote wrote:

> > I think we should remove the words "if not already done" from that
> > comment because 1) that function is called if the partition wasn't
> > already initialized and 2) that function assumes that.  Attached is a
> > small patch for removing the words.
> 
> Thanks, sounds fine.  I think those words remain from earlier versions of
> the patch committed as edd44738bc8 [1], where there used to be a fast-path
> return if the ResultRelInfo was already non-NULL for the passed in index.

Makes sense -- pushed, thanks both.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: Minor comment update in execPartition.c

От
Etsuro Fujita
Дата:
(2018/04/25 11:05), Alvaro Herrera wrote:
> Amit Langote wrote:
>
>>> I think we should remove the words "if not already done" from that
>>> comment because 1) that function is called if the partition wasn't
>>> already initialized and 2) that function assumes that.  Attached is a
>>> small patch for removing the words.
>>
>> Thanks, sounds fine.  I think those words remain from earlier versions of
>> the patch committed as edd44738bc8 [1], where there used to be a fast-path
>> return if the ResultRelInfo was already non-NULL for the passed in index.
>
> Makes sense -- pushed, thanks both.

Thanks for committing, Alvaro!  Thanks for reviewing, Amit!

Best regards,
Etsuro Fujita