Skip to content

Commit 8ebb69f

Browse files
tglsfdctglsfdc
committed
Fix some infelicities in EXPLAIN output for parallel query plans.
In non-text output formats, parallelized aggregates were reporting "Partial" or "Finalize" as a field named "Operation", which might be all right in the absence of any context --- but other plan node types use that field to report SQL-visible semantics, such as Select/Insert/Update/Delete. So that naming choice didn't seem good to me. I changed it to "Partial Mode". Also, the field did not appear at all for a non-parallelized Agg plan node, which is contrary to expectation in non-text formats. We're notionally producing objects that conform to a schema, so the set of fields for a given node type and EXPLAIN mode should be well-defined. I set it up to fill in "Simple" in such cases. Other fields that were added for parallel query, namely "Parallel Aware" and Gather's "Single Copy", had not gotten the word on that point either. Make them appear always in non-text output. Also, the latter two fields were nominally producing boolean output, but were getting it wrong, because bool values shouldn't be quoted in JSON or YAML. Somehow we'd not needed an ExplainPropertyBool formatting subroutine before 9.6; but now we do, so invent it. Discussion: <16002.1466972724@sss.pgh.pa.us>
1 parent 0584df3 commit 8ebb69f

File tree

3 files changed

+33
-15
lines changed

3 files changed

+33
-15
lines changed

‎src/backend/commands/explain.c

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -807,6 +807,7 @@ ExplainNode(PlanState *planstate, List *ancestors,
807807
const char *pname; /* node type name for text output */
808808
const char *sname; /* node type name for non-text output */
809809
const char *strategy = NULL;
810+
const char *partialmode = NULL;
810811
const char *operation = NULL;
811812
const char *custom_name = NULL;
812813
int save_indent = es->indent;
@@ -943,15 +944,10 @@ ExplainNode(PlanState *planstate, List *ancestors,
943944
pname = sname = "Group";
944945
break;
945946
case T_Agg:
946-
sname = "Aggregate";
947947
{
948948
Agg *agg = (Agg *) plan;
949949

950-
if (DO_AGGSPLIT_SKIPFINAL(agg->aggsplit))
951-
operation = "Partial";
952-
else if (DO_AGGSPLIT_COMBINE(agg->aggsplit))
953-
operation = "Finalize";
954-
950+
sname = "Aggregate";
955951
switch (agg->aggstrategy)
956952
{
957953
case AGG_PLAIN:
@@ -972,8 +968,18 @@ ExplainNode(PlanState *planstate, List *ancestors,
972968
break;
973969
}
974970

975-
if (operation != NULL)
976-
pname = psprintf("%s %s", operation, pname);
971+
if (DO_AGGSPLIT_SKIPFINAL(agg->aggsplit))
972+
{
973+
partialmode = "Partial";
974+
pname = psprintf("%s %s", partialmode, pname);
975+
}
976+
else if (DO_AGGSPLIT_COMBINE(agg->aggsplit))
977+
{
978+
partialmode = "Finalize";
979+
pname = psprintf("%s %s", partialmode, pname);
980+
}
981+
else
982+
partialmode = "Simple";
977983
}
978984
break;
979985
case T_WindowAgg:
@@ -1042,6 +1048,8 @@ ExplainNode(PlanState *planstate, List *ancestors,
10421048
ExplainPropertyText("Node Type", sname, es);
10431049
if (strategy)
10441050
ExplainPropertyText("Strategy", strategy, es);
1051+
if (partialmode)
1052+
ExplainPropertyText("Partial Mode", partialmode, es);
10451053
if (operation)
10461054
ExplainPropertyText("Operation", operation, es);
10471055
if (relationship)
@@ -1050,8 +1058,7 @@ ExplainNode(PlanState *planstate, List *ancestors,
10501058
ExplainPropertyText("Subplan Name", plan_name, es);
10511059
if (custom_name)
10521060
ExplainPropertyText("Custom Plan Provider", custom_name, es);
1053-
if (plan->parallel_aware)
1054-
ExplainPropertyText("Parallel Aware", "true", es);
1061+
ExplainPropertyBool("Parallel Aware", plan->parallel_aware, es);
10551062
}
10561063

10571064
switch (nodeTag(plan))
@@ -1349,10 +1356,8 @@ ExplainNode(PlanState *planstate, List *ancestors,
13491356
ExplainPropertyInteger("Workers Launched",
13501357
nworkers, es);
13511358
}
1352-
if (gather->single_copy)
1353-
ExplainPropertyText("Single Copy",
1354-
gather->single_copy ? "true" : "false",
1355-
es);
1359+
if (gather->single_copy || es->format != EXPLAIN_FORMAT_TEXT)
1360+
ExplainPropertyBool("Single Copy", gather->single_copy, es);
13561361
}
13571362
break;
13581363
case T_FunctionScan:
@@ -3031,6 +3036,15 @@ ExplainPropertyFloat(const char *qlabel, double value, int ndigits,
30313036
ExplainProperty(qlabel, buf, true, es);
30323037
}
30333038

3039+
/*
3040+
* Explain a bool-valued property.
3041+
*/
3042+
void
3043+
ExplainPropertyBool(const char *qlabel, bool value, ExplainState *es)
3044+
{
3045+
ExplainProperty(qlabel, value ? "true" : "false", true, es);
3046+
}
3047+
30343048
/*
30353049
* Open a group of related objects.
30363050
*

‎src/include/commands/explain.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,5 +93,7 @@ extern void ExplainPropertyLong(const char *qlabel, long value,
9393
ExplainState *es);
9494
extern void ExplainPropertyFloat(const char *qlabel, double value, int ndigits,
9595
ExplainState *es);
96+
extern void ExplainPropertyBool(const char *qlabel, bool value,
97+
ExplainState *es);
9698

9799
#endif /* EXPLAIN_H */

‎src/test/regress/expected/insert_conflict.out

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,7 @@ explain (costs off, format json) insert into insertconflicttest values (0, 'Bilb
205205
"Plan": { +
206206
"Node Type": "ModifyTable", +
207207
"Operation": "Insert", +
208+
"Parallel Aware": false, +
208209
"Relation Name": "insertconflicttest", +
209210
"Alias": "insertconflicttest", +
210211
"Conflict Resolution": "UPDATE", +
@@ -213,7 +214,8 @@ explain (costs off, format json) insert into insertconflicttest values (0, 'Bilb
213214
"Plans": [ +
214215
{ +
215216
"Node Type": "Result", +
216-
"Parent Relationship": "Member" +
217+
"Parent Relationship": "Member", +
218+
"Parallel Aware": false +
217219
} +
218220
] +
219221
} +

0 commit comments

Comments
 (0)