From: Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>
To: Heikki Linnakangas <hlinnaka@iki.fi>
To: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
To: Amit Langote <amitlangote09@gmail.com>
Subject: Re: SEGFAULT on a concurrent UPDATE of mix of local and foreign partitions
Date: Tue, 10 Aug 2021 14:18:52 +0500
Message-ID: <706b5033-d2cd-92b1-e44e-3349ff3053fb@postgrespro.ru> (raw)
In-Reply-To: <1a6c8eab-87e5-2dce-590f-e4aef85d6e27@iki.fi>
References: <cb2b808d-cbaa-4772-76ee-c8809bafcf3d@postgrespro.ru>
<1a6c8eab-87e5-2dce-590f-e4aef85d6e27@iki.fi>
On 8/10/21 1:29 AM, Heikki Linnakangas wrote:
> 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.
As I understand the code correctly - yes, because a ModifyTable node
higher and will initialize this field earlier. But in many cases it is
not 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.
Agree. It is my first dive into the EPQ code. After studying i think
this makes no sense.
>
> 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?
ModifyTable node may be found in an underlying CTE, for example. And,
right now, it's not clear for me too.
>
> 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 segfaultYour patch looks much better.
--
regards,
Andrey Lepikhov
Postgres Professional
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: a.lepikhov@postgrespro.ru, hlinnaka@iki.fi, pgsql-bugs@lists.postgresql.org, amitlangote09@gmail.com
Subject: Re: SEGFAULT on a concurrent UPDATE of mix of local and foreign partitions
In-Reply-To: <706b5033-d2cd-92b1-e44e-3349ff3053fb@postgrespro.ru>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox