Why do we have MakeSingleTupleTableSlot instead of not using MakeTupleTableSlot?
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t43697psql -h localhost -U postgresBuilt from patchset v3 (message #3), July 27, 2026 at 05:41 PM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t43697_3 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t43697_3 && git checkout t43697_3Patchset v3 (message #3) is on t43697_3
Hi,
I wonder, is there a specific reason that MakeTupleTableSlot is
wrapped up in MakeSingleTupleTableSlot without doing anything than
just returning the slot created by MakeTupleTableSlot? Do we really
need MakeSingleTupleTableSlot? Can't we just use MakeTupleTableSlot
directly? Am I missing something?
I think we can avoid some unnecessary function call costs, for
instance when called 1000 times inside table_slot_create from
copyfrom.c or in some other places where MakeSingleTupleTableSlot is
called in a loop.
If it's okay to remove MakeSingleTupleTableSlot and use
MakeTupleTableSlot instead, we might have to change in a lot of
places. If we don't want to change in those many files, we could
rename MakeTupleTableSlot to MakeSingleTupleTableSlot and change it in
only a few places.
Thoughts?
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
Hi,
MakeSingleTupleTableSlot can be defined as a macro, calling
MakeTupleTableSlot().
Cheers
On Fri, Feb 12, 2021 at 5:44 AM Bharath Rupireddy <
bharath.rupireddyforpostgres@gmail.com> wrote:
Show quoted text
Hi,
I wonder, is there a specific reason that MakeTupleTableSlot is
wrapped up in MakeSingleTupleTableSlot without doing anything than
just returning the slot created by MakeTupleTableSlot? Do we really
need MakeSingleTupleTableSlot? Can't we just use MakeTupleTableSlot
directly? Am I missing something?I think we can avoid some unnecessary function call costs, for
instance when called 1000 times inside table_slot_create from
copyfrom.c or in some other places where MakeSingleTupleTableSlot is
called in a loop.If it's okay to remove MakeSingleTupleTableSlot and use
MakeTupleTableSlot instead, we might have to change in a lot of
places. If we don't want to change in those many files, we could
rename MakeTupleTableSlot to MakeSingleTupleTableSlot and change it in
only a few places.Thoughts?
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
On Fri, Feb 12, 2021 at 9:37 PM Zhihong Yu <zyu@yugabyte.com> wrote:
On Fri, Feb 12, 2021 at 5:44 AM Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote:
Hi,
I wonder, is there a specific reason that MakeTupleTableSlot is
wrapped up in MakeSingleTupleTableSlot without doing anything than
just returning the slot created by MakeTupleTableSlot? Do we really
need MakeSingleTupleTableSlot? Can't we just use MakeTupleTableSlot
directly? Am I missing something?I think we can avoid some unnecessary function call costs, for
instance when called 1000 times inside table_slot_create from
copyfrom.c or in some other places where MakeSingleTupleTableSlot is
called in a loop.If it's okay to remove MakeSingleTupleTableSlot and use
MakeTupleTableSlot instead, we might have to change in a lot of
places. If we don't want to change in those many files, we could
rename MakeTupleTableSlot to MakeSingleTupleTableSlot and change it in
only a few places.Thoughts?
MakeSingleTupleTableSlot can be defined as a macro, calling MakeTupleTableSlot().
Right, we could as well have an inline function. My point was that why
do we need to wrap MakeTupleTableSlot inside MakeSingleTupleTableSlot
which just does nothing. As I said upthread, how about renaming
MakeTupleTableSlot to
MakeSingleTupleTableSlot which requires minimal changes? Patch
attached. Both make check and make check-world passes on it. Please
have a look.
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> writes:
Right, we could as well have an inline function. My point was that why
do we need to wrap MakeTupleTableSlot inside MakeSingleTupleTableSlot
which just does nothing. As I said upthread, how about renaming
MakeTupleTableSlot to
MakeSingleTupleTableSlot which requires minimal changes?
I'm disinclined to change this just to save one level of function call.
If you dig in the git history (see f92e8a4b5 in particular) you'll note
that the current version of MakeTupleTableSlot originated as code shared
between ExecAllocTableSlot and MakeSingleTupleTableSlot. The fact that
the latter is currently just equivalent to that shared functionality is
something that happened later and might need to change again.
It is fair to wonder why execTuples.c exports MakeTupleTableSlot at
all, though. ExecAllocTableSlot is supposed to be used by code that
expects ExecutorEnd to clean up the slot, while MakeSingleTupleTableSlot
is supposed to pair with ExecDropSingleTupleTableSlot. Direct use of
MakeTupleTableSlot leaves one wondering who is holding the bag for
slot cleanup. The external callers of it all look to be pretty new
code, so I wonder how carefully that's been thought through.
In short: I'm not okay with doing
s/MakeTupleTableSlot/MakeSingleTupleTableSlot/g in a patch that doesn't
also introduce matching ExecDropSingleTupleTableSlot calls (unless those
exist somewhere already; but where?). If we did clean that up, maybe
MakeTupleTableSlot could become "static". But I'd still be inclined to
keep it physically separate, leaving it to the compiler to decide whether
to inline it into the callers.
There's a separate question of whether any of the call sites that lack
cleanup support represent live resource-leak bugs. I see that they all
use TTSOpsVirtual, so maybe that's a slot type that never holds any
interesting resources (like buffer pins). If so, maybe the best thing is
to invent a wrapper "MakeVirtualTupleTableSlot" or the like, ensuring such
callers use a TupleTableSlotOps type that doesn't require cleanup.
regards, tom lane