On Fri, Oct 2, 2015 at 4:27 AM, Kouhei Kaigai <kaigai@ak.jp.nec.com> wrote:
>> On Thu, Oct 1, 2015 at 2:35 AM, Kouhei Kaigai <kaigai@ak.jp.nec.com> wrote:
>> > Gather node was oversight by readfunc.c, even though it shall not be
>> > serialized actually.
>> > Also, it used incompatible WRITE_xxx_FIELD() macro on outfuncs.c.
>> >
>> > The attached patch fixes both of incomsistence.
>>
>> Thanks. You missed READ_DONE() but fortunately my compiler noticed
>> that oversight. Committed with that fix.
>>
> I could find one other strangenes, at explain.c.
>
> case T_Gather:
> {
> Gather *gather = (Gather *) plan;
>
> show_scan_qual(plan->qual, "Filter", planstate, ancestors, es);
> if (plan->qual)
> show_instrumentation_count("Rows Removed by Filter", 1,
> planstate, es);
> ExplainPropertyInteger("Number of Workers",
> gather->num_workers, es);
> if (gather->single_copy)
> ExplainPropertyText("Single Copy",
> gather->single_copy ? "true" : "false",
> es);
> }
> break;
>
> What is the intention of the last if-check?
> The single_copy is checked in the argument of ExplainPropertyText().
Oops, that was dumb. single_copy only makes sense if num_workers ==
1, so I intended the if-test to be based on num_workers, not
single_copy. Not sure if we should just make that change now or if
there's a better way to do display it.
I'm sort of tempted to try to come up with a shorthand that only uses
one line in text mode - e.g. set pname to something like "Gather 3" if
there are 3 workers, "Gather 1" if there is worker, or "Gather Single"
if there is one worker and we're in single_copy mode. In non-text
mode, of course, the properties should be displayed separately, for
machine parse-ability.
But maybe I'm getting ahead of myself and we should just change it to
if (gather->num_workers == 1) for now.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
