Sorry, Last mail forget to CC the mailing list. Now the comment is confusing, Maybe someone should correct it.
Here is a simple patch, What do you think ? With Regards, Wu Fei -----Original Message----- From: Amit Kapila [mailto:amit.kapil...@gmail.com] Sent: Wednesday, June 05, 2019 7:18 PM To: Wu, Fei/吴 非 <wufei.f...@cn.fujitsu.com> Cc: pgsql-hack...@postgresql.org Subject: Re: Confusing comment for function ExecParallelEstimate On Wed, Jun 5, 2019 at 11:27 AM Wu, Fei <wufei.f...@cn.fujitsu.com> wrote: > > Thanks for your reply. > From the code below: > (https://github.com/postgres/postgres/blob/REL_10_7/src/backend/execut > or/execParallel.c) > ####################################################################################### > 443 /* > 444 * Give parallel-aware nodes a chance to add to the > estimates, and get a > 445 * count of how many PlanState nodes there are. > 446 */ > 447 e.pcxt = pcxt; > 448 e.nnodes = 0; > 449 ExecParallelEstimate(planstate, &e); > 450 > 451 /* Estimate space for instrumentation, if required. */ > 452 if (estate->es_instrument) > 453 { > 454 instrumentation_len = > 455 offsetof(SharedExecutorInstrumentation, > plan_node_id) + > 456 sizeof(int) * e.nnodes; > 457 instrumentation_len = MAXALIGN(instrumentation_len); > 458 instrument_offset = instrumentation_len; > 459 instrumentation_len += > 460 mul_size(sizeof(Instrumentation), > 461 mul_size(e.nnodes, > nworkers)); > 462 shm_toc_estimate_chunk(&pcxt->estimator, > instrumentation_len); > 463 shm_toc_estimate_keys(&pcxt->estimator, 1); > > ###################################################################### > ################# It seems that e.nnodes which returns from > ExecParallelEstimate(planstate, &e) , determines how much instrumentation > structures in DSM(line459~line461). > And e.nnodes also determines the length of SharedExecutorInstrumentation-> > plan_node_id(line454~line456). > > So, I think here it refers to instrumentation. > Right. I think the way it is mentioned (SharedPlanStateInstrumentation structures ..) in the comment can confuse readers. We can replace SharedPlanStateInstrumentation with Instrumentation in the comment. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
fix_confusing_comment_for_ExecParallelEstimate.patch
Description: fix_confusing_comment_for_ExecParallelEstimate.patch