Date: Thu, 15 Jan 2026 12:41:29 +0800
From: Julien Rouhaud <rjuju123@gmail.com>
To: Sami Imseih <samimseih@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, pgsql-hackers@lists.postgresql.org
Subject: Re: Cleaning up PREPARE query strings?
Message-ID: <aWhv-ZXEJ3kd-dWa@jrouhaud>
References: <aUwKEWGge5jWtaRX@jrouhaud>
 <928501.1766593260@sss.pgh.pa.us>
 <aWNWO8wEvu6YQaKN@jrouhaud>
 <CAA5RZ0sswD2h2w8FkVjToA=3oNYUb1mW-zuCD=J6DDK2zuLuZg@mail.gmail.com>
 <aWbMU6wKpYkJHPP5@jrouhaud>
 <CAA5RZ0s4UPJ5uHzcfDqOLOskOFTPFZnUZwZS7e93LTmG8nHVnA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAA5RZ0s4UPJ5uHzcfDqOLOskOFTPFZnUZwZS7e93LTmG8nHVnA@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/aWhv-ZXEJ3kd-dWa%40jrouhaud>
Precedence: bulk

Hi,

On Wed, Jan 14, 2026 at 09:56:21AM -0600, Sami Imseih wrote:
> > That's very strange.  I'm pretty sure that I tried since the earlier approach I
> > mentioned using doing it in CreateCachedPlan() had that exact problem.
> >
> > Also, src/test/recovery/t/027_stream_regress.pl does run the full regression
> > tests with pg_stat_statements enabled, and it doesn't fail.
> >
> > I'm not sure what is different in your case.
>
> the regression tests succeed because by the time the multi-statement command
> is executed, the queries involved have already been tracked by
> pg_stat_statements,
> so they don't need to get stored again, and no reason to go through
> generate_normalize_query. In this case, SELECT $1 is already tracked.

I see.  I'm not sure why my first prototype used to crash with that exact same
problem in 027_stream_regress but not this one, but anyway.

I only had a quick look at the code but unless I'm missing something it's
mostly an oversight as I pass "new_query" to CreateCachedPlan() but still pass
the original query string to pg_analyze_and_rewrite_varparams().  Using
"new_query" there too should fix the problem?

In an earlier message you wrote

> I am thinking that storing the statement length and location in the entry
> of prepared_queries may be the better option. Having that info, the
> cleaned up query text can be generated on the fly whenever
> pg_prepared_statement is called.

I don't like this approach as it has more execution time overhead, but also
doesn't fix at all the memory waste in the prepared statements cache.

> To repro the crash, just reset pg_stat_statements prior.
>
> ```
> diff --git a/src/test/regress/sql/prepare.sql b/src/test/regress/sql/prepare.sql
> index 0e7fe44725e..51421357f26 100644
> --- a/src/test/regress/sql/prepare.sql
> +++ b/src/test/regress/sql/prepare.sql
> @@ -4,6 +4,9 @@
>
>  SELECT name, statement, parameter_types, result_types FROM
> pg_prepared_statements;
>
> +CREATE EXTENSION pg_stat_statements;
> +SELECT pg_stat_statements_reset();
> +
>  SELECT 'bingo'\;  PREPARE q1 AS SELECT 1 AS a \; SELECT 42;
>  EXECUTE q1;
> ```
>
> For this test, we should either reset statements before or construct a
> statement for the test that has not been tracked. We can maybe
> do that with a query against a newly created table in prepare.sql.
> I like the latter more.

FWIW I think that this should belong to pg_stat_statements testing, no the main
regression tests.  This would also ensure that we see consistent results in
some other scenarios.