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
