On Fri, May 26, 2023 at 10:28 AM Peter Geoghegan <p...@bowt.ie> wrote: > I've added several defensive assertions that make it hard to get the > details wrong. These will catch the issue much earlier than the main > "heapRel != NULL" assertion in _bt_allocbuf(). So, the rules are > reasonably straightforward and enforceable.
Though it's not an issue new to 16, or a problem that anybody is obligated to deal with on this thread, I wonder: Why is it okay that we're using "rel" (the index rel) for our TestForOldSnapshot() calls in nbtree, rather than using heapRel? Why wouldn't the rules be the same as they are for the new code paths needed for logical decoding on standbys? (Namely, the "use heapRel, not rel" rule.) More concretely, I'm pretty sure that RelationIsUsedAsCatalogTable() (which TestForOldSnapshot() relies on) gives an answer that would change if we decided to pass heapRel to the main TestForOldSnapshot() call within _bt_moveright(), instead of doing what we actually do, which is to just pass it the index rel. I suppose that that interaction might have been overlooked when bugfix commit bf9a60ee33 first added RelationIsUsedAsCatalogTable() -- since that happened a couple of months after the initial "snapshot too old" commit went in, a fix that happened under time pressure. More generally, the high level rules/invariants that govern when TestForOldSnapshot() should be called (and with what rel/snapshot) feel less than worked out. I find it suspicious that there isn't any attempt to relate TestForOldSnapshot() behaviors to the conceptually similar PredicateLockPage() behavior. We don't need predicate locks on internal pages, but TestForOldSnapshot() *does* get called for internal pages. Many PredicateLockPage() calls happen very close to TestForOldSnapshot() calls, each of which use the same snapshot -- not addressing that seems like a glaring omission to me. Basically it seems like there should be one standard set of rules for all this stuff. Though it's not the fault of Bertrand or Andres, all that we have now is two poorly documented sets of rules that partially overlap. This has long bothered me. -- Peter Geoghegan
