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.
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.