Re: Adding support for Default partition in partitioning

On Tue, May 16, 2017 at 8:57 AM, Jeevan Ladhe
<jeevan(dot)ladhe(at)enterprisedb(dot)com> wrote:
> I have fixed the crash in attached patch.
> Also the patch needed bit of adjustments due to recent commit.
> I have re-based the patch on latest commit.

+ bool has_default; /* Is there a default partition?
Currently false
+ * for a range partitioned table */
+ int default_index; /* Index of the default list
partition. -1 for
+ * range partitioned tables */

Why do we need both has_default and default_index? If default_index
== -1 means that there is no default, we don't also need a separate
bool to record the same thing, do we?

get_qual_for_default() still returns a list of things that are not
quals. I think that this logic is all pretty poorly organized. The
logic to create a partitioning constraint for a list partition should
be part of get_qual_for_list(), whether or not it is a default. And
when we have range partitions, the logic to create a default range
partitioning constraint should be part of get_qual_for_range(). The
code the way it's organized today makes it look like there are three
kinds of partitions: list, range, and default. But that's not right
at all. There are two kinds: list and range. And a list partition
might or might not be a default partition, and similarly for range.

+ ereport(ERROR, (errcode(ERRCODE_CHECK_VIOLATION),
+ errmsg("DEFAULT partition cannot be used"
+ " without negator of operator %s",
+ get_opname(operoid))));

I don't think ERRCODE_CHECK_VIOLATION is the right error code here,
and we have a policy against splitting message strings like this.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company