Re: Cleaning up PREPARE query strings?

Hi,

On Sun, Jan 18, 2026 at 10:16:16AM -0600, Sami Imseih wrote:
> >
> > > 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.
> >
>
> My point is pstate->p_sourcetext doesn't get freed just because we're not
> referencing it from CachedPlanSource in prepared_queries. Instead, with
> multi-statement strings, prepared_queries now use a newly allocated string,
> new_query, which will be around until DEALLOCATE.
>
> ```
> + tmp = palloc(rawstmt->stmt_len + 1);
> + strlcpy(tmp, cleaned, rawstmt->stmt_len + 1);
> +
> + new_query = tmp;
> ```
>
> So this patch always increases memory usage for multi-statement strings;
> we have both the original multi-statement string and the cleaned up copy
> around.

I think that you have a misunderstanding of how memory context works in
postgres. Both pstate->p_sourcetext and new_query will be freed when their
underlying memory context will be reset whether or not they're individually
freed, so they won't survive until the next EXECUTE or DEALLOCATE. It's
actually usually worse to issue individual pfree() as it's just unneeded
overhead.

CreateCachedPlan() creates a dedicated memory context to save all its
information, including a new copy of the passed query string, and that context
will eventually be linked to a permanent one so that it can survive the
original query execution lifetime.

So with this patch we have an opportunity to allocate less data in the
permanent memory context, which as I said requires a transient extra allocation
of the current portion of the current query string.