From: Tom Lane <tgl@sss.pgh.pa.us>
To: Amit Langote <amitlangote09@gmail.com>
Cc: Andy Fan <zhihui.fan1213@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Justin Pryzby <pryzby@telsasoft.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Subject: Re: FailedAssertion("pd_idx == pinfo->nparts", File: "execPartition.c", Line: 1689)
Date: Thu, 06 Aug 2020 23:31:28 -0400
Message-ID: <2717732.1596771088@sss.pgh.pa.us> (raw)
In-Reply-To: <CA+HiwqGr=vPwPfwKcc+biEc5LxhLE4fWYnBGmzv-vUNq+_fp+Q@mail.gmail.com>
References: <20200802181131.GA27754@telsasoft.com>
<CA+TgmoawZkbuvP5BDBNNVp18F4tuwaZtqKUFxtH61QX7CiAV=Q@mail.gmail.com>
<20200803161133.GA21372@telsasoft.com>
<CA+HiwqETTgTh5SG9bcAdYRahEeTgpPHy+Xi=b93icoTX7j2EFg@mail.gmail.com>
<2528807.1596648630@sss.pgh.pa.us>
<CA+TgmoYMqqqHaOpfMYs8R6VJekykfRABCD=o4ovN2eUz-efYMA@mail.gmail.com>
<2550497.1596651719@sss.pgh.pa.us>
<CAKU4AWpEMxre9zDs-Odx7PnYd7tW3MiAyYBaP=9QrjuD7ps=MQ@mail.gmail.com>
<2598266.1596686548@sss.pgh.pa.us>
<CAKU4AWo0wDDabURj9FQoJU6p6yczy8fk48gSvTTdq1kYPAcWoQ@mail.gmail.com>
<2626504.1596724930@sss.pgh.pa.us>
<CAKU4AWoyeg=K9HXhtubnVwBZnyfVouJw+5LwCS7akyh1BD0-QA@mail.gmail.com>
<2685059.1596760353@sss.pgh.pa.us>
<CA+HiwqGr=vPwPfwKcc+biEc5LxhLE4fWYnBGmzv-vUNq+_fp+Q@mail.gmail.com>
Amit Langote <amitlangote09@gmail.com> writes:
> On Fri, Aug 7, 2020 at 9:32 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> ... Amit's recipe for reproducing the bug works because there are
>> other relation lock acquisitions (and hence AcceptInvalidationMessages
>> calls) later in planning than where he asked us to wait. So this
>> effectively tests a scenario where a very late A.I.M. call within the
>> planner detects an inval event for some already-planned relation, and
>> that seems like a valid-enough scenario.
> Agreed.
> Curiously, Justin mentioned upthread that the crash occurred during
> BIND of a prepared query, so it better had been that a custom plan was
> being executed, because a generic one based on fewer partitions would
> be thrown away due to A.I.M. invoked during AcquireExecutorLocks().
Based on the above, it seems plausible that the plancache did throw away
an old plan and try to replan, but the inval message announcing partition
addition arrived too late during that planning cycle. Just like the
normal execution path, the plancache code path won't do more than one
iteration of planning on the way to a demanded query execution.
>> (BTW, I checked and found that this test does *not* expose the problems
>> with Amit's original patch. Not sure if it's worth trying to finagle
>> it so it does.)
> I tried to figure out a scenario where my patch would fail but
> couldn't come up with one either, but it's no proof that it isn't
> wrong. For example, I can see that pinfo->relid_map[pinfo->nparts]
> can be accessed with my patch which is not correct.
Yeah, touching array entries off the end of the relid_map array definitely
seems possible with that coding. But the scenario I was worried about
was that the loop actually attaches the wrong subplan (one for a different
partition) to a partdesc entry. In an assert-enabled build, that would
have led to assertion failure just below, because then we could not match
up all the remaining relid_map entries; but in a non-assert build, we'd
plow through and bad things would likely happen during execution.
You might need further conditions, like the partitions not being all
identical, for that to actually cause any problem. I'd poked at this
for a little bit without causing an obvious crash, but I can't claim
to have tried hard.
regards, tom lane
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-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, amitlangote09@gmail.com, zhihui.fan1213@gmail.com, robertmhaas@gmail.com, pryzby@telsasoft.com
Subject: Re: FailedAssertion("pd_idx == pinfo->nparts", File: "execPartition.c", Line: 1689)
In-Reply-To: <2717732.1596771088@sss.pgh.pa.us>
* 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