On Mon, Jun 5, 2017 at 1:43 AM, Beena Emerson <memissemer...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers