Re: renaming ExecStoreWhateverTuple

Hi,

On 2019-03-25 11:20:13 -0400, Robert Haas wrote:
> While reviewing some zheap code last week, I noticed that the naming
> of the ExecStore*Tuple() functions seems a bit unfortunate. One
> reason is that we are now using slots in all kinds of places other
> than the executor, so that the "Exec" prefix seems a bit out of place.
> However, you could argue that this is OK, on the grounds that the
> functions are defined in the executor. What I noticed, though, is
> that every new table AM is probably going to need its own type of
> slot, and it's natural to define those slot support functions in the
> AM code rather than having every AM shove whatever it's got into
> execTuples.c.

Yea, it's not accurate. Nor was it before this change though - there's
e.g. plenty executor code that used slots long before v12.

> But if you do that, then you end up with a function with a name like
> ExecStoreZHeapTuple() which isn't in src/backend/executor, and which
> furthermore will never be called from the executor, because the
> executor only knows about a short, hard-coded list of built-in slot
> types, and ZHeapTuples are not one of them. That function will,
> rather, only be called from the zheap code. And having a function
> whose name starts with Exec... that is neither defined in the executor
> nor used in the executor seems wrong.

Yea.

I feel if we go there we probably should also rename ExecClearTuple,
ExecMaterializeSlot, ExecCopySlotHeapTuple, ExecCopySlotMinimalTuple,
ExecCopySlot. Although there we probably can add a compat wrapper.

> So I think we should rename these functions before we get too used to
> the new names. There is a significant advantage in doing that for v12
> because people are already going to have to adjust third-party code to
> compensate for the fact that we no longer have an ExecStoreTuple()
> function any more.

Indeed.

> I'm not sure exactly what names would be better. Perhaps just change
> the "Exec" prefix to "Slot", e.g. SlotStoreHeapTuple(). Or maybe put
> InSlot at the end, like StoreHeapTupleInSlot(). Or just take those
> four words - slot, heap, tuple, and store - and put them in any order
> you like. TupleSlotStoreHeap?

I think I might go for slot_store_heap_tuple etc, given that a lot of
those accessors have been slot_ for about ever. What do you think?

Greetings,

Andres Freund