On Mon, Oct 9, 2017 at 5:51 PM, Ashutosh Bapat <ashutosh.ba...@enterprisedb.com> wrote: > On Mon, Oct 9, 2017 at 4:44 PM, amul sul <sula...@gmail.com> wrote: >
Thanks Ashutosh for your review, please find my comment inline.
>
>> 0002 few changes in partition-wise join code to support
>> hash-partitioned table as well & regression tests.
>
> + switch (key->strategy)
> + {
> + case PARTITION_STRATEGY_HASH:
> + /*
> + * Indexes array is same as the greatest modulus.
> + * See partition_bounds_equal() for more explanation.
> + */
> + num_indexes = DatumGetInt32(src->datums[ndatums - 1][0]);
> + break;
> This logic is duplicated at multiple places. I think it's time we consolidate
> these changes in a function/macro and call it from the places where we have to
> calculate number of indexes based on the information in partition descriptor.
> Refactoring existing code might be a separate patch and then add hash
> partitioning case in hash partitioning patch.
>
Make sense, added get_partition_bound_num_indexes() to get number of index
elements in 0001 & get_greatest_modulus() as name suggested to get the greatest
modulus of the hash partition bound in 0002.
> + int dim = hash_part? 2 : partnatts;
> Call the variable as natts_per_datum or just natts?
>
natts represents the number of attributes, but for the hash partition bound we
are not dealing with the attribute so that I have used short-form of dimension,
thoughts?
> + hash_part? true : key->parttypbyval[j],
> + key->parttyplen[j]);
> parttyplen is the length of partition key attribute, whereas what you want
> here
> is the length of type of modulus and remainder. Is that correct? Probably we
> need some special handling wherever parttyplen and parttypbyval is used e.g.
> in
> call to partition_bounds_equal() from build_joinrel_partition_info().
>
Unless I am missing something, I don't think we should worry about parttyplen
because in the datumCopy() when the datatype is pass-by-value then typelen
is ignored.
Regards,
Amul
0001-partition_bounds_copy-code-refactoring-v1.patch
Description: Binary data
0002-hash-partitioning_another_design-v24.patch
Description: Binary data
0003-Enable-partition-wise-join-support-v2.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
