Bharath Rupireddy <bharath.rupireddyforpostg...@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