Re: Duplicate Workers entries in some EXPLAIN plans - Mailing list pgsql-hackers

From Georgios Kokolatos
Subject Re: Duplicate Workers entries in some EXPLAIN plans
Date
Msg-id 157971458002.742.1102343273264796292.pgcf@coridan.postgresql.org
Whole thread Raw
In response to Re: Duplicate Workers entries in some EXPLAIN plans  (Maciek Sakrejda <m.sakrejda@gmail.com>)
Responses Re: Duplicate Workers entries in some EXPLAIN plans  (Maciek Sakrejda <m.sakrejda@gmail.com>)
List pgsql-hackers
>> +       int num_workers = planstate->worker_instrument->num_workers;
>> +       int n;
>> +       worker_strs = (StringInfo *) palloc0(num_workers * sizeof(StringInfo));
>> +       for (n = 0; n < num_workers; n++) {
>>
>> I think C99 would be better here. Also no parenthesis needed.
>
>
> Pardon my C illiteracy, but what am I doing that's not valid C99 here?

My bad, I should have been more clear. I meant that it is preferable to use
the C99 standard which calls for declaring variables in the scope that you
need them. In this case, 'n' is needed only in the for loop, so something like

for (int n = 0; n < num_workers; n++) 

is preferable. To be clear, your code was perfectly valid. It was only the
style I was referring to.

>> +       for (n = 0; n < w->num_workers; ++n)
>>
>> I think C99 would be better here.
>
>
> And here (if it's not the same problem)?

Exactly the same as above. 

>>     int         indent;         /* current indentation level */
>>     List       *grouping_stack; /* format-specific grouping state */
>> +   bool        print_workers;  /* whether current node has worker metadata */
>>
>> Hmm.. commit <b925a00f4ef> introduced `hide_workers` in the struct. Having both
>> names in the struct so far apart even seems a bit confusing and sloppy. Do you
>> think it would be possible to combine or rename?
>
>
> I noticed that. I was thinking about combining them, but
> "hide_workers" seems to be about "pretend there is no worker output
> even if there is" and "print_workers" is "keep track of whether or not
> there is worker output to print". Maybe I'll rename to
> "has_worker_output"?

The rename sounds a bit better in my humble opinion. Thanks.

pgsql-hackers by date:

Previous
From: Floris Van Nee
Date:
Subject: Re: Index Skip Scan
Next
From: Peter Geoghegan
Date:
Subject: Re: Index Skip Scan