pg.ddx.io  pgsql-committers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-committers@lists.postgresql.org
Subject: pgsql: Don't use a tuplestore if we don't have to for SQL-language func
Date: Fri, 02 May 2025 20:16:32 +0000
Message-ID: <E1uAwoC-000fkH-27@gemulon.postgresql.org> (raw)

Don't use a tuplestore if we don't have to for SQL-language functions.

We only need a tuplestore if we're actually going to accumulate
multiple result tuples.  Obviously then we don't need one for non-set-
returning functions; but even a SRF doesn't need one if we decide to
use "lazyEval" (one row at a time) mode.  In these cases, it's
sufficient to use the junkfilter's result slot to hold the single row
that's due to be returned.  We just need to "materialize" that slot
to ensure it holds onto the data past shutdown of the sub-executor.

The original intent of this patch was partially to save a few cycles
(by not putting tuples into a tuplestore only to pull them back out
immediately), but mostly to ensure that we don't use a tuplestore
in non-set-returning functions.  That's because I had concerns
about whether a tuplestore is safe to keep across queries,
which was possible for functions invoked via long-lived FmgrInfos
such as those kept in the typcache.  There are no cases where SRFs
are called that way, so getting rid of the tuplestore in non-SRFs
should make things safer.

However, it emerges that running fmgr_sql in a short-lived context
(as 595d1efed made it do) makes the existing coding unsafe anyway:
we can end up with a long-lived TupleTableSlot holding a freeable
reference to a short-lived tuple, resulting in a double-free crash.
Not trying to pull tuples out of the tuplestore using that slot
dodges the problem, so I'm going to commit this now rather than
invent a band-aid solution for v18.

Reported-by: Alexander Lakhin <exclusion@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/2443532.1744919968@sss.pgh.pa.us
Discussion: https://postgr.es/m/9f975803-1a1c-4f21-b987-f572e110e860@gmail.com

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/e83a8ae44729bfdd15d31101bd2c99be98cfc691

Modified Files
--------------
src/backend/executor/functions.c | 135 ++++++++++++++++++++++++---------------
1 file changed, 85 insertions(+), 50 deletions(-)



Message-ID: <E1uAwoC-000fkH-27@gemulon.postgresql.org>
Permalink:  ../E1uAwoC-000fkH-27@gemulon.postgresql.org/
Also on:    postgresql.org/message-id/E1uAwoC-000fkH-27@gemulon.postgresql.org

 · 

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: pgsql-committers@postgresql.org
  Cc: tgl@sss.pgh.pa.us, pgsql-committers@lists.postgresql.org
  Subject: Re: pgsql: Don't use a tuplestore if we don't have to for SQL-language func
  In-Reply-To: <E1uAwoC-000fkH-27@gemulon.postgresql.org>

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

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