public inbox for [email protected]  

help / color / mirror / Atom feed

From: Alexander Korotkov <[email protected]>
To: Mark Dilger <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Table AM Interface Enhancements
Date: Sat, 25 Nov 2023 19:47:57 +0200
Message-ID: <CAPpHfdv0g30VPKBwK=jCEmDdybgdu2Ro8VSNbLnc22C6qKsaZA@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAPpHfdurb9ycV8udYqM=o0sPS66PJ4RCBM1g-bBpvzUfogY0EA@mail.gmail.com>
	<[email protected]>

On Fri, Nov 24, 2023 at 5:18 PM Mark Dilger
<[email protected]> wrote:
> > On Nov 23, 2023, at 4:42 AM, Alexander Korotkov <[email protected]> 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

view thread (3+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Table AM Interface Enhancements
  In-Reply-To: <CAPpHfdv0g30VPKBwK=jCEmDdybgdu2Ro8VSNbLnc22C6qKsaZA@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox