On Thu, Dec 17, 2020 at 04:35:33PM +0530, Bharath Rupireddy wrote: > > You made the v2 insert interface a requirement for all table AMs. > > Should it be optional, and fall back to simple inserts if not implemented ? > > I tried to implement the APIs mentioned by Andreas here in [1]. I just > used v2 table am APIs in existing table_insert places to show that it > works. Having said that, if you notice, I moved the bulk insert > allocation and deallocation to the new APIs table_insert_begin() and > table_insert_end() respectively, which make them tableam specific.
I mean I think it should be optional for a tableam to support the optimized insert routines. Here, you've made it a requirement. + Assert(routine->tuple_insert_begin != NULL); + Assert(routine->tuple_insert_v2 != NULL); + Assert(routine->multi_insert_v2 != NULL); + Assert(routine->multi_insert_flush != NULL); + Assert(routine->tuple_insert_end != NULL); +static inline void +table_multi_insert_v2(TableInsertState *state, TupleTableSlot *slot) +{ + state->rel->rd_tableam->multi_insert_v2(state, slot); +} If multi_insert_v2 == NULL, I think table_multi_insert_v2() would just call table_insert_v2(), and begin/flush/end would do nothing. If table_multi_insert_v2!=NULL, then you should assert that the other routines are provided. Are you thinking that TableInsertState would eventually have additional attributes which would apply to other tableams, but not to heap ? Is heap_insert_begin() really specific to heap ? It's allocating and populating a structure based on its arguments, but those same arguments would be passed to every other AM's insert_begin routine, too. Do you need a more flexible data structure, something that would also accomodate extensions? I'm thinking of reloptions as a loose analogy. -- Justin