Re: runtime error copying oids field

Hi,
See attached patch which is along the line Alvaro outlined.

Cheers

On Mon, Nov 30, 2020 at 3:01 PM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
wrote:

> On 2020-Nov-30, Zhihong Yu wrote:
>
> > This was the line runtime error was raised:
> >
> > memcpy(part_oids, partdesc->oids, sizeof(Oid) * nparts);
> >
> > From RelationBuildPartitionDesc we can see that:
> >
> > if (nparts > 0)
> > {
> > PartitionBoundInfo boundinfo;
> > int *mapping;
> > int next_index = 0;
> >
> > result->oids = (Oid *) palloc0(nparts * sizeof(Oid));
> >
> > The cause was oids field was not assigned due to nparts being 0.
> > This is verified by additional logging added just prior to the memcpy
> call.
> >
> > I want to get the community's opinion on whether a null check should be
> > added prior to the memcpy() call.
>
> As far as I understand, we do want to avoid memcpy's of null pointers;
> see [1].
>
> In this case I think it'd be sane to skip the complete block, not just
> the memcpy, something like
>
> diff --git a/src/backend/commands/indexcmds.c
> b/src/backend/commands/indexcmds.c
> index ca24620fd0..d35deb433a 100644
> --- a/src/backend/commands/indexcmds.c
> +++ b/src/backend/commands/indexcmds.c
> @@ -1163,15 +1163,17 @@ DefineIndex(Oid relationId,
>
> if (partitioned)
> {
> + PartitionDesc partdesc;
> +
> /*
> * Unless caller specified to skip this step (via ONLY),
> process each
> * partition to make sure they all contain a corresponding
> index.
> *
> * If we're called internally (no stmt->relation), recurse
> always.
> */
> - if (!stmt->relation || stmt->relation->inh)
> + partdesc = RelationGetPartitionDesc(rel);
> + if ((!stmt->relation || stmt->relation->inh) &&
> partdesc->nparts > 0)
> {
> - PartitionDesc partdesc =
> RelationGetPartitionDesc(rel);
> int nparts = partdesc->nparts;
> Oid *part_oids = palloc(sizeof(Oid)
> * nparts);
> bool invalidate_parent = false;
>
> [1]
> https://www.postgresql.org/message-id/flat/20200904023648.GB3426768%40rfd.leadboat.com
>