Re: A few patches to clarify snapshot management

On Tue, Aug 19, 2025 at 11:45:01PM +0300, Heikki Linnakangas wrote:
> How about the attached, then? It reverts the GetTransactionSnapshot()
> change. But to still catch at least some of the invalid uses of the historic
> snapshot, it adds checks to heap_beginscan() and index_beginscan(), to
> complain if they are called on a non-catalog relation with a historic
> snapshot.

> @@ -1143,6 +1143,15 @@ heap_beginscan(Relation relation, Snapshot snapshot,
>      if (!(snapshot && IsMVCCSnapshot(snapshot)))
>          scan->rs_base.rs_flags &= ~SO_ALLOW_PAGEMODE;
>  
> +    /* Check that a historic snapshot is not used for non-catalog tables */
> +    if (snapshot &&
> +        IsHistoricMVCCSnapshot(snapshot) &&
> +        !RelationIsAccessibleInLogicalDecoding(relation))
> +    {
> +        elog(ERROR, "cannot query non-catalog table \"%s\" during logical decoding",
> +             RelationGetRelationName(relation));
> +    }
> +

I feel post-beta3 is late for debut of restrictions like this.  How about a
pure revert, then add those restrictions in v19?  Should be s/elog/ereport/,
also.