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
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 |