Re: [HACKERS] Default Partition for Range

Hello,

On Thu, Aug 24, 2017 at 3:08 PM, Jeevan Ladhe
<jeevan.ladhe@enterprisedb.com> wrote:
> Hi Beena,
>
>
> On Tue, Aug 22, 2017 at 5:22 PM, Beena Emerson <memissemerson@gmail.com>
> wrote:
>>
>> On Tue, Aug 22, 2017 at 4:20 PM, Beena Emerson <memissemerson@gmail.com>
>> wrote:
>> > Hi Jeevan,
>> >
>> > On Tue, Aug 22, 2017 at 7:53 AM, Jeevan Ladhe
>> > <jeevan.ladhe@enterprisedb.com> wrote:
>> >>
>> >>
>> >> 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.
>> >
>> >
>>
>> Please find attached a patch which removes the for_default flag from
>> the get_qual_for_range function. This patch is just to show an idea
>> and should be applied over my earlier patch. There could be a better
>> way to do this. Let me know your opinion.
>>
>
> +
> + foreach (lc, list_tmp)
> + {
> + Node *n = (Node *) lfirst(lc);
> +
> + if (IsA(n, NullTest))
> + {
> + list_delete(part_qual, n);
> + count++;
> + }
> + }
> +
>
> I think its an unnecessary overhead to have the nulltest prepared first
> and then search for it and remove it from the partition qual. This is very
> ugly.

Yes, I felt the same but just put it out there to see if someone can
improve on this.

> I think the original idea is still better compared to this. May be we can
> rename
> the 'for_default' flag to something like 'part_of_default_qual'.
>

Ya. I think that would work.



--

Beena Emerson

EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company