Re: Cleaning up PREPARE query strings?

From: Julien Rouhaud <rjuju123(at)gmail(dot)com>
To: Sami Imseih <samimseih(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Cleaning up PREPARE query strings?
Date: 2026-01-15 04:41:29
Message-ID: aWhv-ZXEJ3kd-dWa@jrouhaud
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tatsuro Yamada 2026-01-15 04:43:50 Re: [PATCH] psql: add \dcs to list all constraints
Previous Message jian he 2026-01-15 04:31:07 Re: support ALTER COLUMN SET EXPRESSION over virtual generated column with check constraint