SQL functions: avoid making a tuplestore unnecessarily
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Date: 2025-04-17T19:59:28Z
Lists: pgsql-hackers
Attachments
- v1-avoid-unnecessary-tuplestore.patch (text/x-diff) patch v1
(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
Commits
-
Don't use a tuplestore if we don't have to for SQL-language functions.
- e83a8ae44729 18.0 landed