Re: heapam_relation_toast_am() returns the wrong AM for a wrapped heap AM

On 2026-08-21 Fr 12:50 PM, Álvaro Herrera wrote:
> On 2026-Aug-21, Andrew Dunstan wrote:
>
>> heapam_relation_toast_am() returns rel->rd_rel->relam instead of the
>> literal heap AM oid, on the assumption the two are always equal since
>> it's only meant to run for relations that are themselves heap. That
>> breaks for a table AM that copies heap's whole TableAmRoutine (via
>> GetHeapamTableAmRoutine()) and overrides only a few callbacks -- a
>> pattern heap_getnext()'s own identity check explicitly anticipates,
>> per its comment about allowing "regression tests that create another
>> AM reusing the heap handler." For such an AM, rel->rd_rel->relam is
>> its own oid, so NewRelationCreateToastTable() creates the TOAST table
>> with that AM too, and building its chunk_id/chunk_seq index then
>> fails in heap_getnext(), which requires rd_tableam to be literally
>> GetHeapamTableAmRoutine(): "only heap AM is supported" for any such
>> AM as soon as a table needs a TOAST table.
>>
>> Fix by returning the literal HEAP_TABLE_AM_OID, which is what the
>> function's own comment already claims it does ("TOAST tables for
>> heap relations are just heap relations").
> I'm not sure that this patch is correct. The heapam_relation_toast_am
> function is the implementation for the relation_toast_am callback in the
> heapam table AM. So this function is specific to heapam, and other
> table AMs should have their own if they want to have different behavior.
>
> The function is really small, so if a table AM that's not heapam can
> very easily set up its own callback function that returns
> HEAP_TABLE_AM_OID to get a regular heapam TOAST table, right?
>
> So "a table AM that [copies the whole heap TableAmRoutine and overrides
> only a handful of callbacks]" is Doing It Wrong by failing to also
> override relation_toast_am.

OK, fair enough, I withdraw the proposal. I guess the dummy AM I set up
for testing the AM options patch Did It Wrong ;-)

Maybe this hazard is worth documenting somewhere. I'll look for a good
place.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com