Re: Fallback table AM for relkinds without storage

On Mon, Feb 15, 2021 at 04:21:38PM +0900, Michael Paquier wrote:
> On Tue, Feb 09, 2021 at 04:27:34PM +0900, Michael Paquier wrote:
> > Putting sanity checks within all the table_* functions of tableam.h
> > would not be a good idea, as nothing prevents the call of what's
> > stored in rel->rd_tableam.
>
> I have been playing with this idea, and finished with the attached,
> which is not the sexiest patch around. The table AM used as fallback
> for tables without storage is called no_storage (this could be called
> virtual_am?). Reverting e786be5 or dd705a0 leads to an error coming
> from no_storage instead of a crash.

If you apply this patch, will you want to actually revert those earlier changes?

> One thing to note is that this simplifies a bit slot_callbacks as
> views, foreign tables and partitioned tables can grab their slot type
> directly from this new table AM.

Also (related), this still crashes if methods are omitted from the initializer,
like:

// .slot_callbacks = no_storage_slot_callbacks,

I'm not sure if there's any better way to enforce that's updated when callbacks
are added.

Most of the methods have Assert( != NULL), so maybe this one is missing?

src/backend/access/table/tableamapi.c
GetTableAmRoutine(Oid amhandler)
...
Assert(routine->slot_callbacks != NULL);

See also
https://www.postgresql.org/message-id/CALfoeisgdZhYDrJOukaBzvXfJOK2FQ0szVMK7dzmcy6w93iDUA%40mail.gmail.com

--
Justin