Thanks for the review!
On Tue, 2023-03-21 at 16:32 +0100, Christoph Berg wrote:
> I have some comments:
>
> > This allows EXPLAIN to generate generic plans for parameterized statements
> > (that have parameter placeholders like $1 in the statement text).
>
> > + <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> (but then it has to be a
> > statement
> > + that supports parameters). This option cannot be used together with
> > + <literal>ANALYZE</literal>, since a statement with unknown parameters
> > + cannot be executed.
>
> Like in the commit message quoted above, I would put more emphasis on
> "parameterized query" here:
>
> Allow the statement to contain parameter placeholders like
> <literal>$1</literal> and generate a generic plan for it.
> This option cannot be used together with <literal>ANALYZE</literal>.
I went with
Allow the statement to contain parameter placeholders like
<literal>$1</literal> and generate a generic plan for it.
See <xref linkend="sql-prepare"/> for details about generic plans
and the statements that support parameters.
This option cannot be used together with <literal>ANALYZE</literal>.
> > + /* 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")));
>
> To put that in line with the other error messages in that context, I'd
> inject an extra "option":
>
> errmsg("EXPLAIN option ANALYZE cannot be used with GENERIC_PLAN")));
Done.
> > --- a/src/test/regress/sql/explain.sql
> > +++ b/src/test/regress/sql/explain.sql
> > [...]
> > +create extension if not exists postgres_fdw;
>
> "create extension postgres_fdw" cannot be used from src/test/regress/
> since contrib/ might not have been built.
Ouch. Good catch.
> I suggest leaving this test in place here, but with local tables (to
> show that plan time pruning using the one provided parameter works),
> and add a comment here explaining that is being tested:
>
> -- create a partition hierarchy to show that plan time pruning removes
> -- the key1=2 table but generates a generic plan for key2=$1
I did that, with a different comment.
> The test involving postgres_fdw is still necessary to exercise the new
> EXEC_FLAG_EXPLAIN_GENERIC code path, but needs to be moved elsewhere,
> probably src/test/modules/.
Tests for postgres_fdw are in contrib/postgres_fdw/sql/postgres_fdw.sql,
so I added the test there.
Version 9 of the patch is attached.
Yours,
Laurenz Albe
From 85aa88280069ca2befe7f4308d7e6f724cdb143a Mon Sep 17 00:00:00 2001
From: Laurenz Albe <[email protected]>
Date: Wed, 22 Mar 2023 14:08:49 +0100
Subject: [PATCH] Add EXPLAIN option GENERIC_PLAN
This allows EXPLAIN to generate generic plans for parameterized statements
(that have parameter placeholders like $1 in the statement text).
Invent a new executor flag EXEC_FLAG_EXPLAIN_GENERIC that disables runtime
partition pruning for such plans: that would fail if the non-existing
parameters are involved, and we want to show all subplans anyway.
Author: Laurenz Albe
Reviewed-by: Julien Rouhaud, Christoph Berg, Michel Pelletier, Jim Jones
Discussion: https://postgr.es/m/0a29b954b10b57f0d135fe12aa0909bd41883eb0.camel%40cybertec.at
---
.../postgres_fdw/expected/postgres_fdw.out | 30 +++++++++++++
contrib/postgres_fdw/sql/postgres_fdw.sql | 25 +++++++++++
doc/src/sgml/ref/explain.sgml | 14 +++++++
src/backend/commands/explain.c | 11 +++++
src/backend/executor/execMain.c | 3 ++
src/backend/executor/execPartition.c | 9 ++--
src/backend/parser/analyze.c | 29 +++++++++++++
src/include/commands/explain.h | 1 +
src/include/executor/executor.h | 18 +++++---
src/test/regress/expected/explain.out | 42 +++++++++++++++++++
src/test/regress/sql/explain.sql | 24 +++++++++++
11 files changed, 197 insertions(+), 9 deletions(-)
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 04a3ef450c..25b91ab2e1 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -11783,3 +11783,33 @@ ANALYZE analyze_table;
-- cleanup
DROP FOREIGN TABLE analyze_ftable;
DROP TABLE analyze_table;
+-- ===================================================
+-- test EXPLAIN (GENERIC_PLAN) with foreign partitions
+-- ===================================================
+-- this is needed to exercise the EXEC_FLAG_EXPLAIN_GENERIC flag
+CREATE TABLE gen_part (
+ key1 integer NOT NULL,
+ key2 integer NOT NULL
+) PARTITION BY LIST (key1);
+CREATE TABLE gen_part_1
+ PARTITION OF gen_part FOR VALUES IN (1)
+ PARTITION BY RANGE (key2);
+CREATE FOREIGN TABLE gen_part_1_1
+ PARTITION OF gen_part_1 FOR VALUES FROM (1) TO (2)
+ SERVER testserver1 OPTIONS (table_name 'whatever_1_1');
+CREATE FOREIGN TABLE gen_part_1_2
+ PARTITION OF gen_part_1 FOR VALUES FROM (2) TO (3)
+ SERVER testserver1 OPTIONS (table_name 'whatever_1_2');
+CREATE FOREIGN TABLE gen_part_2
+ PARTITION OF gen_part FOR VALUES IN (2)
+ SERVER testserver1 OPTIONS (table_name 'whatever_2');
+-- this should only scan "gen_part_1_1" and "gen_part_1_2", but not "gen_part_2"
+EXPLAIN (GENERIC_PLAN, COSTS OFF) SELECT key1, key2 FROM gen_part WHERE key1 = 1 AND key2 = $1;
+ QUERY PLAN
+-----------------------------------------------
+ Append
+ -> Foreign Scan on gen_part_1_1 gen_part_1
+ -> Foreign Scan on gen_part_1_2 gen_part_2
+(3 rows)
+
+DROP TABLE gen_part;
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 4f3088c03e..6adc3f2c78 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -3979,3 +3979,28 @@ ANALYZE analyze_table;
-- cleanup
DROP FOREIGN TABLE analyze_ftable;
DROP TABLE analyze_table;
+
+-- ===================================================
+-- test EXPLAIN (GENERIC_PLAN) with foreign partitions
+-- ===================================================
+
+-- this is needed to exercise the EXEC_FLAG_EXPLAIN_GENERIC flag
+CREATE TABLE gen_part (
+ key1 integer NOT NULL,
+ key2 integer NOT NULL
+) PARTITION BY LIST (key1);
+CREATE TABLE gen_part_1
+ PARTITION OF gen_part FOR VALUES IN (1)
+ PARTITION BY RANGE (key2);
+CREATE FOREIGN TABLE gen_part_1_1
+ PARTITION OF gen_part_1 FOR VALUES FROM (1) TO (2)
+ SERVER testserver1 OPTIONS (table_name 'whatever_1_1');
+CREATE FOREIGN TABLE gen_part_1_2
+ PARTITION OF gen_part_1 FOR VALUES FROM (2) TO (3)
+ SERVER testserver1 OPTIONS (table_name 'whatever_1_2');
+CREATE FOREIGN TABLE gen_part_2
+ PARTITION OF gen_part FOR VALUES IN (2)
+ SERVER testserver1 OPTIONS (table_name 'whatever_2');
+-- this should only scan "gen_part_1_1" and "gen_part_1_2", but not "gen_part_2"
+EXPLAIN (GENERIC_PLAN, COSTS OFF) SELECT key1, key2 FROM gen_part WHERE key1 = 1 AND key2 = $1;
+DROP TABLE gen_part;
diff --git a/doc/src/sgml/ref/explain.sgml b/doc/src/sgml/ref/explain.sgml
index 0fce622423..4985545c78 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> ]
@@ -168,6 +169,19 @@ ROLLBACK;
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>GENERIC_PLAN</literal></term>
+ <listitem>
+ <para>
+ Allow the statement to contain parameter placeholders like
+ <literal>$1</literal> and generate a generic plan for it.
+ See <xref linkend="sql-prepare"/> for details about generic plans
+ and the statements that support parameters.
+ This option cannot be used together with <literal>ANALYZE</literal>.
+ </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 e57bda7b62..aaa9783d73 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 option 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),
@@ -574,6 +583,8 @@ ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es,
eflags = EXEC_FLAG_EXPLAIN_ONLY;
if (into)
eflags |= GetIntoRelEFlags(into);
+ if (es->generic)
+ eflags |= EXEC_FLAG_EXPLAIN_GENERIC;
/* call ExecutorStart to prepare the plan for execution */
ExecutorStart(queryDesc, eflags);
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index b32f419176..23ffcbf1aa 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -131,6 +131,9 @@ static void EvalPlanQualStart(EPQState *epqstate, Plan *planTree);
void
ExecutorStart(QueryDesc *queryDesc, int eflags)
{
+ /* EXEC_FLAG_EXPLAIN_GENERIC can only occur with EXEC_FLAG_EXPLAIN_ONLY */
+ Assert((eflags & EXEC_FLAG_EXPLAIN_ONLY) ||
+ !(eflags & EXEC_FLAG_EXPLAIN_GENERIC));
/*
* In some cases (e.g. an EXECUTE statement) a query execution will skip
* parse analysis, which means that the query_id won't be reported. Note
diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c
index fd6ca8a5d9..6333822ff9 100644
--- a/src/backend/executor/execPartition.c
+++ b/src/backend/executor/execPartition.c
@@ -2044,10 +2044,12 @@ CreatePartitionPruneState(PlanState *planstate, PartitionPruneInfo *pruneinfo)
pprune->present_parts = bms_copy(pinfo->present_parts);
/*
- * Initialize pruning contexts as needed.
+ * Initialize pruning contexts as needed. Specifically, we want to
+ * skip execution-time partition pruning for EXPLAIN (GENERIC_PLAN).
*/
pprune->initial_pruning_steps = pinfo->initial_pruning_steps;
- if (pinfo->initial_pruning_steps)
+ if (pinfo->initial_pruning_steps &&
+ !(econtext->ecxt_estate->es_top_eflags & EXEC_FLAG_EXPLAIN_GENERIC))
{
InitPartitionPruneContext(&pprune->initial_context,
pinfo->initial_pruning_steps,
@@ -2057,7 +2059,8 @@ CreatePartitionPruneState(PlanState *planstate, PartitionPruneInfo *pruneinfo)
prunestate->do_initial_prune = true;
}
pprune->exec_pruning_steps = pinfo->exec_pruning_steps;
- if (pinfo->exec_pruning_steps)
+ if (pinfo->exec_pruning_steps &&
+ !(econtext->ecxt_estate->es_top_eflags & EXEC_FLAG_EXPLAIN_GENERIC))
{
InitPartitionPruneContext(&pprune->exec_context,
pinfo->exec_pruning_steps,
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/include/executor/executor.h b/src/include/executor/executor.h
index dbd77050c7..7b4c1834ef 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -36,6 +36,11 @@
* of startup should occur. However, error checks (such as permission checks)
* should be performed.
*
+ * EXPLAIN_GENERIC can only be used together with EXPLAIN_ONLY. It indicates
+ * that a generic plan is being calculated using EXPLAIN (GENERIC_PLAN), which
+ * means that missing parameters must be tolerated. Currently, the only effect
+ * is to suppress execution-time partition pruning.
+ *
* REWIND indicates that the plan node should try to efficiently support
* rescans without parameter changes. (Nodes must support ExecReScan calls
* in any case, but if this flag was not given, they are at liberty to do it
@@ -53,12 +58,13 @@
* mean that the plan can't queue any AFTER triggers; just that the caller
* is responsible for there being a trigger context for them to be queued in.
*/
-#define EXEC_FLAG_EXPLAIN_ONLY 0x0001 /* EXPLAIN, no ANALYZE */
-#define EXEC_FLAG_REWIND 0x0002 /* need efficient rescan */
-#define EXEC_FLAG_BACKWARD 0x0004 /* need backward scan */
-#define EXEC_FLAG_MARK 0x0008 /* need mark/restore */
-#define EXEC_FLAG_SKIP_TRIGGERS 0x0010 /* skip AfterTrigger calls */
-#define EXEC_FLAG_WITH_NO_DATA 0x0020 /* rel scannability doesn't matter */
+#define EXEC_FLAG_EXPLAIN_ONLY 0x0001 /* EXPLAIN, no ANALYZE */
+#define EXEC_FLAG_REWIND 0x0002 /* need efficient rescan */
+#define EXEC_FLAG_BACKWARD 0x0004 /* need backward scan */
+#define EXEC_FLAG_MARK 0x0008 /* need mark/restore */
+#define EXEC_FLAG_SKIP_TRIGGERS 0x0010 /* skip AfterTrigger calls */
+#define EXEC_FLAG_WITH_NO_DATA 0x0020 /* rel scannability doesn't matter */
+#define EXEC_FLAG_EXPLAIN_GENERIC 0x0040 /* EXPLAIN (GENERIC_PLAN) */
/* Hook for plugins to get control in ExecutorStart() */
diff --git a/src/test/regress/expected/explain.out b/src/test/regress/expected/explain.out
index 48620edbc2..253b818c77 100644
--- a/src/test/regress/expected/explain.out
+++ b/src/test/regress/expected/explain.out
@@ -517,3 +517,45 @@ 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 option ANALYZE cannot be used with GENERIC_PLAN
+CONTEXT: PL/pgSQL function explain_filter(text) line 5 at FOR over EXECUTE statement
+-- Test EXPLAIN (GENERIC_PLAN) with partition pruning
+-- partitions should be pruned at plan time, based on constants,
+-- but there should be no pruning based on parameter placeholders
+create table gen_part (
+ key1 integer not null,
+ key2 integer not null
+) partition by list (key1);
+create table gen_part_1
+ partition of gen_part for values in (1)
+ partition by range (key2);
+create table gen_part_1_1
+ partition of gen_part_1 for values from (1) to (2);
+create table gen_part_1_2
+ partition of gen_part_1 for values from (2) to (3);
+create table gen_part_2
+ partition of gen_part for values in (2);
+-- should only scan gen_part_1_1 and gen_part_1_2, but not gen_part_2
+select explain_filter('explain (generic_plan) select key1, key2 from gen_part where key1 = 1 and key2 = $1');
+ explain_filter
+---------------------------------------------------------------------------
+ Append (cost=N.N..N.N rows=N width=N)
+ -> Seq Scan on gen_part_1_1 gen_part_1 (cost=N.N..N.N rows=N width=N)
+ Filter: ((key1 = N) AND (key2 = $N))
+ -> Seq Scan on gen_part_1_2 gen_part_2 (cost=N.N..N.N rows=N width=N)
+ Filter: ((key1 = N) AND (key2 = $N))
+(5 rows)
+
+drop table gen_part;
diff --git a/src/test/regress/sql/explain.sql b/src/test/regress/sql/explain.sql
index ae3f7a308d..ff9c51e1d1 100644
--- a/src/test/regress/sql/explain.sql
+++ b/src/test/regress/sql/explain.sql
@@ -128,3 +128,27 @@ 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');
+-- Test EXPLAIN (GENERIC_PLAN) with partition pruning
+-- partitions should be pruned at plan time, based on constants,
+-- but there should be no pruning based on parameter placeholders
+create table gen_part (
+ key1 integer not null,
+ key2 integer not null
+) partition by list (key1);
+create table gen_part_1
+ partition of gen_part for values in (1)
+ partition by range (key2);
+create table gen_part_1_1
+ partition of gen_part_1 for values from (1) to (2);
+create table gen_part_1_2
+ partition of gen_part_1 for values from (2) to (3);
+create table gen_part_2
+ partition of gen_part for values in (2);
+-- should only scan gen_part_1_1 and gen_part_1_2, but not gen_part_2
+select explain_filter('explain (generic_plan) select key1, key2 from gen_part where key1 = 1 and key2 = $1');
+drop table gen_part;
--
2.39.2
