Re: [HACKERS] Default Partition for Range

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