Re: SEGFAULT on a concurrent UPDATE of mix of local and foreign partitions

On 06/08/2021 08:34, Andrey Lepikhov wrote:
> Hi,
> Postgres SEGFAULT'ed on the UPDATE of mix of local and foreign partitions.
> Initialization - see t.sql
> For replaying this segfault just execute in parallel:
> UPDATE test SET x = x - 1;
> 
> The problem was introduced by commit 1375422.
> ExecUpdate has found a concurrently updated tuples and starts subplan
> evaluation. This operation creates new EState for EPQState and sets
> es_result_relations in NULL value. Next, ExecInitNode(subplan) is
> launched and underlying ExecInitForeignScan tries to access to an
> element of es_result_relations. This causes SEGFAULT.

Thanks for the report!

> I studied this problem shortly. I think, EPQState can use
> es_result_relations of a parent EState. Patch in attachment fixes this.
> check-world passed clearly.

Hmm, that seems to work, but I have a few questions:

Can we be sure that the es_result_relations entry of the result relation 
has been initialized when the EPQState is constructed? The general model 
now is that you initialize them lazily, when needed.

Does it ever make sense to re-evaluate a direct Foreign Update as part 
of EvalPlanQual? It doesn't seem sane to me, you cannot just run an 
UPDATE statement again and expect it to be idempotent. So I think it 
would make sense skip this during EvalPlanQual processing altogether. 
And no need to call the FDW's Begin/EndDirectModify functions either.

It's not clear to me what guarantee there is that a direct-modify 
Foreign Update/Delete node is never re-evaluated as part of 
EvalPlanQual. Is it because the a ModifyTable node is always at the top 
of the plan tree? Or is it because the FDW never uses direct-modify in 
queries where EvalPlanQual might be needed? Or because we use 
ROW_MARK_COPY in those cases?

In any case, re-evaluating a direct-modify statement doesn't seem sane, 
so I propose the attached patch to add some runtime checks for that and 
to avoid the original segfault.

- Heikki