Re: Make EXPLAIN generate a generic plan for a parameterized query - Mailing list pgsql-hackers
From | Laurenz Albe |
---|---|
Subject | Re: Make EXPLAIN generate a generic plan for a parameterized query |
Date | |
Msg-id | 9a9a5fa33c4a3e7da3f262e170835cb90fdfb787.camel@cybertec.at Whole thread Raw |
In response to | Re: Make EXPLAIN generate a generic plan for a parameterized query (Christoph Berg <myon@debian.org>) |
Responses |
Re: Make EXPLAIN generate a generic plan for a parameterized query
(Laurenz Albe <laurenz.albe@cybertec.at>)
Re: Make EXPLAIN generate a generic plan for a parameterized query (Tom Lane <tgl@sss.pgh.pa.us>) |
List | pgsql-hackers |
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
Attachment
pgsql-hackers by date: