Rearrange explain.c's API so callers need not embed sizeof(ExplainState).

The folly of the previous arrangement was just demonstrated: there's no
convenient way to add fields to ExplainState without breaking ABI, even
if callers have no need to touch those fields.  Since we might well need
to do that again someday in back branches, let's change things so that
only explain.c has to have sizeof(ExplainState) compiled into it.  This
costs one extra palloc() per EXPLAIN operation, which is surely pretty
negligible.

diff --git a/contrib/auto_explain/auto_explain.c b/contrib/auto_explain/auto_explain.c
index 179b33a79387c1230e7009ca0e8b7d9bca9a3a8d..2a184ed886fa4e13d3d0a179d831139518c4f757 100644 (file)
--- a/contrib/auto_explain/auto_explain.c
+++ b/contrib/auto_explain/auto_explain.c
@@ -294,32 +294,31 @@ explain_ExecutorEnd(QueryDesc *queryDesc)
        msec = queryDesc->totaltime->total * 1000.0;
        if (msec >= auto_explain_log_min_duration)
        {
-           ExplainState es;
-
-           ExplainInitState(&es);
-           es.analyze = (queryDesc->instrument_options && auto_explain_log_analyze);
-           es.verbose = auto_explain_log_verbose;
-           es.buffers = (es.analyze && auto_explain_log_buffers);
-           es.timing = (es.analyze && auto_explain_log_timing);
-           es.summary = es.analyze;
-           es.format = auto_explain_log_format;
-
-           ExplainBeginOutput(&es);
-           ExplainQueryText(&es, queryDesc);
-           ExplainPrintPlan(&es, queryDesc);
-           if (es.analyze && auto_explain_log_triggers)
-               ExplainPrintTriggers(&es, queryDesc);
-           ExplainEndOutput(&es);
+           ExplainState *es = NewExplainState();
+
+           es->analyze = (queryDesc->instrument_options && auto_explain_log_analyze);
+           es->verbose = auto_explain_log_verbose;
+           es->buffers = (es->analyze && auto_explain_log_buffers);
+           es->timing = (es->analyze && auto_explain_log_timing);
+           es->summary = es->analyze;
+           es->format = auto_explain_log_format;
+
+           ExplainBeginOutput(es);
+           ExplainQueryText(es, queryDesc);
+           ExplainPrintPlan(es, queryDesc);
+           if (es->analyze && auto_explain_log_triggers)
+               ExplainPrintTriggers(es, queryDesc);
+           ExplainEndOutput(es);
 
            /* Remove last line break */
-           if (es.str->len > 0 && es.str->data[es.str->len - 1] == '\n')
-               es.str->data[--es.str->len] = '\0';
+           if (es->str->len > 0 && es->str->data[es->str->len - 1] == '\n')
+               es->str->data[--es->str->len] = '\0';
 
            /* Fix JSON to output an object */
            if (auto_explain_log_format == EXPLAIN_FORMAT_JSON)
            {
-               es.str->data[0] = '{';
-               es.str->data[es.str->len - 1] = '}';
+               es->str->data[0] = '{';
+               es->str->data[es->str->len - 1] = '}';
            }
 
            /*
@@ -330,10 +329,10 @@ explain_ExecutorEnd(QueryDesc *queryDesc)
             */
            ereport(LOG,
                    (errmsg("duration: %.3f ms  plan:\n%s",
-                           msec, es.str->data),
+                           msec, es->str->data),
                     errhidestmt(true)));
 
-           pfree(es.str->data);
+           pfree(es->str->data);
        }
    }
 

diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 39ceaf266391801421caaf2ad19dae5c66113519..2b84ac8af3724535dbc4140aa606e2ca15514b6a 100644 (file)
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -125,45 +125,42 @@ void
 ExplainQuery(ExplainStmt *stmt, const char *queryString,
             ParamListInfo params, DestReceiver *dest)
 {
-   ExplainState es;
+   ExplainState *es = NewExplainState();
    TupOutputState *tstate;
    List       *rewritten;
    ListCell   *lc;
    bool        timing_set = false;
 
-   /* Initialize ExplainState. */
-   ExplainInitState(&es);
-
    /* Parse options list. */
    foreach(lc, stmt->options)
    {
        DefElem    *opt = (DefElem *) lfirst(lc);
 
        if (strcmp(opt->defname, "analyze") == 0)
-           es.analyze = defGetBoolean(opt);
+           es->analyze = defGetBoolean(opt);
        else if (strcmp(opt->defname, "verbose") == 0)
-           es.verbose = defGetBoolean(opt);
+           es->verbose = defGetBoolean(opt);
        else if (strcmp(opt->defname, "costs") == 0)
-           es.costs = defGetBoolean(opt);
+           es->costs = defGetBoolean(opt);
        else if (strcmp(opt->defname, "buffers") == 0)
-           es.buffers = defGetBoolean(opt);
+           es->buffers = defGetBoolean(opt);
        else if (strcmp(opt->defname, "timing") == 0)
        {
            timing_set = true;
-           es.timing = defGetBoolean(opt);
+           es->timing = defGetBoolean(opt);
        }
        else if (strcmp(opt->defname, "format") == 0)
        {
            char       *p = defGetString(opt);
 
            if (strcmp(p, "text") == 0)
-               es.format = EXPLAIN_FORMAT_TEXT;
+               es->format = EXPLAIN_FORMAT_TEXT;
            else if (strcmp(p, "xml") == 0)
-               es.format = EXPLAIN_FORMAT_XML;
+               es->format = EXPLAIN_FORMAT_XML;
            else if (strcmp(p, "json") == 0)
-               es.format = EXPLAIN_FORMAT_JSON;
+               es->format = EXPLAIN_FORMAT_JSON;
            else if (strcmp(p, "yaml") == 0)
-               es.format = EXPLAIN_FORMAT_YAML;
+               es->format = EXPLAIN_FORMAT_YAML;
            else
                ereport(ERROR,
                        (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
@@ -177,22 +174,22 @@ ExplainQuery(ExplainStmt *stmt, const char *queryString,
                            opt->defname)));
    }
 
-   if (es.buffers && !es.analyze)
+   if (es->buffers && !es->analyze)
        ereport(ERROR,
                (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
                 errmsg("EXPLAIN option BUFFERS requires ANALYZE")));
 
    /* if the timing was not set explicitly, set default value */
-   es.timing = (timing_set) ? es.timing : es.analyze;
+   es->timing = (timing_set) ? es->timing : es->analyze;
 
    /* check that timing is used with EXPLAIN ANALYZE */
-   if (es.timing && !es.analyze)
+   if (es->timing && !es->analyze)
        ereport(ERROR,
                (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
                 errmsg("EXPLAIN option TIMING requires ANALYZE")));
 
    /* currently, summary option is not exposed to users; just set it */
-   es.summary = es.analyze;
+   es->summary = es->analyze;
 
    /*
     * Parse analysis was done already, but we still have to run the rule
@@ -210,7 +207,7 @@ ExplainQuery(ExplainStmt *stmt, const char *queryString,
    rewritten = QueryRewrite((Query *) copyObject(stmt->query));
 
    /* emit opening boilerplate */
-   ExplainBeginOutput(&es);
+   ExplainBeginOutput(es);
 
    if (rewritten == NIL)
    {
@@ -218,8 +215,8 @@ ExplainQuery(ExplainStmt *stmt, const char *queryString,
         * In the case of an INSTEAD NOTHING, tell at least that.  But in
         * non-text format, the output is delimited, so this isn't necessary.
         */
-       if (es.format == EXPLAIN_FORMAT_TEXT)
-           appendStringInfoString(es.str, "Query rewrites to nothing\n");
+       if (es->format == EXPLAIN_FORMAT_TEXT)
+           appendStringInfoString(es->str, "Query rewrites to nothing\n");
    }
    else
    {
@@ -228,41 +225,44 @@ ExplainQuery(ExplainStmt *stmt, const char *queryString,
        /* Explain every plan */
        foreach(l, rewritten)
        {
-           ExplainOneQuery((Query *) lfirst(l), NULL, &es,
+           ExplainOneQuery((Query *) lfirst(l), NULL, es,
                            queryString, params);
 
            /* Separate plans with an appropriate separator */
            if (lnext(l) != NULL)
-               ExplainSeparatePlans(&es);
+               ExplainSeparatePlans(es);
        }
    }
 
    /* emit closing boilerplate */
-   ExplainEndOutput(&es);
-   Assert(es.indent == 0);
+   ExplainEndOutput(es);
+   Assert(es->indent == 0);
 
    /* output tuples */
    tstate = begin_tup_output_tupdesc(dest, ExplainResultDesc(stmt));
-   if (es.format == EXPLAIN_FORMAT_TEXT)
-       do_text_output_multiline(tstate, es.str->data);
+   if (es->format == EXPLAIN_FORMAT_TEXT)
+       do_text_output_multiline(tstate, es->str->data);
    else
-       do_text_output_oneline(tstate, es.str->data);
+       do_text_output_oneline(tstate, es->str->data);
    end_tup_output(tstate);
 
-   pfree(es.str->data);
+   pfree(es->str->data);
 }
 
 /*
- * Initialize ExplainState.
+ * Create a new ExplainState struct initialized with default options.
  */
-void
-ExplainInitState(ExplainState *es)
+ExplainState *
+NewExplainState(void)
 {
-   /* Set default options. */
-   memset(es, 0, sizeof(ExplainState));
+   ExplainState *es = (ExplainState *) palloc0(sizeof(ExplainState));
+
+   /* Set default options (most fields can be left as zeroes). */
    es->costs = true;
    /* Prepare output buffer. */
    es->str = makeStringInfo();
+
+   return es;
 }
 
 /*

diff --git a/src/include/commands/explain.h b/src/include/commands/explain.h
index 598f37428d26d67fed7bb708afb981ab683e282c..c9f722361a128f819504a6f62ec4d18699828958 100644 (file)
--- a/src/include/commands/explain.h
+++ b/src/include/commands/explain.h
@@ -60,7 +60,7 @@ extern PGDLLIMPORT explain_get_index_name_hook_type explain_get_index_name_hook;
 extern void ExplainQuery(ExplainStmt *stmt, const char *queryString,
             ParamListInfo params, DestReceiver *dest);
 
-extern void ExplainInitState(ExplainState *es);
+extern ExplainState *NewExplainState(void);
 
 extern TupleDesc ExplainResultDesc(ExplainStmt *stmt);