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