Hi Beena,
On Tue, Aug 22, 2017 at 5:22 PM, Beena Emerson <memissemer...@gmail.com> wrote: > On Tue, Aug 22, 2017 at 4:20 PM, Beena Emerson <memissemer...@gmail.com> > wrote: > > Hi Jeevan, > > > > On Tue, Aug 22, 2017 at 7:53 AM, Jeevan Ladhe > > <jeevan.la...@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. 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