I'm curious to hear what you and others think of the refactoring.
It'd be nice if there's a good way to add a test case for verbose output involving parallel workers, but the output is unstable ...
I've reviewed this patch, and it works as expected - the refactoring changes by Justin also appear to make sense to me.
I've briefly thought whether this needs documentation (currently the patch includes none), but there does not appear to be a good place to add documentation about this from a quick glance, so it seems acceptable to leave this out given the lack of more detailed EXPLAIN documentation in general.
The one item that still feels a bit open to me is benchmarking, based on Andres' comment a while ago:
On Mon, Oct 19, 2020 at 4:20 PM Andres Freund <andres@anarazel.de> wrote:
I'm a bit worried that further increasing the size of struct Instrumentation will increase the overhead of EXPLAIN ANALYZE further - in some workloads we spend a fair bit of time in code handling that. It would be good to try to find a few bad cases, and see what the overhead is.
Whilst no specific bad cases were provided, I wonder if even a simple pgbench with auto_explain (and log_analyze=1) would be a way to test this?
The overhead of the Instrumentation struct size should show regardless of whether a plan actually includes a Nested Loop.