Re: FOR PORTION OF code review

Hi,

On 2026-09-11 10:56:40 -0700, Paul A Jungwirth wrote:
> On Fri, Sep 11, 2026 at 10:30 AM Andres Freund <andres(at)anarazel(dot)de> wrote:
> >
> > On 2026-09-11 12:49:54 -0400, Andres Freund wrote:
> > > I wonder if there may be additional issues with DELETE ... FOR PORTION OF, due
> > > to not having a ctid chain to follow.
> >
> > Yep. There's lost updates even with full-key locks, once DELETE FPO enters the
> > picture.
> >
> > See the AI generated spec file (although I really needed to force both Opus 5
> > and Fable 5.1 to get to it, they both swore up and down that this isn't a real
> > issue at first).
> >
> >
> > The problem is that with UPDATE FPO different backends serialize on the
> > surviving row, allowing only one backend to acquire the FOR UPDATE lock on
> > that row, with the other transaction waiting for the second transaction to
> > either abort, or to lock the subsequent row.
> >
> > But with DELETE FPO, there's no such serialization, once the first transaction
> > commits all concurrent FOR UPDATEs complete, *without* needing a row lock.
> >
> >
> > So I think either FPO needs a fair bit more work (e.g. using the speculative
> > insert infrastructure from ON CONFLICT and/or perhaps some careful scanning
> > with a dirty snapshot), or the feature ought to just refuse to be used with
> > READ COMMITTED. I'm a bit sceptical that the latter is acceptable. And the
> > former seems very clearly out of scope for 19.
>
> Thanks for diving into the concurrency issues. I think it is the most
> serious issue here. I was a little surprised that it was originally
> considered acceptable, actually, and I would be happy to change it.

FWIW, personally I don't think it is acceptable. I'm quite baffled that it was
committed with such a glaring hole.

> Not having a successful workaround makes it even more of a problem.
>
> Rather than forbidding READ COMMITTED at all, I think we should raise
> a serialization failure. Then the user can retry.

Personally I don't find that it's a convincing feature with that
limitation.

For ON CONFLICT Peter (and, to a lesser degree I) spent a *lot* of time
getting the concurrency behaviour somewhat right. I think this feature really
ought to have working concurrency behaviour, not a cheap copout. For better
or worse, READ COMMITTED is extremely widely used, with one reason for that
being not needing to retry, and making FPO not really work that way doesn't
seem convincing to me.

> Do you have any objections to that approach?

I think it this should be properly fixed instead.

That'll likely require a protocol of scanning for relevant rows with something
like ExecCheckIndexConstraints() (modified to be able to search for multiple
conflicts), making sure those are locked, with a visibility check for higher
isolation levels. I suspect it'll be hard to get this right without requiring
a WITHOUT OVERLAPS index, but I'm not sure.

Greetings,

Andres Freund