Re: BUG #15900: `executor could not find named tuplestore` in triggers with transition table and row locks

Tom Lane <tgl@sss.pgh.pa.us>

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>
Date: 2019-07-09T15:38:13Z
Lists: pgsql-bugs
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



Commits

  1. Pass QueryEnvironment down to EvalPlanQual's EState.

  2. Store tuples for EvalPlanQual in slots, rather than as HeapTuples.