On Tue, Mar 14, 2017 at 6:28 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
> On 2017/03/14 9:17, Robert Haas wrote:
>> On Mon, Mar 13, 2017 at 3:24 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>>> Haven't looked at 0007 yet.
>>
>> Overall this one looks pretty good and straightforward.
>
> In the following code of find_partition_scheme():
>
> + /* Did not find matching partition scheme. Create one. */
> + part_scheme = (PartitionScheme) palloc0(sizeof(PartitionSchemeData));
> +
> + /* Copy partition bounds/lists. */
> + part_scheme->nparts = part_desc->nparts;
> + part_scheme->strategy = part_key->strategy;
> + part_scheme->boundinfo = part_desc->boundinfo;
> +
> + /* Store partition key information. */
> + part_scheme->partnatts = part_key->partnatts;
> +
> + part_scheme->partopfamily = (Oid *) palloc(sizeof(Oid) * partnatts);
> + memcpy(part_scheme->partopfamily, part_key->partopfamily,
> + sizeof(Oid) * partnatts);
> +
> + part_scheme->partopcintype = (Oid *) palloc(sizeof(Oid) * partnatts);
> + memcpy(part_scheme->partopcintype, part_key->partopcintype,
> + sizeof(Oid) * partnatts);
> +
> + part_scheme->key_types = (Oid *) palloc(sizeof(Oid) * partnatts);
> + memcpy(part_scheme->key_types, part_key->parttypid,
> + sizeof(Oid) * partnatts);
> +
> + part_scheme->key_typmods = (int32 *) palloc(sizeof(int32) * partnatts);
> + memcpy(part_scheme->key_typmods, part_key->parttypmod,
> + sizeof(int32) * partnatts);
> +
> + part_scheme->key_collations = (Oid *) palloc(sizeof(Oid) * partnatts);
> + memcpy(part_scheme->key_collations, part_key->parttypcoll,
> + sizeof(Oid) * partnatts);
>
> Couldn't we avoid the memcpy() on individual members of part_key? After
> all, RelationData.rd_partkey is guarded just like rd_partdesc by
> relcache.c in face of invalidations (see keep_partkey logic in
> RelationClearRelation).
This suggestion looks good to me. Incorporated in the latest set of patches.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
