Re: Duplicate Workers entries in some EXPLAIN plans

From: Georgios Kokolatos <gkokolatos(at)pm(dot)me>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Cc: Maciek Sakrejda <m(dot)sakrejda(at)gmail(dot)com>
Subject: Re: Duplicate Workers entries in some EXPLAIN plans
Date: 2020-01-22 17:36:20
Message-ID: 157971458002.742.1102343273264796292.pgcf@coridan.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Geoghegan 2020-01-22 18:40:33 Re: Index Skip Scan
Previous Message Floris Van Nee 2020-01-22 17:24:43 Re: Index Skip Scan