Re: runtime error copying oids field

From: Zhihong Yu <zyu(at)yugabyte(dot)com>
To: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: runtime error copying oids field
Date: 2020-12-01 03:49:34
Message-ID: CALNJ-vSR+qqNAoLV8z_iuZjNU0y0674ASga_D_QrR=k2gd9LAA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Alvaro, et al:
Please let me know how to proceed with the patch.

Running test suite with the patch showed no regression.

Cheers

On Mon, Nov 30, 2020 at 3:24 PM Zhihong Yu <zyu(at)yugabyte(dot)com> wrote:

> 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
>>
>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2020-12-01 03:54:45 Re: [HACKERS] logical decoding of two-phase transactions
Previous Message Laurenz Albe 2020-12-01 03:38:35 Confusing behavior of psql's \e