TestForOldSnapshot() seems to be in the wrong place

Hi,

Currently TestForOldSnapshot() is defined as

extern void TestForOldSnapshot_impl(Snapshot snapshot, Relation relation);
...
/*
* Check whether the given snapshot is too old to have safely read the given
* page from the given table. If so, throw a "snapshot too old" error.
*
* This test generally needs to be performed after every BufferGetPage() call
* that is executed as part of a scan. It is not needed for calls made for
* modifying the page (for example, to position to the right place to insert a
* new index tuple or for vacuuming). It may also be omitted where calls to
* lower-level functions will have already performed the test.
*
* Note that a NULL snapshot argument is allowed and causes a fast return
* without error; this is to support call sites which can be called from
* either scans or index modification areas.
*
* For best performance, keep the tests that are fastest and/or most likely to
* exclude a page from old snapshot testing near the front.
*/
static inline void
TestForOldSnapshot(Snapshot snapshot, Relation relation, Page page)
{
Assert(relation != NULL);

if (old_snapshot_threshold >= 0
&& (snapshot) != NULL
&& ((snapshot)->satisfies == HeapTupleSatisfiesMVCC
|| (snapshot)->satisfies == HeapTupleSatisfiesToast)
&& !XLogRecPtrIsInvalid((snapshot)->lsn)
&& PageGetLSN(page) > (snapshot)->lsn)
TestForOldSnapshot_impl(snapshot, relation);
}

to me it seems wrong from a layering POV to have this in
bufmgr.[ch]. Due to the inline functions this makes bufmgr.h have a
dependency on snapmgr.h and tqual.h, which to me seems wrong from a
layer POV. Why isn't this declared in snapmgr.h or a new header file?

Greetings,

Andres Freund