Re: Make EXPLAIN generate a generic plan for a parameterized query

Hi,

I reviewed the patch and find the idea of allowing $placeholders with
EXPLAIN very useful, it will solve relevant real-world use-cases.
(Queries from pg_stat_statements, found in the log, or in application
code.)

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>.

> + /* 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")));

> --- a/src/test/regress/sql/explain.sql
> +++ b/src/test/regress/sql/explain.sql
> @@ -128,3 +128,33 @@ 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
> +-- should prune at plan time, but not at execution time
> +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.

> +create server loop42 foreign data wrapper postgres_fdw;
> +create user mapping for current_role server loop42 options (password_required 'false');
> +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 loop42 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 loop42 options (table_name 'whatever_1_2');
> +create foreign table gen_part_2
> + partition of gen_part for values in (2)
> + server loop42 options (table_name 'whatever_2');
> +select explain_filter('explain (generic_plan) select key1, key2 from gen_part where key1 = 1 and key2 = $1');

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

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/.

In the new location, likewise add a comment why this test needs to
look this way.

Christoph