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
