Print a given subplan only once in EXPLAIN.

We have, for a very long time, allowed the same subplan (same member of the
PlannedStmt.subplans list) to be referenced by more than one SubPlan node;
this avoids problems for cases such as subplans within an IndexScan's
indxqual and indxqualorig fields.  However, EXPLAIN had not gotten the memo
and would print each reference as though it were an independent identical
subplan.  To fix, track plan_ids of subplans we've printed and don't print
the same plan_id twice.  Per report from Pavel Stehule.

BTW: the particular case of IndexScan didn't cause visible duplication
in a plain EXPLAIN, only EXPLAIN ANALYZE, because in the former case we
short-circuit executor startup before the indxqual field is processed by
ExecInitExpr.  That seems like it could easily lead to other EXPLAIN
problems in future, but it's not clear how to avoid it without breaking
the "EXPLAIN a plan using hypothetical indexes" use-case.  For now I've
left that issue alone.

Although this is a longstanding bug, it's purely cosmetic (no great harm
is done by the repeat printout) and we haven't had field complaints before.
So I'm hesitant to back-patch it, especially since there is some small risk
of ABI problems due to the need to add a new field to ExplainState.

In passing, rearrange order of fields in ExplainState to be less random,
and update some obsolete comments about when/where to initialize them.

Report: <CAFj8pRAimq+NK-menjt+3J4-LFoodDD8Or6=Lc_stcFD+eD4DA@mail.gmail.com>

diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 9873022bf826ec95b255945a3b8b8ef9e37a0c2c..dbd27e53bc3d49de749251917492002de2417f03 100644 (file)
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -565,8 +565,9 @@ ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es,
  *       convert a QueryDesc's plan tree to text and append it to es->str
  *
  * The caller should have set up the options fields of *es, as well as
- * initializing the output buffer es->str.  Other fields in *es are
- * initialized here.
+ * initializing the output buffer es->str.  Also, output formatting state
+ * such as the indent level is assumed valid.  Plan-tree-specific fields
+ * in *es are initialized here.
  *
  * NB: will not work on utility statements
  */
@@ -576,6 +577,7 @@ ExplainPrintPlan(ExplainState *es, QueryDesc *queryDesc)
        Bitmapset  *rels_used = NULL;
        PlanState  *ps;
 
+       /* Set up ExplainState fields associated with this plan tree */
        Assert(queryDesc->plannedstmt != NULL);
        es->pstmt = queryDesc->plannedstmt;
        es->rtable = queryDesc->plannedstmt->rtable;
@@ -583,6 +585,7 @@ ExplainPrintPlan(ExplainState *es, QueryDesc *queryDesc)
        es->rtable_names = select_rtable_names_for_explain(es->rtable, rels_used);
        es->deparse_cxt = deparse_context_for_plan_rtable(es->rtable,
                                                                                                          es->rtable_names);
+       es->printed_subplans = NULL;
 
        /*
         * Sometimes we mark a Gather node as "invisible", which means that it's
@@ -2798,6 +2801,21 @@ ExplainSubPlans(List *plans, List *ancestors,
                SubPlanState *sps = (SubPlanState *) lfirst(lst);
                SubPlan    *sp = (SubPlan *) sps->xprstate.expr;
 
+               /*
+                * There can be multiple SubPlan nodes referencing the same physical
+                * subplan (same plan_id, which is its index in PlannedStmt.subplans).
+                * We should print a subplan only once, so track which ones we already
+                * printed.  This state must be global across the plan tree, since the
+                * duplicate nodes could be in different plan nodes, eg both a bitmap
+                * indexscan's indexqual and its parent heapscan's recheck qual.  (We
+                * do not worry too much about which plan node we show the subplan as
+                * attached to in such cases.)
+                */
+               if (bms_is_member(sp->plan_id, es->printed_subplans))
+                       continue;
+               es->printed_subplans = bms_add_member(es->printed_subplans,
+                                                                                         sp->plan_id);
+
                ExplainNode(sps->planstate, ancestors,
                                        relationship, sp->plan_name, es);
        }

diff --git a/src/include/commands/explain.h b/src/include/commands/explain.h
index 2e48f0f2331240525af1174a68da6093cea3d8ab..3d0a5abbc2ff5e452668094aadfdc15f1cadf9a1 100644 (file)
--- a/src/include/commands/explain.h
+++ b/src/include/commands/explain.h
@@ -35,13 +35,15 @@ typedef struct ExplainState
        bool            timing;                 /* print detailed node timing */
        bool            summary;                /* print total planning and execution timing */
        ExplainFormat format;           /* output format */
-       /* other states */
+       /* state for output formatting --- not reset for each new plan tree */
+       int                     indent;                 /* current indentation level */
+       List       *grouping_stack; /* format-specific grouping state */
+       /* state related to the current plan tree (filled by ExplainPrintPlan) */
        PlannedStmt *pstmt;                     /* top of plan */
        List       *rtable;                     /* range table */
        List       *rtable_names;       /* alias names for RTEs */
-       int                     indent;                 /* current indentation level */
-       List       *grouping_stack; /* format-specific grouping state */
        List       *deparse_cxt;        /* context list for deparsing expressions */
+       Bitmapset  *printed_subplans;           /* ids of SubPlans we've printed */
 } ExplainState;
 
 /* Hook for plugins to get control in ExplainOneQuery() */