MIME-Version: 1.0
References: <CALj2ACXdrOmB6Na9amHWZHKvRT3Z0nwTRsCwoMT-npOBtmXLXg@mail.gmail.com>
 <20230603223824.o7iyochli2dwwi7k@alap3.anarazel.de> <CALj2ACX5UMWVFdrRNUE0KDrg54WV1cumBXwcETXhrPc1ibKAQA@mail.gmail.com>
 <CAAWbhmj5Pio3nOUakObzLGCSS9dwFfgsNVDhwTGzXNwZc00uCQ@mail.gmail.com>
 <CALj2ACVHC=c6eC9SRxhcTUrnXvNDNkEBgedi2WkVJYRb=0sWYw@mail.gmail.com>
 <CALj2ACVE2h=LnFnpr3rh+6SZzdwzW5EZOYG2Z0t=p28Fn75eag@mail.gmail.com>
 <CALj2ACWT0Rz8oybWBm5W4CeS0DvFkwaw-pEvGArhDLyPbZnW_g@mail.gmail.com>
 <CALj2ACWxO3HPtpYZb765LZk-uKVuAvZPO1HDeZ8=mzMgVPgaww@mail.gmail.com>
 <CALj2ACXJA4QQ_6zAHez0Uy-9t-ebmpox2y1QBja+mF4QP+h8WQ@mail.gmail.com>
 <CAD21AoD97mhzF8cqsd2v1jg9z8xfvAJrPx6Wvi+Ev0Hmu96LJA@mail.gmail.com>
 <CALj2ACUcv5pZoB0=gRrz54M9+YT9JCmo6FYyo5WUS6wnS+em=A@mail.gmail.com>
 <CALj2ACWm77YofBMs9x3Zmp3ctNAhcS4TvPVuXKdfwCr22FqOHg@mail.gmail.com>
 <6be6f58815dc0844fbe058edf56b4e735a6efc1c.camel@j-davis.com>
 <CALj2ACWqVzhxDuWNTWAH-LuADvsyX0r-wpwgeJ+Q1FnAKjY5Yw@mail.gmail.com>
 <2280bf7241119bb88cbe0fe5eb36490cbd04c0c0.camel@j-davis.com>
 <CALj2ACU70HZm+0QRJdkGA5RdJUo4zPYnV2hzkiV-wH5QS2PAEQ@mail.gmail.com>
 <12f1a2d8dd3b6305c0354f1c701f44b7be5e54eb.camel@j-davis.com>
 <CALj2ACVMV=gMROte2=0LBFnSCRvzL4D9WK6oQ9ZHr4Qj2S8xWA@mail.gmail.com>
 <8633171cb034aafc260fdf37df04b6c779aa1e2f.camel@j-davis.com>
 <CALj2ACX9vVYHYkX8e6w058EuAs8JL5EsnzadTxGhpiE_Ep_ByA@mail.gmail.com>
 <f2540511eff777eca435e60b9129eaa43e2ad548.camel@j-davis.com>
 <CALj2ACWTrx1zxWvq8Uj2rEwCsDgQHeJ53WdvzZUw3kW+_VPG6A@mail.gmail.com>
 <CALj2ACUz5+_YNEa4ZY-XG960_oXefM50MjD71VgSCAVDkF3bzQ@mail.gmail.com>
 <229c4f7219ed164088dadc935df21e1cf125e191.camel@j-davis.com>
 <CALj2ACX90L5Mb5Vv=jsvhOdZ8BVsfpZf-CdCGhtm2N+bGUCSjg@mail.gmail.com>
 <CALj2ACXnCvcYvaz4aDH_ezUBJanz_Boi8W=76fOwrxiwSnUFOQ@mail.gmail.com> <d079c042180a698179384d6a7a3109cd8310e285.camel@j-davis.com>
In-Reply-To: <d079c042180a698179384d6a7a3109cd8310e285.camel@j-davis.com>
From: Matthias van de Meent <boekewurm+postgres@gmail.com>
Date: Mon, 26 Aug 2024 23:59:28 +0200
Message-ID: <CAEze2WgmyTVyZv+u+D44f+3Xk5TyZm=+4Gp5qdEX14FwBJA1Hw@mail.gmail.com>
Subject: Re: Introduce new multi insert Table AM and improve performance of
 various SQL commands with it for Heap AM
To: Jeff Davis <pgsql@j-davis.com>, 
	Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>, 
	PostgreSQL-development <pgsql-hackers@postgresql.org>, Andres Freund <andres@anarazel.de>, 
	Dilip Kumar <dilipbalaut@gmail.com>, Luc Vlaming <luc@swarm64.com>, 
	Justin Pryzby <pryzby@telsasoft.com>, Michael Paquier <michael@paquier.xyz>, 
	Alexander Korotkov <aekorotkov@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CAEze2WgmyTVyZv%2Bu%2BD44f%2B3Xk5TyZm%3D%2B4Gp5qdEX14FwBJA1Hw%40mail.gmail.com>
Precedence: bulk

On Mon, 26 Aug 2024 at 23:18, Jeff Davis <pgsql@j-davis.com> wrote:
>
> On Mon, 2024-08-26 at 11:09 +0530, Bharath Rupireddy wrote:
> > On Wed, Jun 5, 2024 at 12:42=E2=80=AFPM Bharath Rupireddy
> > <bharath.rupireddyforpostgres@gmail.com> wrote:
> > >
> > > Please find the v22 patches with the above changes.
> >
> > Please find the v23 patches after rebasing 0005 and adapting 0004 for
> > 9758174e2e.
>
>
> Thank you.
>
> 0001 API design:
>
> * Remove TableModifyState.modify_end_callback.
>
> * This patch means that we will either remove or deprecate
> TableAmRoutine.multi_insert and finish_bulk_insert. Are there any
> strong opinions about maintaining support for multi-insert, or should
> we just remove it outright and force any new AMs to implement the new
> APIs to maintain COPY performance?

I don't think there is a significant enough difference in the
capabilities and requirements between the two APIs as currently
designed that removal of the old API would mean a significant
difference in capabilities. Maybe we could supply an equivalent API
shim to help the transition, but I don't think we should keep the old
API around in the TableAM.

> * Why do we need a separate "modify_flags" and "options"? Can't we just
> combine them into TABLE_MODIFY_* flags?
>
>
> Alexander, you had some work in this area as well, such b1484a3f19. I
> believe 0001 covers this use case in a different way: rather than
> giving complete responsibility to the AM to insert into the indexes,
> the caller provides a callback and the AM is responsible for calling it
> at the time the tuples are flushed. Is that right?
>
> The design has been out for a while, so unless others have suggestions,
> I'm considering the major design points mostly settled and I will move
> forward with something like 0001 (pending implementation issues).

Sorry about this late feedback, but while I'm generally +1 on the idea
and primary design, I feel that it doesn't quite cover all the areas
I'd expected it to cover.

Specifically, I'm having trouble seeing how this could be used to
implement ```INSERT INTO ... SELECT ... RETURNING ctid``` as I see no
returning output path for the newly inserted tuples' data, which is
usually required for our execution nodes' output path. Is support for
RETURN-clauses planned for this API? In a previous iteration, the
flush operation was capable of returning a TTS, but that seems to have
been dropped, and I can't quite figure out why.

> Note: I believe this API will extend naturally to updates and deletes,
> as well.

I have the same concern about UPDATE ... RETURNING not fitting with
this callback-based design.

Kind regards,

Matthias van de Meent
Neon (https://neon.tech)