Hi,
On Thu, Jan 15, 2026 at 01:13:36PM -0600, Sami Imseih wrote:
> > 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?
>
> That will be a fix, but that seems way too invasive for me. Not sure if it may
> break other things.
>
> > > 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.
>
> ISTM that your proposal will actually use more memory because
> pstate->p_sourcetext does not get free'd, but we must now allocate
> space for a new "cleaned" query.
I'm not sure that I understand. Sure we allocate a new temporary buffer for
the cleaned up query string but this will be freed soon. The query string
stored in the prepared statement will stay in memory as long as the prepare
statement exist so this any cleanup can actually save a lot of memory.
I'm attaching a v2 that fixes the crash you describe earlier, and with some
minimal regression test in pg_stat_statements to cover it.
> As far as execution overhead, this will only be paid when you query
> pg_prepared_statement, and we can even optimize this a bit by only
> cleaning once and reusing the results.
Actually I thought that there was a requirement to have a zero terminated
string in the API to transform C strings to text datums, but there is not.
It's actually slightly faster if you know the size as there is on strlen() less
call in that case. This could be done in both case though.
> > 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.
>
> I agree.
As mentioned basic tests added in v2.
