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