Обсуждение: [HACKERS] NOT NULL constraints on range partition key columns
Starting a new thread to discuss the proposal I put forward in [1] to stop creating explicit NOT NULL constraint on range partition keys that are simple columns. I said the following: On 2017/05/12 11:20, Robert Haas wrote: > On Thu, May 11, 2017 at 10:15 PM, Amit Langote > <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote: >> On 2017/05/12 10:42, Robert Haas wrote: >>> 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. Can we still decide to do that instead? Thanks, Amit [1] https://www.postgresql.org/message-id/f52fb5c3-b6ad-b6ff-4bb6-145fdb805fa6%40lab.ntt.co.jp
On Mon, May 15, 2017 at 11:46 AM, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote: > Starting a new thread to discuss the proposal I put forward in [1] to stop > creating explicit NOT NULL constraint on range partition keys that are > simple columns. I said the following: > > On 2017/05/12 11:20, Robert Haas wrote: >> On Thu, May 11, 2017 at 10:15 PM, Amit Langote >> <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote: >>> On 2017/05/12 10:42, Robert Haas wrote: >>>> 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. > Can't we allow NULL to get inserted into the partition (leaf partition) if the user uses the partition name in Insert statement? For root partitions, I think for now giving an error is okay, but once we have default partitions (Rahila's patch), we can route NULLS to default partition. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
On Mon, May 15, 2017 at 9:12 AM, Amit Kapila <amit.kapila16@gmail.com> wrote: > Can't we allow NULL to get inserted into the partition (leaf > partition) if the user uses the partition name in Insert statement? That would be terrible behavior - the behavior of tuple routing should match the enforced constraints. > For root partitions, I think for now giving an error is okay, but once > we have default partitions (Rahila's patch), we can route NULLS to > default partition. Yeah, that's exactly why I think we should make the change Amit is proposing here. If we don't, then we won't be able to accept NULL values even after we have the default partitioning stuff. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 2017/05/16 4:29, Robert Haas wrote: > On Mon, May 15, 2017 at 9:12 AM, Amit Kapila <amit.kapila16@gmail.com> wrote: >> Can't we allow NULL to get inserted into the partition (leaf >> partition) if the user uses the partition name in Insert statement? > > That would be terrible behavior - the behavior of tuple routing should > match the enforced constraints. > >> For root partitions, I think for now giving an error is okay, but once >> we have default partitions (Rahila's patch), we can route NULLS to >> default partition. > > Yeah, that's exactly why I think we should make the change Amit is > proposing here. If we don't, then we won't be able to accept NULL > values even after we have the default partitioning stuff. Attached is a patch for consideration. There are 2 actually, but maybe they should be committed together if we decide do go with this. Thanks, Amit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Вложения
On Tue, May 16, 2017 at 3:26 PM, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote: > On 2017/05/16 4:29, Robert Haas wrote: >> On Mon, May 15, 2017 at 9:12 AM, Amit Kapila <amit.kapila16@gmail.com> wrote: >>> Can't we allow NULL to get inserted into the partition (leaf >>> partition) if the user uses the partition name in Insert statement? >> >> That would be terrible behavior - the behavior of tuple routing should >> match the enforced constraints. >> Right and that makes sense. >>> For root partitions, I think for now giving an error is okay, but once >>> we have default partitions (Rahila's patch), we can route NULLS to >>> default partition. >> >> Yeah, that's exactly why I think we should make the change Amit is >> proposing here. If we don't, then we won't be able to accept NULL >> values even after we have the default partitioning stuff. > > Attached is a patch for consideration. There are 2 actually, but maybe > they should be committed together if we decide do go with this. > After your changes in get_qual_for_range(), below comment in that function needs an update and I feel it is better to update the function header as well. /* * A range-partitioned table does not allow partition keys to be null. For * simple columns, their NOT NULL constraint suffices for the enforcement * of non-nullability. But for the expression keys, which are still * nullable, we must emit a IS NOT NULL expression. Collect them in * result first. */ -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
On 2017/05/16 21:16, Amit Kapila wrote: > On Tue, May 16, 2017 at 3:26 PM, Amit Langote > <Langote_Amit_f8@lab.ntt.co.jp> wrote: >> On 2017/05/16 4:29, Robert Haas wrote: >>> Yeah, that's exactly why I think we should make the change Amit is >>> proposing here. If we don't, then we won't be able to accept NULL >>> values even after we have the default partitioning stuff. >> >> Attached is a patch for consideration. There are 2 actually, but maybe >> they should be committed together if we decide do go with this. >> > > After your changes in get_qual_for_range(), below comment in that > function needs an update and I feel it is better to update the > function header as well. > > /* > * A range-partitioned table does not allow partition keys to be null. For > * simple columns, their NOT NULL constraint suffices for the enforcement > * of non-nullability. But for the expression keys, which are still > * nullable, we must emit a IS NOT NULL expression. Collect them in > * result first. > */ Thanks for the review. I updated the comments. Thanks, Amit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Вложения
On Tue, May 16, 2017 at 8:19 PM, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote: > Thanks for the review. I updated the comments. I found several more places that also needed to be updated using 'git grep'. Committed with various additions. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 2017/05/19 3:06, Robert Haas wrote: > On Tue, May 16, 2017 at 8:19 PM, Amit Langote > <Langote_Amit_f8@lab.ntt.co.jp> wrote: >> Thanks for the review. I updated the comments. > > I found several more places that also needed to be updated using 'git > grep'. Committed with various additions. Thanks a lot. Regards, Amit