Re: [HACKERS] Prevent ALTER TABLE DROP NOT NULL on child tables if parent column has it
Michael Paquier writes: > On Wed, Jun 15, 2016 at 10:34 PM, Tom Lane wrote: >> My feeling is that we'd keep >> the pg_attribute.attnotnull field and continue to drive actual enforcement >> off that, but it would just reflect a summary of the pg_constraint state. > OK, I see. Hm, by storing this information I would actually think that > we want to drop this attnotnull so as we don't need to bother about > updating pg_attribute through the whole tree when dropping a NOT NULL > constraint on the parent, and we do not actually need to store this > information in two different places.. There are a couple of reasons for not removing attnotnull: one is to not need to touch the executor for this, and another is to not break client-side code that is accustomed to looking at attnotnull. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Prevent ALTER TABLE DROP NOT NULL on child tables if parent column has it
On 6/15/16, Michael Paquier wrote: > On Thu, Jun 16, 2016 at 12:10 PM, Vitaly Burovoy > wrote: >> In the initial letter[1] I posted a digest from the SQL-2011 standard >> and a conclusion as a design of a new patch. >> Now I have more free time and I'm hacking it that way. The new patch >> is at the very early stage, full of WIPs and TODOs. I hope it'll be >> ready to be shown in a month (may be two). > > I have just read both your patch and the one of Alvaro, but yours does > not touch pg_constraint in any way. Isn't that unexpected? The consensus was reached to use CHECK constraints instead of new type of constrains. Alvaro made attempt[1] to write PoC in 2012 but it failed to apply on top of master (and after solving conflicts led to core dumps) in Jan 2016. I just rebased Alvaro's one to understand how he wanted to solve issue and to run tests and queries. After all I sent rebased working patch for anyone who wants to read it and try it without core dumps. I have not published my patch for NOT NULLs yet. Alvaro, the correct path[2] in the second message[3] of the thread. What's wrong in it (I got the source in the previous[1] thread)? [1] https://www.postgresql.org/message-id/flat/1343682669-sup-2...@alvh.no-ip.org [2] https://www.postgresql.org/message-id/attachment/41886/catalog-notnull-2-c477e84_cleaned.patch [3] https://www.postgresql.org/message-id/CAKOSWNnXbOY4pEiwN9wePOx8J%2BB44yTj40BQ8RVo-eWkdiGDFQ%40mail.gmail.com -- Best regards, Vitaly Burovoy -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Prevent ALTER TABLE DROP NOT NULL on child tables if parent column has it
On Thu, Jun 16, 2016 at 1:27 PM, Alvaro Herrera wrote: > Michael Paquier wrote: >> On Wed, Jun 15, 2016 at 10:34 PM, Tom Lane wrote: > >> > IIRC, Alvaro posted a WIP patch for that awhile back. Not sure what the >> > current state is. >> >> Are you talking about that? >> https://www.postgresql.org/message-id/20110707213401.GA27098%40alvh.no-ip.org >> This is not a small patch :) >> >> Alvaro, others, any opinions? > > I haven't looked at this in a while. I suppose I should get it in shape > sometime. Unless you would like to work on it? Well, I could, but there is another big patch I am getting into shape for the next CF (Ahem. scram. Ahem), so I am going to stay focused on this one to have fuel for it during the CF. But I'm fine reviewing the patch for the DROP NOT NULL. > Note that Vitaly Burovoy rebased it, but I think the rebase is wrong. > https://www.postgresql.org/message-id/cakoswnkn6hsyatuys8xzxzrcr-kl1okhs5-b9qd9bf1rad3...@mail.gmail.com Yes, it is definitely wrong. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Prevent ALTER TABLE DROP NOT NULL on child tables if parent column has it
Michael Paquier wrote: > On Wed, Jun 15, 2016 at 10:34 PM, Tom Lane wrote: > > IIRC, Alvaro posted a WIP patch for that awhile back. Not sure what the > > current state is. > > Are you talking about that? > https://www.postgresql.org/message-id/20110707213401.GA27098%40alvh.no-ip.org > This is not a small patch :) > > Alvaro, others, any opinions? I haven't looked at this in a while. I suppose I should get it in shape sometime. Unless you would like to work on it? Note that Vitaly Burovoy rebased it, but I think the rebase is wrong. https://www.postgresql.org/message-id/cakoswnkn6hsyatuys8xzxzrcr-kl1okhs5-b9qd9bf1rad3...@mail.gmail.com -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Prevent ALTER TABLE DROP NOT NULL on child tables if parent column has it
On Thu, Jun 16, 2016 at 12:10 PM, Vitaly Burovoy wrote: > In the initial letter[1] I posted a digest from the SQL-2011 standard > and a conclusion as a design of a new patch. > Now I have more free time and I'm hacking it that way. The new patch > is at the very early stage, full of WIPs and TODOs. I hope it'll be > ready to be shown in a month (may be two). I have just read both your patch and the one of Alvaro, but yours does not touch pg_constraint in any way. Isn't that unexpected? > But it already forbids dropping NOT NULLs if they were set as result > of inheritance. Okay, I'll drop any proposal on my side in this case. Looking forward to seeing what you got for the first commit fest of 10.0. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Prevent ALTER TABLE DROP NOT NULL on child tables if parent column has it
On 6/15/16, Tom Lane wrote: > Michael Paquier writes: >> To put it short, it should not be possible to drop a NOT NULL >> constraint on a child relation when its parent table is using it, this >> should only be possible from the parent. Attached is a patch handling >> this problem by adding a new function in pg_inherits.c to fetch the >> list of parent relations for a given relation OID, and did some >> refactoring to stick with what is done when scanning child relations. > > This doesn't sound like the right approach; in particular, it won't really > help for deciding whether to propagate a DROP NOT NULL on a parent rel to > its children. What we've discussed in the past is to store NOT NULL > constraints in pg_constraint, much like CHECK constraints are already, and > use use-count logic identical to the CHECK case to keep track of whether > NOT NULL constraints are inherited or not. My feeling is that we'd keep > the pg_attribute.attnotnull field and continue to drive actual enforcement > off that, but it would just reflect a summary of the pg_constraint state. > > IIRC, Alvaro posted a WIP patch for that awhile back. Not sure what the > current state is. > > regards, tom lane The last thread about NOT NULLs as constraints is accessible by the link[1]. I rebased[2] Alvaro's patch to the actual master at that time, but I have not repeated it since then. In the initial letter[1] I posted a digest from the SQL-2011 standard and a conclusion as a design of a new patch. Now I have more free time and I'm hacking it that way. The new patch is at the very early stage, full of WIPs and TODOs. I hope it'll be ready to be shown in a month (may be two). But it already forbids dropping NOT NULLs if they were set as result of inheritance. [1] https://www.postgresql.org/message-id/flat/CAKOSWNkN6HSyatuys8xZxzRCR-KL1OkHS5-b9qd9bf1Rad3PLA%40mail.gmail.com [2] https://www.postgresql.org/message-id/attachment/41886/catalog-notnull-2-c477e84_cleaned.patch -- Best regards, Vitaly Burovoy -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Prevent ALTER TABLE DROP NOT NULL on child tables if parent column has it
On Wed, Jun 15, 2016 at 10:34 PM, Tom Lane wrote: > Michael Paquier writes: > This doesn't sound like the right approach; in particular, it won't really > help for deciding whether to propagate a DROP NOT NULL on a parent rel to > its children. What we've discussed in the past is to store NOT NULL > constraints in pg_constraint, much like CHECK constraints are already, and > use use-count logic identical to the CHECK case to keep track of whether > NOT NULL constraints are inherited or not. My feeling is that we'd keep > the pg_attribute.attnotnull field and continue to drive actual enforcement > off that, but it would just reflect a summary of the pg_constraint state. OK, I see. Hm, by storing this information I would actually think that we want to drop this attnotnull so as we don't need to bother about updating pg_attribute through the whole tree when dropping a NOT NULL constraint on the parent, and we do not actually need to store this information in two different places.. I would also rather do nothing for the DDL interface regarding for example the possibility to change the constraint names for domains and tables to keep things simple. A patch of this caliber would be complicated enough if a catalog switch is done. > IIRC, Alvaro posted a WIP patch for that awhile back. Not sure what the > current state is. Are you talking about that? https://www.postgresql.org/message-id/20110707213401.GA27098%40alvh.no-ip.org This is not a small patch :) Alvaro, others, any opinions? -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Prevent ALTER TABLE DROP NOT NULL on child tables if parent column has it
Michael Paquier writes: > To put it short, it should not be possible to drop a NOT NULL > constraint on a child relation when its parent table is using it, this > should only be possible from the parent. Attached is a patch handling > this problem by adding a new function in pg_inherits.c to fetch the > list of parent relations for a given relation OID, and did some > refactoring to stick with what is done when scanning child relations. This doesn't sound like the right approach; in particular, it won't really help for deciding whether to propagate a DROP NOT NULL on a parent rel to its children. What we've discussed in the past is to store NOT NULL constraints in pg_constraint, much like CHECK constraints are already, and use use-count logic identical to the CHECK case to keep track of whether NOT NULL constraints are inherited or not. My feeling is that we'd keep the pg_attribute.attnotnull field and continue to drive actual enforcement off that, but it would just reflect a summary of the pg_constraint state. IIRC, Alvaro posted a WIP patch for that awhile back. Not sure what the current state is. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers