On Fri, Nov 24, 2023 at 5:18 PM Mark Dilger
<mark.dil...@enterprisedb.com> wrote:
> > On Nov 23, 2023, at 4:42 AM, Alexander Korotkov <aekorot...@gmail.com> 
> > wrote:
>
>
> > 0006-Generalize-table-AM-API-for-INSERT-.-ON-CONFLICT-v1.patch
> >
> > Provides a new table AM API method to encapsulate the whole INSERT ...
> > ON CONFLICT ... algorithm rather than just implementation of
> > speculative tokens.
>
> I *think* I understand that you are taking the part of INSERT..ON CONFLICT 
> that lives outside the table AM and pulling it inside so that table AM 
> authors are free to come up with whatever implementation is more suited for 
> them.  The most straightforward way of doing so results in an EState 
> parameter in the interface definition.  That seems not so good, as the EState 
> is a fairly complicated structure, and future changes in the executor might 
> want to rearrange what EState tracks, which would change which values 
> tuple_insert_with_arbiter() can depend on.

I think this is the correct understanding.

> Should the patch at least document which parts of the EState are expected to 
> be in which states, and which parts should be viewed as undefined?  If the 
> implementors of table AMs rely on any/all aspects of EState, doesn't that 
> prevent future changes to how that structure is used?

New tuple tuple_insert_with_arbiter() table AM API method needs EState
argument to call executor functions: ExecCheckIndexConstraints(),
ExecUpdateLockMode(), and ExecInsertIndexTuples().  I think we
probably need to invent some opaque way to call this executor function
without revealing EState to table AM.  Do you think this could work?

> > 0008-Let-table-AM-insertion-methods-control-index-inse-v1.patch
> >
> > Allows table AM to skip index insertions in the executor and handle
> > those insertions itself.
>
> The new parameter could use more documentation.
>
> > 0009-Custom-reloptions-for-table-AM-v1.patch
> >
> > Enables table AMs to define and override reloptions for tables and indexes.
>
> This could use some regression tests to exercise the custom reloptions.

Thank you for these notes.  I'll take this into account for the next
patchset version.

------
Regards,
Alexander Korotkov


Reply via email to