Re: table AM option passing

On Tue, Mar 17, 2026 at 05:50:41PM +0100, Álvaro Herrera wrote:
> We could solve this easily by adding one more boolean to each, but I
> think this is a good time to instead consolidate the API by using a
> bitmask; this also allows us not to have changingPart in the wrong place
> of the heap_delete API.
> 
> So here's a patch to do that, which shouldn't change any behavior.

Seems entirely reasonable to me.  I read through the patch and nothing
stood out.

> (This change is vaguely similar to b7271aa1d71a, except I used 'int'
> instead of 'bits32', to keep the interface consistent with the existing
> heap_insert() one.  Maybe I should make all three take bits64 instead?
> We don't actually have that type at present, so I'd have to add that
> too.)

Why bits64 and not bits32?  I must be missing something.

> While at it, I noticed that the table_insert() and heap_insert() uses
> one set of value definitions for each half of the interface; that is, in
> tableam.h we have
> 
> /* "options" flag bits for table_tuple_insert */
> /* TABLE_INSERT_SKIP_WAL was 0x0001; RelationNeedsWAL() now governs */
> #define TABLE_INSERT_SKIP_FSM        0x0002
> #define TABLE_INSERT_FROZEN        0x0004
> #define TABLE_INSERT_NO_LOGICAL        0x0008
> 
> and in heapam.h we have
> /* "options" flag bits for heap_insert */
> #define HEAP_INSERT_SKIP_FSM        TABLE_INSERT_SKIP_FSM
> #define HEAP_INSERT_FROZEN        TABLE_INSERT_FROZEN
> #define HEAP_INSERT_NO_LOGICAL        TABLE_INSERT_NO_LOGICAL
> #define HEAP_INSERT_SPECULATIVE     0x0010
> 
> This seems rather odd to me -- how could heapam.c have a different set
> of behaviors than what table AM uses?  I find it even more weird that
> HEAP_INSERT_SPECULATIVE is defined so that as soon as some future patch
> defines the next "free" tableam.h flag value to do something new, we'll
> have a conflict.  I think this would be cleaner if we removed from
> heapam.h the flags that correspond to anything in tableam.h, and use
> heapam.c and all its direct callers use the tableam.h flag definitions
> instead; and perhaps move HEAP_INSERT_SPECULATIVE to be at the other end
> of the bitmask (0x1000) -- maybe simply say in tableam.h that the first
> byte of the options int is reserved for internal use.

Probably a good idea.

-- 
nathan