On Wed, 2024-03-27 at 01:19 +0530, Bharath Rupireddy wrote: > > Similarly, with this new AM, the onus lies on the table AM > implementers to provide an implementation for these new AMs even if > they just do single inserts.
Why not fall back to using the plain tuple_insert? Surely some table AMs might be simple and limited, and we shouldn't break them just because they don't implement the new APIs. > > table_multi_insert needs to be there for sure as COPY ... FROM uses > it. After we have these new APIs fully in place and used by COPY, what will happen to those other APIs? Will they be deprecated or will there be a reason to keep them? > FWIW, I can try writing a test table AM that uses this new AM but > just > does single inserts, IOW, equivalent to table_tuple_insert(). > Thoughts? More table AMs to test against would be great, but I also know that can be a lot of work. > > Firstly, we are not storing CommandId and options in > TableModifyState, > because we expect CommandId to be changing (per Andres comment). Trying to make this feature work across multiple commands poses a lot of challenges: what happens when there are SELECTs and subtransactions and non-deferrable constraints? Regardless, if we care about multiple CIDs, they should be stored along with the tuples, not supplied at the time of flushing. > You mean, writes are not flushed to the disk? Can you please > elaborate > why it's different for INSERT INTO ... SELECT and not others? Can't > the new flush AM be helpful here to implement any flush related > things? Not a major problem. We can discuss while working on IIS support. I am concnerned that the flush callback is not a part of the API. We will clearly need that to support index insertions for COPY/IIS, so as- is the API feels incomplete. Thoughts? Regards, Jeff Davis