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-23 10:38:38
Message-ID: 157977591818.742.16465207652636670142.pgcf@coridan.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: not tested
Spec compliant: not tested
Documentation: not tested

> Ah, I see. I think I got that from ExplainPrintSettings. I've
> corrected my usage--thanks for pointing it out. I appreciate the
> effort to maintain a consistent style.

Thanks, I am just following the reviewing guide to be honest.

> Also, reviewing my code again, I noticed that when I moved the general
> worker output earlier, I missed part of the merge conflict:

Right. I thought that was intentional.

> which ignores the es->hide_workers flag (it did not fail the tests,
> but the intent is pretty clear). I've corrected this in the current
> patch.

Noted and appreciated.

> I also noticed that we can now handle worker buffer output more
> consistently across TEXT and structured formats, so I made that small
> change too:

Looks good.

> I think the "producing plan output for a worker" process is easier to
> reason about now, and while it changes TEXT format worker output
> order, the other changes in this patch are more drastic so this
> probably does not matter.
>
> I've also addressed the other feedback above, and reworded a couple of
> comments slightly.

Thanks for the above. Looks clean, does what it says in the tin and solves a
problem that needs solving. All relevant installcheck-world pass. As far as I
am concerned, I think it is ready to be sent to a committer. Updating the status
accordingly.

The new status of this patch is: Ready for Committer

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2020-01-23 10:47:03 Re: Parallel grouping sets
Previous Message Amit Langote 2020-01-23 10:10:05 Re: adding partitioned tables to publications