From: Tom Lane <tgl@sss.pgh.pa.us>
To: Amit Langote <amitlangote09@gmail.com>
Cc: Justin Pryzby <pryzby@telsasoft.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Subject: Re: FailedAssertion("pd_idx == pinfo->nparts", File: "execPartition.c", Line: 1689)
Date: Wed, 05 Aug 2020 13:30:30 -0400
Message-ID: <2528807.1596648630@sss.pgh.pa.us> (raw)
In-Reply-To: <CA+HiwqETTgTh5SG9bcAdYRahEeTgpPHy+Xi=b93icoTX7j2EFg@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>
Amit Langote <amitlangote09@gmail.com> writes:
> The crash reported here is in the other case where the concurrently
> added partitions cause the execution-time PartitionDesc to have more
> partitions than the one that PartitionedRelPruneInfo is based on.
> I was able to reproduce such a crash as follows:
Yeah, I can repeat the case per these directions. I concur that the
issue is that ExecCreatePartitionPruneState is failing to cope with
zeroes in the relid_map.
> The attached patch should fix that.
I don't like this patch at all though; I do not think it is being nearly
careful enough to ensure that it's matched the surviving relation OIDs
correctly. In particular it blithely assumes that a zero in relid_map
*must* match the immediately next entry in partdesc->oids, which is easy
to break if the new partition is adjacent to the one the planner managed
to prune. So I think we should do it more like the attached.
I'm strongly tempted to convert the trailing Assert to an actual
test-and-elog, too, but didn't do so here.
regards, tom lane
Attachments:fix-sloppy-partition-mapping-2.patch (text/x-diff, 1.6K) downloaddiff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.cindex fb6ce49056..221a34e738 100644--- a/src/backend/executor/execPartition.c+++ b/src/backend/executor/execPartition.c@@ -1673,7 +1673,19 @@ ExecCreatePartitionPruneState(PlanState *planstate,
pprune->subpart_map = palloc(sizeof(int) * partdesc->nparts);
for (pp_idx = 0; pp_idx < partdesc->nparts; ++pp_idx)
{
- if (pinfo->relid_map[pd_idx] != partdesc->oids[pp_idx])+ /*+ * pinfo->relid_map can contain InvalidOid entries for+ * partitions pruned by the planner. We cannot tell+ * exactly which of the partdesc entries these correspond+ * to, but we don't have to; just skip over them. The+ * non-pruned relid_map entries, however, had better be a+ * subset of the partdesc entries and in the same order.+ */+ while (pd_idx < pinfo->nparts &&+ !OidIsValid(pinfo->relid_map[pd_idx]))+ pd_idx++;+ if (pd_idx >= pinfo->nparts ||+ pinfo->relid_map[pd_idx] != partdesc->oids[pp_idx])
{
pprune->subplan_map[pp_idx] = -1;
pprune->subpart_map[pp_idx] = -1;
@@ -1686,6 +1698,14 @@ ExecCreatePartitionPruneState(PlanState *planstate,
pinfo->subpart_map[pd_idx++];
}
}
++ /*+ * It might seem that we need to skip any trailing InvalidOid+ * entries in pinfo->relid_map before asserting that we+ * scanned all of the relid_map. But we will have skipped+ * them above, because they must correspond to some+ * partdesc->oids entries; we just couldn't tell which.+ */
Assert(pd_idx == pinfo->nparts);
}
=
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, pryzby@telsasoft.com, robertmhaas@gmail.com
Subject: Re: FailedAssertion("pd_idx == pinfo->nparts", File: "execPartition.c", Line: 1689)
In-Reply-To: <2528807.1596648630@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