Thread: Minor comment update in execPartition.c

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

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

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

(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