Hi Beena, On Thu, Aug 17, 2017 at 4:59 PM, Beena Emerson <memissemer...@gmail.com> wrote:
> PFA the patch rebased over v25 patches of default list partition [1] > Thanks for rebasing. Range partition review: 1. There are lot of changes in RelationBuildPartitionDesc(). It was hard to understand why these changes are needed for default partition. I did not find any explanation for the same, may be I am missing some discussion? Only when I looked into it carefully I could understand that these changes are nothing but optimization for identifying the distinct bounds. I think it is better to keep the patch for code optimisation/simplification in a separate patch. I have separated the patch(0001) in attached patches for this purpose. 2. + * For default partition, it returns the negation of the constraints of all + * the other partitions. + * + * If we end up with an empty result list, we return NULL. We can rephrase the comment as below: "For default partition, function returns the negation of the constraints of all the other partitions. If default is the only partition then returns NULL." 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. 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. I still need to look at the test, and need to do some manual testing. Will update here with any findings. Patches: 0001: Refactoring/simplification of code in RelationBuildPartitionDesc(), 0002: implementation of default range partition by Beena. Regards, Jeevan
0001-Refactor-RelationBuildPartitionDesc.patch
Description: Binary data
0002-Add-support-for-default-partition-for-range.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers