SQL functions: avoid making a tuplestore unnecessarily

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: SQL functions: avoid making a tuplestore unnecessarily
Date: 2025-04-17 19:59:28
Message-ID: 2443532.1744919968@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

(I'm not proposing this for v18, but I wanted to get the patch
written while functions.c is still fresh in mind.)

The attached patch changes functions.c so that we only make a
tuplestore object if we're actually going to return multiple
rows in it, that is if it's a set-returning function and we
cannot use "lazyEval" mode. Otherwise, we can just rely on
the result slot we made anyway to hold the one tuple we need.

This saves a small number of cycles by not shoving a tuple into the
tuplestore only to pull it right back out. But the real reason for
changing it is not speed but resource-management worries. The
existing code (as it was before 0dca5d68d and is again as of
0313c5dc6) makes the tuplestore in the potentially long-lived fn_mcxt.
This'd be merely a slight annoyance if a tuplestore were purely a
memory object, but it isn't: it might contain an open file. If it
does, that file reference will be backed by a ResourceOwner,
specifically whichever ResourceOwner was CurrentResourceOwner at the
time of creating the tuplestore. What I am afraid of is that I don't
think there's any guarantee that that ResourceOwner is as long-lived
as the FmgrInfo. It should be fine within normal SQL query execution,
where the FmgrInfo will be part of the executor's state tree. But
there are long-lived FmgrInfos, such as those within the typcache, or
within an index's relcache entry. What if somebody tries to use a
SQL function to implement functionality that's reached through those
mechanisms?

Given the lack of field complaints over the many years it's been
like this, there doesn't seem to be a live problem. I think the
explanation for that is
(1) those mechanisms are never used to call set-returning functions,
(2) therefore, the tuplestore will not be called on to hold more
than one result row,
(3) therefore, it won't get large enough that it wants to spill
to disk,
(4) therefore, its potentially dangling resowner pointer is never
used.
However, this is an uncomfortably shaky chain of assumptions.
I want to cut it down by fixing things so that there is no
tuplestore, period, in a non-set-returning SQL function.

Furthermore, this patch changes things so that when we do make a
tuplestore, it lives in the per-query context not the fn_mcxt,
providing additional surety that it won't outlive its ResourceManager.
This is a change I made in 0dca5d68d and then had to undo for
performance reasons, but because of these resource-lifetime
considerations I really want it back that way again.

Since I think there is probably not a reachable bug here today,
I'm going to park this for v19.

regards, tom lane

Attachment Content-Type Size
v1-avoid-unnecessary-tuplestore.patch text/x-diff 10.0 KB

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2025-04-17 21:11:38 Re: Align memory context level numbering in pg_log_backend_memory_contexts()
Previous Message Nico Williams 2025-04-17 19:14:37 Re: Giving the shared catalogues a defined encoding