Re: POC: Lock updated tuples in tuple_update() and tuple_delete()

Andres Freund <andres@anarazel.de>

From: Andres Freund <andres@anarazel.de>
To: Alexander Korotkov <aekorotkov@gmail.com>
Cc: Chris Travers <chris.travers@gmail.com>, pgsql-hackers@lists.postgresql.org
Date: 2023-04-01T05:21:27Z
Lists: pgsql-hackers
Hi,

On 2023-03-31 16:57:41 +0300, Alexander Korotkov wrote:
> On Wed, Mar 29, 2023 at 8:34 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> > On Mon, Mar 27, 2023 at 1:49 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> > > On Fri, Mar 24, 2023 at 3:39 AM Andres Freund <andres@anarazel.de> wrote:
> > > > On 2023-03-23 23:24:19 +0300, Alexander Korotkov wrote:
> > > > > On Thu, Mar 23, 2023 at 8:06 PM Andres Freund <andres@anarazel.de> wrote:
> > > > > > I seriously doubt that solving this at the tuple locking level is the right
> > > > > > thing. If we want to avoid refetching tuples, why don't we add a parameter to
> > > > > > delete/update to generally put the old tuple version into a slot, not just as
> > > > > > an optimization for a subsequent lock_tuple()? Then we could remove all
> > > > > > refetching tuples for triggers. It'd also provide the basis for adding support
> > > > > > for referencing the OLD version in RETURNING, which'd be quite powerful.
> > >
> > > After some thoughts, I think I like idea of fetching old tuple version
> > > in update/delete.  Everything that evades extra tuple fetching and do
> > > more of related work in a single table AM call, makes table AM API
> > > more flexible.
> > >
> > > I'm working on patch implementing this.  I'm going to post it later today.
> >
> > Here is the patchset.  I'm continue to work on comments and refactoring.
> >
> > My quick question is why do we need ri_TrigOldSlot for triggers?
> > Can't we just pass the old tuple for after row trigger in
> > ri_oldTupleSlot?
> >
> > Also, I wonder if we really need a LazyTupleSlot.  It allows to evade
> > extra tuple slot allocation.  But as I get in the end the tuple slot
> > allocation is just a single palloc.  I bet the effect would be
> > invisible in the benchmarks.
> 
> Sorry, previous patches don't even compile.  The fixed version is attached.
> I'm going to post significantly revised patchset soon.

Given that the in-tree state has been broken for a week, I think it probably
is time to revert the commits that already went in.

Greetings,

Andres Freund



Commits

  1. Add EvalPlanQual delete returning isolation test

  2. Allow locking updated tuples in tuple_update() and tuple_delete()

  3. Revert 764da7710b

  4. Revert 11470f544e

  5. Evade extra table_tuple_fetch_row_version() in ExecUpdate()/ExecDelete()

  6. Check that xmax didn't commit in freeze check.