From b925a00f4ef65db9359e1c60fbf0e56d05afb25a Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 16 Dec 2019 20:14:25 -0500 Subject: [PATCH] Fix "force_parallel_mode = regress" to work with ANALYZE + VERBOSE. force_parallel_mode = regress is supposed to force use of a Gather node without having any impact on EXPLAIN output. But it failed to accomplish that if both ANALYZE and VERBOSE are given, because that enables per-worker output data that you wouldn't see if the Gather hadn't been inserted. Improve the logic so that we suppress the per-worker data too. This allows putting the new test case added by commit 5935917ce back into the originally intended form (cf. 776a2c887, 22864f6e0). We can also get rid of a kluge in subselect.sql, which previously had to clean up after force_parallel_mode's failure to do what it said on the tin. Discussion: https://postgr.es/m/18445.1576177309@sss.pgh.pa.us --- src/backend/commands/explain.c | 27 ++++++++++++++++--- src/include/commands/explain.h | 1 + src/test/regress/expected/partition_prune.out | 10 +++---- src/test/regress/expected/subselect.out | 2 -- src/test/regress/sql/partition_prune.sql | 2 +- src/test/regress/sql/subselect.sql | 2 -- 6 files changed, 30 insertions(+), 14 deletions(-) diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index 7ae5a42e455..949fefa23ae 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -695,13 +695,19 @@ ExplainPrintPlan(ExplainState *es, QueryDesc *queryDesc) /* * Sometimes we mark a Gather node as "invisible", which means that it's - * not displayed in EXPLAIN output. The purpose of this is to allow + * not to be displayed in EXPLAIN output. The purpose of this is to allow * running regression tests with force_parallel_mode=regress to get the * same results as running the same tests with force_parallel_mode=off. + * Such marking is currently only supported on a Gather at the top of the + * plan. We skip that node, and we must also hide per-worker detail data + * further down in the plan tree. */ ps = queryDesc->planstate; if (IsA(ps, GatherState) &&((Gather *) ps->plan)->invisible) + { ps = outerPlanState(ps); + es->hide_workers = true; + } ExplainNode(ps, NIL, NULL, NULL, es); /* @@ -806,6 +812,10 @@ ExplainPrintJIT(ExplainState *es, int jit_flags, if (!ji || ji->created_functions == 0) return; + /* don't print per-worker info if we're supposed to hide that */ + if (for_workers && es->hide_workers) + return; + /* calculate total time */ INSTR_TIME_SET_ZERO(total_time); INSTR_TIME_ADD(total_time, ji->generation_counter); @@ -1877,7 +1887,8 @@ ExplainNode(PlanState *planstate, List *ancestors, show_buffer_usage(es, &planstate->instrument->bufusage); /* Show worker detail */ - if (es->analyze && es->verbose && planstate->worker_instrument) + if (es->analyze && es->verbose && !es->hide_workers && + planstate->worker_instrument) { WorkerInstrumentation *w = planstate->worker_instrument; bool opened_group = false; @@ -2574,6 +2585,12 @@ show_sort_info(SortState *sortstate, ExplainState *es) } } + /* + * You might think we should just skip this stanza entirely when + * es->hide_workers is true, but then we'd get no sort-method output at + * all. We have to make it look like worker 0's data is top-level data. + * Currently, we only bother with that for text-format output. + */ if (sortstate->shared_info != NULL) { int n; @@ -2596,9 +2613,11 @@ show_sort_info(SortState *sortstate, ExplainState *es) if (es->format == EXPLAIN_FORMAT_TEXT) { appendStringInfoSpaces(es->str, es->indent * 2); + if (n > 0 || !es->hide_workers) + appendStringInfo(es->str, "Worker %d: ", n); appendStringInfo(es->str, - "Worker %d: Sort Method: %s %s: %ldkB\n", - n, sortMethod, spaceType, spaceUsed); + "Sort Method: %s %s: %ldkB\n", + sortMethod, spaceType, spaceUsed); } else { diff --git a/src/include/commands/explain.h b/src/include/commands/explain.h index 8639891c164..e4f4b704dfa 100644 --- a/src/include/commands/explain.h +++ b/src/include/commands/explain.h @@ -46,6 +46,7 @@ typedef struct ExplainState List *rtable_names; /* alias names for RTEs */ List *deparse_cxt; /* context list for deparsing expressions */ Bitmapset *printed_subplans; /* ids of SubPlans we've printed */ + bool hide_workers; /* set if we find an invisible Gather */ } ExplainState; /* Hook for plugins to get control in ExplainOneQuery() */ diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out index 8219abc813e..424d51d5216 100644 --- a/src/test/regress/expected/partition_prune.out +++ b/src/test/regress/expected/partition_prune.out @@ -3163,12 +3163,12 @@ execute mt_q1(35); deallocate mt_q1; prepare mt_q2 (int) as select * from ma_test where a >= $1 order by b limit 1; -- Ensure output list looks sane when the MergeAppend has no subplans. -explain (verbose, costs off) execute mt_q2 (35); - QUERY PLAN --------------------------------- - Limit +explain (analyze, verbose, costs off, summary off, timing off) execute mt_q2 (35); + QUERY PLAN +-------------------------------------------- + Limit (actual rows=0 loops=1) Output: ma_test.a, ma_test.b - -> Merge Append + -> Merge Append (actual rows=0 loops=1) Sort Key: ma_test.b Subplans Removed: 3 (5 rows) diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out index ee9c5db0d51..547686166a6 100644 --- a/src/test/regress/expected/subselect.out +++ b/src/test/regress/expected/subselect.out @@ -1166,8 +1166,6 @@ begin select * from (select pk,c2 from sq_limit order by c1,pk) as x limit 3 loop ln := regexp_replace(ln, 'Memory: \S*', 'Memory: xxx'); - -- this case might occur if force_parallel_mode is on: - ln := regexp_replace(ln, 'Worker 0: Sort Method', 'Sort Method'); return next ln; end loop; end; diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sql index 61ef6e637e4..d9daba3af30 100644 --- a/src/test/regress/sql/partition_prune.sql +++ b/src/test/regress/sql/partition_prune.sql @@ -841,7 +841,7 @@ deallocate mt_q1; prepare mt_q2 (int) as select * from ma_test where a >= $1 order by b limit 1; -- Ensure output list looks sane when the MergeAppend has no subplans. -explain (verbose, costs off) execute mt_q2 (35); +explain (analyze, verbose, costs off, summary off, timing off) execute mt_q2 (35); deallocate mt_q2; diff --git a/src/test/regress/sql/subselect.sql b/src/test/regress/sql/subselect.sql index 20c4f99c9c4..3e119ce1274 100644 --- a/src/test/regress/sql/subselect.sql +++ b/src/test/regress/sql/subselect.sql @@ -631,8 +631,6 @@ begin select * from (select pk,c2 from sq_limit order by c1,pk) as x limit 3 loop ln := regexp_replace(ln, 'Memory: \S*', 'Memory: xxx'); - -- this case might occur if force_parallel_mode is on: - ln := regexp_replace(ln, 'Worker 0: Sort Method', 'Sort Method'); return next ln; end loop; end;