Hi Jeevan,
On Tue, Aug 22, 2017 at 7:53 AM, Jeevan Ladhe
<jeevan.ladhe@enterprisedb.com> wrote:
>
> Hi Beena,
>
> On Thu, Aug 17, 2017 at 4:59 PM, Beena Emerson <memissemerson@gmail.com>
> wrote:
>>
>> PFA the patch rebased over v25 patches of default list partition [1]
>
>
> Thanks for rebasing.
>
> Range partition review:
Thank you for your review.
>
> 3.
> @@ -2396,6 +2456,8 @@ make_one_range_bound(PartitionKey key, int index, List
> *datums, bool lower)
> ListCell *lc;
> int i;
>
> + Assert(datums != NULL);
> +
> bound = (PartitionRangeBound *) palloc0(sizeof(PartitionRangeBound));
> bound->index = index;
> bound->datums = (Datum *) palloc0(key->partnatts * sizeof(Datum));
>
> I am not really convinced, why should we have above Assert.
The function should never be called for default partition where datums = NULL.
>
> 4.
> static List *
> -get_qual_for_range(PartitionKey key, PartitionBoundSpec *spec)
> +get_qual_for_range(Relation parent, PartitionBoundSpec *spec,
> + bool for_default)
> {
>
> The addition and the way flag ‘for_default’ is being used is very confusing.
> At this moment I am not able to think about a alternate solution to the
> way you have used this flag. But will try and see if I can come up with
> an alternate approach.
Well, we need to skip the get_range_nulltest while collecting the qual
of other partition to make one for default. We could skip this flag
and remove the NullTest from the qual returned for each partition but
I am not sure if thats a good way to go about it.
--
Beena Emerson
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company