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
