Re: Default Partition for Range

Hi Beena,

On Tue, Aug 22, 2017 at 5:22 PM, Beena Emerson <memissemerson(at)gmail(dot)com>
wrote:

> On Tue, Aug 22, 2017 at 4:20 PM, Beena Emerson <memissemerson(at)gmail(dot)com>
> wrote:
> > Hi Jeevan,
> >
> > On Tue, Aug 22, 2017 at 7:53 AM, Jeevan Ladhe
> > <jeevan(dot)ladhe(at)enterprisedb(dot)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.
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'.

Also, as discussed offline I will merge this with default list partitioning
patch.

Regards,
Jeevan Ladhe