Re: partition routing layering in nodeModifyTable.c

From: Amit Langote <amitlangote09(at)gmail(dot)com>
To: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Daniel Gustafsson <daniel(at)yesql(dot)se>, Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: partition routing layering in nodeModifyTable.c
Date: 2020-10-23 02:56:57
Message-ID: CA+HiwqFEuq8AAAmxXsTDVZ1r38cHbfYuiPQx_=YyKe2DC-6q4A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Oct 22, 2020 at 11:25 PM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
>
> On 2020-Oct-22, Amit Langote wrote:
>
> > 0001 fixes a thinko of the recent commit 1375422c782 that I discovered
> > when debugging a problem with 0003.
>
> Hmm, how hard is it to produce a test case that fails because of this
> problem?

I checked and don't think there's any live bug here. You will notice
if you take a look at 1375422c7 that we've made es_result_relations an
array of pointers while the individual ModifyTableState nodes own the
actual ResultRelInfos. So, EvalPlanQualStart() setting the parent
EState's es_result_relations array to NULL implies that those pointers
become inaccessible to the parent query's execution after
EvalPlanQual() returns. However, nothing in the tree today accesses
ResulRelInfos through es_result_relations array, except during
ExecInit* stage (see ExecInitForeignScan()) but it would still be
intact at that stage.

With the lazy-initialization patch though, we do check
es_result_relations when trying to open a result relation to see if it
has already been initialized (a non-NULL pointer in that array means
yes), so resetting it in the middle of the execution can't be safe.
For one example, we will end up initializing the same relation many
times after not finding it in es_result_relations and also add it
*duplicatively* to es_opened_result_relations list, breaking the
invariant that that list contains distinct relations.

--
Amit Langote
EDB: http://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Masahiko Sawada 2020-10-23 02:58:16 Re: Global snapshots
Previous Message Ian Lawrence Barwick 2020-10-23 02:34:06 "unix_socket_directories" should be GUC_LIST_INPUT?