On Mon, Jun 5, 2017 at 1:43 AM, Beena Emerson <memissemerson@gmail.com> wrote:
> The new patch is rebased over default_partition_v18.patch
> [http://www.mail-archive.com/pgsql-hackers@postgresql.org/msg315831.html]
I have done the initial review of the patch, I have few comments.
+ if ((lower->content[0] == RANGE_DATUM_DEFAULT))
+ {
+ if (partition_bound_has_default(partdesc->boundinfo))
+ {
+ overlap = true;
+ with = partdesc->boundinfo->default_index;
+ }
I think we can change if ((lower->content[0] == RANGE_DATUM_DEFAULT))
check to if (spec->is_default) that way it will be consistent with the
check in the PARTITION_STRATEGY_LIST. Or we can move this complete
check outside of the switch.
-------------
- PartitionRangeDatum *datum = castNode(PartitionRangeDatum, lfirst(lc));
+ Node *node = lfirst(lc);
+ PartitionRangeDatum *datum;
+
+ datum = castNode(PartitionRangeDatum, node);
Why do you need to change this?
--------------
+ if (!datums)
+ bound->content[i] = RANGE_DATUM_DEFAULT;
+
Better to check if (datums != NULL) for non boolean types.
-------------
+ if (content1[i] == RANGE_DATUM_DEFAULT ||
+ content2[i] == RANGE_DATUM_DEFAULT)
+ {
+ if (content1[i] == content2[i])
+ return 0;
+ else if (content1[i] == RANGE_DATUM_DEFAULT)
+ return -1;
I don't see any comments why default partition should be considered
smallest in the index comparison. For negative infinity, it's pretty
obvious by the enum name itself.
-------------
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com