Hi,
On 2019-07-10 16:02:50 -0400, Tom Lane wrote:
> A bigger problem with this is that in the tableam world, this seems
> like a complete disaster modularity-wise. I think it won't actually
> fail --- non-heap AMs are probably not returning
> BufferHeapTupleTableSlots, and even if they are, they shouldn't be
> marking tuples HOT_UPDATED unless that concept applies to them.
> But it sure seems like this leaves get_actual_variable_range() knowing
> way more than it ought to about storage-level concerns.
Yea, I think that's not pretty. I'm not concerned about wrong results
due to BufferHeapTupleTableSlots being returned, and misinterpreted, but
layering wise this seems not good. Even leaving tableam aside, it seems
not nice to have this concern leak into selfuncs.c
> Should we try to transpose some of this logic to below the AM API,
> and if so, what should that look like?
I wonder if we could push this down into a separate type of visibility
(or maybe redefine NonVacuumable)? So that the scan wouldn't ever
return them in the first place? It's a bit annoying to have a visibility
type that's perhaps best called "what selfuncs.c needs", but still seams
cleaner than having a separate callback? And the explanation for why
that's possibly needed also seems to better belong inside the AMs.
- Andres