]> granicus.if.org Git - postgresql/commitdiff
Fix some infelicities in EXPLAIN output for parallel query plans.
authorTom Lane <tgl@sss.pgh.pa.us>
Wed, 29 Jun 2016 22:51:20 +0000 (18:51 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Wed, 29 Jun 2016 22:51:20 +0000 (18:51 -0400)
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>

src/backend/commands/explain.c
src/include/commands/explain.h
src/test/regress/expected/insert_conflict.out

index 319dd8e22481602bf22296ae93af221c0299ab1c..9873022bf826ec95b255945a3b8b8ef9e37a0c2c 100644 (file)
@@ -807,6 +807,7 @@ ExplainNode(PlanState *planstate, List *ancestors,
        const char *pname;                      /* node type name for text output */
        const char *sname;                      /* node type name for non-text output */
        const char *strategy = NULL;
+       const char *partialmode = NULL;
        const char *operation = NULL;
        const char *custom_name = NULL;
        int                     save_indent = es->indent;
@@ -943,15 +944,10 @@ ExplainNode(PlanState *planstate, List *ancestors,
                        pname = sname = "Group";
                        break;
                case T_Agg:
-                       sname = "Aggregate";
                        {
                                Agg                *agg = (Agg *) plan;
 
-                               if (DO_AGGSPLIT_SKIPFINAL(agg->aggsplit))
-                                       operation = "Partial";
-                               else if (DO_AGGSPLIT_COMBINE(agg->aggsplit))
-                                       operation = "Finalize";
-
+                               sname = "Aggregate";
                                switch (agg->aggstrategy)
                                {
                                        case AGG_PLAIN:
@@ -972,8 +968,18 @@ ExplainNode(PlanState *planstate, List *ancestors,
                                                break;
                                }
 
-                               if (operation != NULL)
-                                       pname = psprintf("%s %s", operation, pname);
+                               if (DO_AGGSPLIT_SKIPFINAL(agg->aggsplit))
+                               {
+                                       partialmode = "Partial";
+                                       pname = psprintf("%s %s", partialmode, pname);
+                               }
+                               else if (DO_AGGSPLIT_COMBINE(agg->aggsplit))
+                               {
+                                       partialmode = "Finalize";
+                                       pname = psprintf("%s %s", partialmode, pname);
+                               }
+                               else
+                                       partialmode = "Simple";
                        }
                        break;
                case T_WindowAgg:
@@ -1042,6 +1048,8 @@ ExplainNode(PlanState *planstate, List *ancestors,
                ExplainPropertyText("Node Type", sname, es);
                if (strategy)
                        ExplainPropertyText("Strategy", strategy, es);
+               if (partialmode)
+                       ExplainPropertyText("Partial Mode", partialmode, es);
                if (operation)
                        ExplainPropertyText("Operation", operation, es);
                if (relationship)
@@ -1050,8 +1058,7 @@ ExplainNode(PlanState *planstate, List *ancestors,
                        ExplainPropertyText("Subplan Name", plan_name, es);
                if (custom_name)
                        ExplainPropertyText("Custom Plan Provider", custom_name, es);
-               if (plan->parallel_aware)
-                       ExplainPropertyText("Parallel Aware", "true", es);
+               ExplainPropertyBool("Parallel Aware", plan->parallel_aware, es);
        }
 
        switch (nodeTag(plan))
@@ -1349,10 +1356,8 @@ ExplainNode(PlanState *planstate, List *ancestors,
                                        ExplainPropertyInteger("Workers Launched",
                                                                                   nworkers, es);
                                }
-                               if (gather->single_copy)
-                                       ExplainPropertyText("Single Copy",
-                                                                         gather->single_copy ? "true" : "false",
-                                                                               es);
+                               if (gather->single_copy || es->format != EXPLAIN_FORMAT_TEXT)
+                                       ExplainPropertyBool("Single Copy", gather->single_copy, es);
                        }
                        break;
                case T_FunctionScan:
@@ -3031,6 +3036,15 @@ ExplainPropertyFloat(const char *qlabel, double value, int ndigits,
        ExplainProperty(qlabel, buf, true, es);
 }
 
+/*
+ * Explain a bool-valued property.
+ */
+void
+ExplainPropertyBool(const char *qlabel, bool value, ExplainState *es)
+{
+       ExplainProperty(qlabel, value ? "true" : "false", true, es);
+}
+
 /*
  * Open a group of related objects.
  *
index 904925d7123394427b895f853bde10c634b57db5..2e48f0f2331240525af1174a68da6093cea3d8ab 100644 (file)
@@ -93,5 +93,7 @@ extern void ExplainPropertyLong(const char *qlabel, long value,
                                        ExplainState *es);
 extern void ExplainPropertyFloat(const char *qlabel, double value, int ndigits,
                                         ExplainState *es);
+extern void ExplainPropertyBool(const char *qlabel, bool value,
+                                       ExplainState *es);
 
 #endif   /* EXPLAIN_H */
index e5c8b4aaacb1ddc59b5cb6bb97e6129e3556e5b2..6f4ec867d9f48244e82a23ab80816abac9abd579 100644 (file)
@@ -205,6 +205,7 @@ explain (costs off, format json) insert into insertconflicttest values (0, 'Bilb
      "Plan": {                                                         +
        "Node Type": "ModifyTable",                                     +
        "Operation": "Insert",                                          +
+       "Parallel Aware": false,                                        +
        "Relation Name": "insertconflicttest",                          +
        "Alias": "insertconflicttest",                                  +
        "Conflict Resolution": "UPDATE",                                +
@@ -213,7 +214,8 @@ explain (costs off, format json) insert into insertconflicttest values (0, 'Bilb
        "Plans": [                                                      +
          {                                                             +
            "Node Type": "Result",                                      +
-           "Parent Relationship": "Member"                             +
+           "Parent Relationship": "Member",                            +
+           "Parallel Aware": false                                     +
          }                                                             +
        ]                                                               +
      }                                                                 +