From: Tom Lane <tgl@sss.pgh.pa.us>
To: Thomas Munro <thomas.munro@gmail.com>
cc: Alex Aktsipetrov <alex.akts@gmail.com>,
        PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>,
        Andres Freund <andres@anarazel.de>
Subject: Re: BUG #15900: `executor could not find named tuplestore` in
 triggers with transition table and row locks
In-reply-to: 
 <CA+hUKG+3YYDHn4VzEh9hxbWta3s7dYMQk6bODbz6bYJwpNV=FQ@mail.gmail.com>
References: <15900-bc482754fe8d7415@postgresql.org>
 <CALPsoAL2x_C=hMK0M459rijUVcrchjVT6k7vFduP47iCMRh77Q@mail.gmail.com>
 <CA+hUKG+HeXeDEPBU-f0ZMViy+=VP5O8FVeLhz7u7BvXtaobv+g@mail.gmail.com>
 <CA+hUKG+XHZaCrX81CHwphG-dRTK_dmqWSmzW-vGYGEOGwZJjBw@mail.gmail.com>
 <CA+hUKG+3YYDHn4VzEh9hxbWta3s7dYMQk6bODbz6bYJwpNV=FQ@mail.gmail.com>
Comments: In-reply-to Thomas Munro <thomas.munro@gmail.com>
	message dated "Tue, 09 Jul 2019 23:47:28 +1200"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <24529.1562686693.1@sss.pgh.pa.us>
Date: Tue, 09 Jul 2019 11:38:13 -0400
Message-ID: <24530.1562686693@sss.pgh.pa.us>
Precedence: bulk

Thomas Munro <thomas.munro@gmail.com> writes:
> On Tue, Jul 9, 2019 at 1:13 PM Thomas Munro <thomas.munro@gmail.com> wrote:
>> I think it's highly likely that bug #15720 was a case of this bug and
>> would be fixed by this patch.

Agreed.  I think your version of the fix is good, and you should
mention #15720 too in the commit message.

>> Alex's repro doesn't work on 11 though,
>> because EPQ is not entered at all.  Which raises the question: why do
>> we need to enter EPQ after commit ad0bda5d on 12/master, for a row
>> that hasn't been updated by anyone else?

> Explanation: since ad0bda5d24ea, ExecLockRows() always calls
> EvalPlanQualBegin() which initialises the plan state, and in this case
> ExecInitNamedTuplestoreScan() errors out due to the bug.  Before, you
> needed the right concurrency scenario (epq_needed) before we did that,
> as the reporter of bug #15720 discovered.

I'm quite desperately unhappy about this observation, because
EvalPlanQualBegin is a *large* amount of overhead that is usually
unnecessary, and is now going to be paid for *every locked row*
whether there's any conflict on it or not.  I do not find that
acceptable.  Why is it necessary to do this before finding that
there's an update conflict?

			regards, tom lane