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.
