On Fri, May 12, 2017 at 8:08 AM, Amit Langote <langote_amit...@lab.ntt.co.jp> wrote: > On 2017/05/12 11:20, Robert Haas wrote: >> On Thu, May 11, 2017 at 10:15 PM, Amit Langote >> <langote_amit...@lab.ntt.co.jp> wrote: >>> On 2017/05/12 10:42, Robert Haas wrote: >>>> On Thu, May 11, 2017 at 12:02 PM, Dilip Kumar <dilipbal...@gmail.com> >>>> wrote: >>>>> We need to add PARTITION_STRATEGY_HASH as well, we don't support NULL >>>>> for hash also, right? >>>> >>>> I think it should. >>>> >>>> Actually, I think that not supporting nulls for range partitioning may >>>> have been a fairly bad decision. >>> >>> I think the relevant discussion concluded [1] that way, because we >>> couldn't decide which interface to provide for specifying where NULLs are >>> placed or because we decided to think about it later. >> >> Yeah, but I have a feeling that marking the columns NOT NULL is going >> to make it really hard to support that in the future when we get the >> syntax hammered out. If it had only affected the partition >> constraints that'd be different. > > So, adding keycol IS NOT NULL (like we currently do for expressions) in > the implicit partition constraint would be more future-proof than > generating an actual catalogued NOT NULL constraint on the keycol? I now > tend to think it would be better. Directly inserting into a range > partition with a NULL value for a column currently generates a "null value > in column \"%s\" violates not-null constraint" instead of perhaps more > relevant "new row for relation \"%s\" violates partition constraint". > That said, we *do* document the fact that a NOT NULL constraint is added > on range key columns, but we might as well document instead that we don't > currently support routing tuples with NULL values in the partition key > through a range-partitioned table and so NULL values cause error.
in get_partition_for_tuple() we have if (key->strategy == PARTITION_STRATEGY_RANGE) { /* Disallow nulls in the range partition key of the tuple */ for (i = 0; i < key->partnatts; i++) if (isnull[i]) ereport(ERROR, (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED), errmsg("range partition key of row contains null"))); } Instead of throwing an error here, we should probably return -1 and let the error be ""no partition of relation \"%s\" found for row", which is the real error, not having a partition which can accept NULL. If in future we decide to support NULL values in partition keys, we need to just remove above code from get_partition_for_tuple() and everything will work as is. I am assuming that we don't add any implicit/explicit NOT NULL constraint right now. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers