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-14 14:44:51
Message-ID: 157901309119.709.13609314854524006500.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

This is a high level review only. However, seeing that there is a conflict and does not merge cleanly after commit <b925a00f4ef>, I return to Author.

To be fair the resolution seems quite straight forward and I took the liberty of applying the necessary changes to include the new element of ExplainState introduced in the above commit, namely hide_workers. However since the author might have a different idea on how to incorporate this change I leave it up to him.

Another very high level comment is the introduction of a new test file, namely explain. Seeing `explain.sql` in the tests suits, personally and very opinion based, I would have expected the whole spectrum of the explain properties to be tested. However only a slight fraction of the functionality is tested. Since this is a bit more of a personal opinion, I don't expect any changes unless the author happens to agree.

Other than these minor nitpick, the code seems clear, concise and does what it says on the box. It follows the comments in the discussion thread(s) and solves a real issue.

Please have a look on how commit <b925a00f4ef> affects this patch and rebase.

The new status of this patch is: Waiting on Author

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2020-01-14 14:49:27 Re: Setting min/max TLS protocol in clientside libpq
Previous Message Tom Lane 2020-01-14 14:40:06 Re: Create/alter policy and exclusive table lock