Re: A few patches to clarify snapshot management, part 2

On 05/01/2026 19:01, Álvaro Herrera wrote:
> On 2025-Dec-22, Heikki Linnakangas wrote:
>
>> @@ -146,7 +146,11 @@ find_inheritance_children_extended(Oid parentrelId, bool omit_detached,
>> Snapshot snap;
>>
>> xmin = HeapTupleHeaderGetXmin(inheritsTuple->t_data);
>> +
>> + /* XXX: what should we do with a non-MVCC snapshot? */
>> snap = GetActiveSnapshot();
>> + if (snap->snapshot_type != SNAPSHOT_MVCC)
>> + elog(ERROR, "cannot look up partition information with a non-MVCC snapshot");
>>
>> if (!XidInMVCCSnapshot(xmin, snap))
>> {
>
> I think this code should be skipped when a non-MVCC snapshot is in use.
> I assume you won't hit the case unless you test for it specifically, so
> I'm not surprised that just adding an elog(ERROR) does not cause visible
> failures; however in production it probably would. And looking up
> partition info in that case is, most likely, useless. So changing the
> 'if' test
> if (!XidInMVCCSnapshot(xmin, snap))
> to only do this dance if "->snapshot_type == SNAPSHOT_MVCC" seems
> reasonable to me.

Yeah, I suppose the most reasonable behavior is to treat a non-MVCC
snapshot the same as no active snapshot at all here. Although I'm not
convinced that this is reachable in any sensible scenario (see below).

> I'm not sure what's a good way to verify this, however. We would need a
> test that adds partitions pending detach to partitioned tables used by
> as many other tests as possible ... (Maybe: have test_setup.sql add an
> event trigger that runs on CREATE TABLE, detects PARTITION BY, and
> creates and partially detaches a partition. Not sure how feasible this
> is.)

None of the built-in code pushes a historic snapshot to the active
stack, so that won't find anything. The only known instance of that is
pglogical's row filters. Is it sane for a row filter, or other
hypothetical expression that is executed during logical decoding, to
access a partitioned table?

We don't allow marking a partitioned table as a "user catalog table":

postgres=# CREATE TABLE parttab (i int) PARTITION BY RANGE (i);
CREATE TABLE
postgres=# ALTER TABLE parttab SET (user_catalog_table = true);
ERROR: cannot specify storage parameters for a partitioned table
HINT: Specify storage parameters for its leaf partitions instead.

And it's not sane to access tables other than catalog tables with a
historic snapshot. So I'm not sure this reachable in any sensible scenario.

- Heikki