Minor comment update in execPartition.c

Started by Etsuro Fujitaover 7 years ago4 messages

fujita.etsuro@lab.ntt.co.jp

over 7 years ago

1 attachment(s)

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

Attachments:

ExecInitPartitionInfo-comment.patchtext/x-diff; name=ExecInitPartitionInfo-comment.patchDownload

diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c
index a2f6b29..4c9f71f 100644
--- a/src/backend/executor/execPartition.c
+++ b/src/backend/executor/execPartition.c
@@ -294,8 +294,7 @@ ExecFindPartition(ResultRelInfo *resultRelInfo, PartitionDispatch *pd,
 
 /*
  * ExecInitPartitionInfo
- *		Initialize ResultRelInfo and other information for a partition if not
- *		already done
+ *		Initialize ResultRelInfo and other information for a partition
  *
  * Returns the ResultRelInfo
  */

Amit Langote

Langote_Amit_f8@lab.ntt.co.jp

over 7 years ago

In reply to: Etsuro Fujita (#1)

Re: Minor comment update in execPartition.c

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]https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=edd44738bc8, 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]/messages/by-id/5A7443DF.1010408@lab.ntt.co.jp. 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]: /messages/by-id/5A7443DF.1010408@lab.ntt.co.jp

Alvaro Herrera

alvherre@alvh.no-ip.org

over 7 years ago

In reply to: Amit Langote (#2)

Re: Minor comment update in execPartition.c

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

Etsuro Fujita

fujita.etsuro@lab.ntt.co.jp

over 7 years ago

In reply to: Alvaro Herrera (#3)

Re: Minor comment update in execPartition.c

(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