Re: Why do we have MakeSingleTupleTableSlot instead of not using MakeTupleTableSlot?

Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)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