]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Fix handling of "Subplans Removed" field in EXPLAIN output.
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 4 Feb 2020 18:07:13 +0000 (13:07 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 4 Feb 2020 18:07:13 +0000 (13:07 -0500)
Commit 499be013d added this field in a rather poorly-thought-through
manner, with the result being that rather than being a field of the
Append or MergeAppend plan node as intended (and as it seems to be,
in text format), it was actually an element of the "Plans" subgroup.
At least in JSON format, that's flat out invalid syntax, because
"Plans" is an array not an object.

While it's not hard to move the generation of the field so that it
appears where it's supposed to, this does result in a visible change
in field order in text format, in cases where a Append or MergeAppend
plan node has any InitPlans attached.  That's slightly annoying to
do in stable branches; but the alternative of continuing to emit
broken non-text formats seems worse.

Also, since the set of fields emitted is not supposed to be
data-dependent in non-text formats, make sure that "Subplans Removed"
appears in Append and MergeAppend nodes even when it's zero, in those
formats.  (The previous coding made it look like it could appear in
some other node types such as BitmapAnd, but we don't actually support
runtime pruning there, so don't emit it in those cases.)

Per bug #16171 from Mahadevan Ramachandran.  Fix by Daniel Gustafsson
and Tom Lane, reviewed by Hamid Akhtar.  Back-patch to v11 where this
code came in.

Discussion: https://postgr.es/m/16171-b72259ab75505fa2@postgresql.org

src/backend/commands/explain.c
src/test/regress/expected/partition_prune.out

index 799a22e9d5537a6ce97a4512fe9321c9b2c4a0a9..031ab54180809d8d5d4c457cce914fa05ff2ce13 100644 (file)
@@ -118,8 +118,9 @@ static void ExplainModifyTarget(ModifyTable *plan, ExplainState *es);
 static void ExplainTargetRel(Plan *plan, Index rti, ExplainState *es);
 static void show_modifytable_info(ModifyTableState *mtstate, List *ancestors,
                                          ExplainState *es);
-static void ExplainMemberNodes(PlanState **planstates, int nsubnodes,
-                                  int nplans, List *ancestors, ExplainState *es);
+static void ExplainMemberNodes(PlanState **planstates, int nplans,
+                                  List *ancestors, ExplainState *es);
+static void ExplainMissingMembers(int nplans, int nchildren, ExplainState *es);
 static void ExplainSubPlans(List *plans, List *ancestors,
                                const char *relationship, ExplainState *es);
 static void ExplainCustomChildren(CustomScanState *css,
@@ -1862,6 +1863,30 @@ ExplainNode(PlanState *planstate, List *ancestors,
                        ExplainCloseGroup("Workers", "Workers", false, es);
        }
 
+       /*
+        * If partition pruning was done during executor initialization, the
+        * number of child plans we'll display below will be less than the number
+        * of subplans that was specified in the plan.  To make this a bit less
+        * mysterious, emit an indication that this happened.  Note that this
+        * field is emitted now because we want it to be a property of the parent
+        * node; it *cannot* be emitted within the Plans sub-node we'll open next.
+        */
+       switch (nodeTag(plan))
+       {
+               case T_Append:
+                       ExplainMissingMembers(((AppendState *) planstate)->as_nplans,
+                                                                 list_length(((Append *) plan)->appendplans),
+                                                                 es);
+                       break;
+               case T_MergeAppend:
+                       ExplainMissingMembers(((MergeAppendState *) planstate)->ms_nplans,
+                                                                 list_length(((MergeAppend *) plan)->mergeplans),
+                                                                 es);
+                       break;
+               default:
+                       break;
+       }
+
        /* Get ready to display the child plans */
        haschildren = planstate->initPlan ||
                outerPlanState(planstate) ||
@@ -1902,31 +1927,26 @@ ExplainNode(PlanState *planstate, List *ancestors,
                case T_ModifyTable:
                        ExplainMemberNodes(((ModifyTableState *) planstate)->mt_plans,
                                                           ((ModifyTableState *) planstate)->mt_nplans,
-                                                          list_length(((ModifyTable *) plan)->plans),
                                                           ancestors, es);
                        break;
                case T_Append:
                        ExplainMemberNodes(((AppendState *) planstate)->appendplans,
                                                           ((AppendState *) planstate)->as_nplans,
-                                                          list_length(((Append *) plan)->appendplans),
                                                           ancestors, es);
                        break;
                case T_MergeAppend:
                        ExplainMemberNodes(((MergeAppendState *) planstate)->mergeplans,
                                                           ((MergeAppendState *) planstate)->ms_nplans,
-                                                          list_length(((MergeAppend *) plan)->mergeplans),
                                                           ancestors, es);
                        break;
                case T_BitmapAnd:
                        ExplainMemberNodes(((BitmapAndState *) planstate)->bitmapplans,
                                                           ((BitmapAndState *) planstate)->nplans,
-                                                          list_length(((BitmapAnd *) plan)->bitmapplans),
                                                           ancestors, es);
                        break;
                case T_BitmapOr:
                        ExplainMemberNodes(((BitmapOrState *) planstate)->bitmapplans,
                                                           ((BitmapOrState *) planstate)->nplans,
-                                                          list_length(((BitmapOr *) plan)->bitmapplans),
                                                           ancestors, es);
                        break;
                case T_SubqueryScan:
@@ -3237,32 +3257,33 @@ show_modifytable_info(ModifyTableState *mtstate, List *ancestors,
  *
  * The ancestors list should already contain the immediate parent of these
  * plans.
-*
-* nsubnodes indicates the number of items in the planstates array.
-* nplans indicates the original number of subnodes in the Plan, some of these
-* may have been pruned by the run-time pruning code.
  */
 static void
-ExplainMemberNodes(PlanState **planstates, int nsubnodes, int nplans,
+ExplainMemberNodes(PlanState **planstates, int nplans,
                                   List *ancestors, ExplainState *es)
 {
        int                     j;
 
-       /*
-        * The number of subnodes being lower than the number of subplans that was
-        * specified in the plan means that some subnodes have been ignored per
-        * instruction for the partition pruning code during the executor
-        * initialization.  To make this a bit less mysterious, we'll indicate
-        * here that this has happened.
-        */
-       if (nsubnodes < nplans)
-               ExplainPropertyInteger("Subplans Removed", NULL, nplans - nsubnodes, es);
-
-       for (j = 0; j < nsubnodes; j++)
+       for (j = 0; j < nplans; j++)
                ExplainNode(planstates[j], ancestors,
                                        "Member", NULL, es);
 }
 
+/*
+ * Report about any pruned subnodes of an Append or MergeAppend node.
+ *
+ * nplans indicates the number of live subplans.
+ * nchildren indicates the original number of subnodes in the Plan;
+ * some of these may have been pruned by the run-time pruning code.
+ */
+static void
+ExplainMissingMembers(int nplans, int nchildren, ExplainState *es)
+{
+       if (nplans < nchildren || es->format != EXPLAIN_FORMAT_TEXT)
+               ExplainPropertyInteger("Subplans Removed", NULL,
+                                                          nchildren - nplans, es);
+}
+
 /*
  * Explain a list of SubPlans (or initPlans, which also use SubPlan nodes).
  *
index 8ae254c673b5b1c2976fb8c81be662cb2457fd08..46a4df8869497deedfed3d79907574bd49957639 100644 (file)
@@ -1887,9 +1887,9 @@ explain (analyze, costs off, summary off, timing off) execute ab_q2 (2, 2);
                        QUERY PLAN                       
 --------------------------------------------------------
  Append (actual rows=0 loops=1)
+   Subplans Removed: 6
    InitPlan 1 (returns $0)
      ->  Result (actual rows=1 loops=1)
-   Subplans Removed: 6
    ->  Seq Scan on ab_a2_b1 (actual rows=0 loops=1)
          Filter: ((a >= $1) AND (a <= $2) AND (b < $0))
    ->  Seq Scan on ab_a2_b2 (actual rows=0 loops=1)
@@ -1930,9 +1930,9 @@ explain (analyze, costs off, summary off, timing off) execute ab_q3 (2, 2);
                        QUERY PLAN                       
 --------------------------------------------------------
  Append (actual rows=0 loops=1)
+   Subplans Removed: 6
    InitPlan 1 (returns $0)
      ->  Result (actual rows=1 loops=1)
-   Subplans Removed: 6
    ->  Seq Scan on ab_a1_b2 (actual rows=0 loops=1)
          Filter: ((b >= $1) AND (b <= $2) AND (a < $0))
    ->  Seq Scan on ab_a2_b2 (actual rows=0 loops=1)