public inbox for [email protected]  

help / color / mirror / Atom feed

From: Julien Rouhaud <[email protected]>
To: Sami Imseih <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: [email protected]
Subject: Re: Cleaning up PREPARE query strings?
Date: Thu, 15 Jan 2026 12:41:29 +0800
Message-ID: <aWhv-ZXEJ3kd-dWa@jrouhaud> (raw)
In-Reply-To: <CAA5RZ0s4UPJ5uHzcfDqOLOskOFTPFZnUZwZS7e93LTmG8nHVnA@mail.gmail.com>
References: <aUwKEWGge5jWtaRX@jrouhaud>
	<[email protected]>
	<aWNWO8wEvu6YQaKN@jrouhaud>
	<CAA5RZ0sswD2h2w8FkVjToA=3oNYUb1mW-zuCD=J6DDK2zuLuZg@mail.gmail.com>
	<aWbMU6wKpYkJHPP5@jrouhaud>
	<CAA5RZ0s4UPJ5uHzcfDqOLOskOFTPFZnUZwZS7e93LTmG8nHVnA@mail.gmail.com>

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.

view thread (7+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Cleaning up PREPARE query strings?
  In-Reply-To: <aWhv-ZXEJ3kd-dWa@jrouhaud>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox