Re: Make EXPLAIN generate a generic plan for a parameterized query - Mailing list pgsql-hackers

Laurenz Albe <laurenz.albe@cybertec.at> writes:
> [ 0001-Add-EXPLAIN-option-GENERIC_PLAN.v4.patch ]

I took a closer look at this patch, and didn't like the implementation
much.  You're not matching the behavior of PREPARE at all: for example,
this patch is content to let $1 be resolved with different types in
different places.  We should be using the existing infrastructure that
parse_analyze_varparams uses.

Also, I believe that in contexts such as plpgsql, it is possible that
there's an external source of $N definitions, which we should probably
continue to honor even with GENERIC_PLAN.

So that leads me to think the code should be more like this.  I'm not
sure if it's worth spending documentation and testing effort on the
case where we don't override an existing p_paramref_hook.

            regards, tom lane

diff --git a/doc/src/sgml/ref/explain.sgml b/doc/src/sgml/ref/explain.sgml
index d4895b9d7d..58350624e7 100644
--- a/doc/src/sgml/ref/explain.sgml
+++ b/doc/src/sgml/ref/explain.sgml
@@ -40,6 +40,7 @@ EXPLAIN [ ANALYZE ] [ VERBOSE ] <replaceable class="parameter">statement</replac
     VERBOSE [ <replaceable class="parameter">boolean</replaceable> ]
     COSTS [ <replaceable class="parameter">boolean</replaceable> ]
     SETTINGS [ <replaceable class="parameter">boolean</replaceable> ]
+    GENERIC_PLAN [ <replaceable class="parameter">boolean</replaceable> ]
     BUFFERS [ <replaceable class="parameter">boolean</replaceable> ]
     WAL [ <replaceable class="parameter">boolean</replaceable> ]
     TIMING [ <replaceable class="parameter">boolean</replaceable> ]
@@ -167,6 +168,20 @@ ROLLBACK;
     </listitem>
    </varlistentry>

+   <varlistentry>
+    <term><literal>GENERIC_PLAN</literal></term>
+    <listitem>
+     <para>
+      Generate a generic plan for the statement (see <xref linkend="sql-prepare"/>
+      for details about generic plans).  The statement can contain parameter
+      placeholders like <literal>$1</literal>, if it is a statement that can
+      use parameters.  This option cannot be used together with
+      <literal>ANALYZE</literal>, since a statement with unknown parameters
+      cannot be executed.
+     </para>
+    </listitem>
+   </varlistentry>
+
    <varlistentry>
     <term><literal>BUFFERS</literal></term>
     <listitem>
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 35c23bd27d..ab21a67862 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -190,6 +190,8 @@ ExplainQuery(ParseState *pstate, ExplainStmt *stmt,
             es->wal = defGetBoolean(opt);
         else if (strcmp(opt->defname, "settings") == 0)
             es->settings = defGetBoolean(opt);
+        else if (strcmp(opt->defname, "generic_plan") == 0)
+            es->generic = defGetBoolean(opt);
         else if (strcmp(opt->defname, "timing") == 0)
         {
             timing_set = true;
@@ -227,6 +229,13 @@ ExplainQuery(ParseState *pstate, ExplainStmt *stmt,
                      parser_errposition(pstate, opt->location)));
     }

+    /* check that GENERIC_PLAN is not used with EXPLAIN ANALYZE */
+    if (es->generic && es->analyze)
+        ereport(ERROR,
+                (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                 errmsg("EXPLAIN ANALYZE cannot be used with GENERIC_PLAN")));
+
+    /* check that WAL is used with EXPLAIN ANALYZE */
     if (es->wal && !es->analyze)
         ereport(ERROR,
                 (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index e892df9819..9143964e78 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -27,6 +27,7 @@
 #include "access/sysattr.h"
 #include "catalog/pg_proc.h"
 #include "catalog/pg_type.h"
+#include "commands/defrem.h"
 #include "miscadmin.h"
 #include "nodes/makefuncs.h"
 #include "nodes/nodeFuncs.h"
@@ -2906,10 +2907,38 @@ static Query *
 transformExplainStmt(ParseState *pstate, ExplainStmt *stmt)
 {
     Query       *result;
+    bool        generic_plan = false;
+    Oid           *paramTypes = NULL;
+    int            numParams = 0;
+
+    /*
+     * If we have no external source of parameter definitions, and the
+     * GENERIC_PLAN option is specified, then accept variable parameter
+     * definitions (as occurs in PREPARE, for example).
+     */
+    if (pstate->p_paramref_hook == NULL)
+    {
+        ListCell   *lc;
+
+        foreach(lc, stmt->options)
+        {
+            DefElem    *opt = (DefElem *) lfirst(lc);
+
+            if (strcmp(opt->defname, "generic_plan") == 0)
+                generic_plan = defGetBoolean(opt);
+            /* don't "break", as we want the last value */
+        }
+        if (generic_plan)
+            setup_parse_variable_parameters(pstate, ¶mTypes, &numParams);
+    }

     /* transform contained query, allowing SELECT INTO */
     stmt->query = (Node *) transformOptionalSelectInto(pstate, stmt->query);

+    /* make sure all is well with parameter types */
+    if (generic_plan)
+        check_variable_parameters(pstate, (Query *) stmt->query);
+
     /* represent the command as a utility Query */
     result = makeNode(Query);
     result->commandType = CMD_UTILITY;
diff --git a/src/include/commands/explain.h b/src/include/commands/explain.h
index 7c1071ddd1..3d3e632a0c 100644
--- a/src/include/commands/explain.h
+++ b/src/include/commands/explain.h
@@ -46,6 +46,7 @@ typedef struct ExplainState
     bool        timing;            /* print detailed node timing */
     bool        summary;        /* print total planning and execution timing */
     bool        settings;        /* print modified settings */
+    bool        generic;        /* generate a generic plan */
     ExplainFormat format;        /* output format */
     /* state for output formatting --- not reset for each new plan tree */
     int            indent;            /* current indentation level */
diff --git a/src/test/regress/expected/explain.out b/src/test/regress/expected/explain.out
index 48620edbc2..18f7ac93c4 100644
--- a/src/test/regress/expected/explain.out
+++ b/src/test/regress/expected/explain.out
@@ -517,3 +517,17 @@ select explain_filter('explain (verbose) select * from int8_tbl i8');
  Query Identifier: N
 (3 rows)

+-- Test EXPLAIN (GENERIC_PLAN)
+select explain_filter('explain (generic_plan) select unique1 from tenk1 where thousand = $1');
+                                 explain_filter
+---------------------------------------------------------------------------------
+ Bitmap Heap Scan on tenk1  (cost=N.N..N.N rows=N width=N)
+   Recheck Cond: (thousand = $N)
+   ->  Bitmap Index Scan on tenk1_thous_tenthous  (cost=N.N..N.N rows=N width=N)
+         Index Cond: (thousand = $N)
+(4 rows)
+
+-- should fail
+select explain_filter('explain (analyze, generic_plan) select unique1 from tenk1 where thousand = $1');
+ERROR:  EXPLAIN ANALYZE cannot be used with GENERIC_PLAN
+CONTEXT:  PL/pgSQL function explain_filter(text) line 5 at FOR over EXECUTE statement
diff --git a/src/test/regress/sql/explain.sql b/src/test/regress/sql/explain.sql
index ae3f7a308d..fce031775a 100644
--- a/src/test/regress/sql/explain.sql
+++ b/src/test/regress/sql/explain.sql
@@ -128,3 +128,8 @@ select explain_filter('explain (verbose) select * from t1 where pg_temp.mysin(f1
 -- Test compute_query_id
 set compute_query_id = on;
 select explain_filter('explain (verbose) select * from int8_tbl i8');
+
+-- Test EXPLAIN (GENERIC_PLAN)
+select explain_filter('explain (generic_plan) select unique1 from tenk1 where thousand = $1');
+-- should fail
+select explain_filter('explain (analyze, generic_plan) select unique1 from tenk1 where thousand = $1');