From: | Dilip Kumar <dilipbalaut(at)gmail(dot)com> |
---|---|
To: | Beena Emerson <memissemerson(at)gmail(dot)com> |
Cc: | Rafia Sabih <rafia(dot)sabih(at)enterprisedb(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Jeevan Ladhe <jeevan(dot)ladhe(at)enterprisedb(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Default Partition for Range |
Date: | 2017-06-05 15:14:15 |
Message-ID: | CAFiTN-u5Uirpg4-4bhwR8o77tG5ass66w5oqDm-SJWd4Kdwybg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Jun 5, 2017 at 1:43 AM, Beena Emerson <memissemerson(at)gmail(dot)com> wrote:
> The new patch is rebased over default_partition_v18.patch
> [http://www.mail-archive.com/pgsql-hackers(at)postgresql(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2017-06-05 15:19:41 | shm_toc_lookup API |
Previous Message | Tom Lane | 2017-06-05 14:59:07 | Re: intermittent failures in Cygwin from select_parallel tests |