On 2016/08/19 5:35, Robert Haas wrote:
> On Thu, Aug 18, 2016 at 5:15 AM, Amit Langote
> <langote_amit...@lab.ntt.co.jp> wrote:
>> On 2016/07/25 17:18, Amit Langote wrote:
>>> The comment seems to have been copied from ATExecAddColumn, which says:
>>>
>>>  /*
>>>   * If we are told not to recurse, there had better not be any
>>> - * child tables; else the addition would put them out of step.
>>>
>>> For ATExecValidateConstraint, it should say something like:
>>>
>>> + * child tables; else validating the constraint would put them
>>> + * out of step.
>>>
>>> Attached patch fixes it.
>>
>> I noticed that the ALTER TABLE documentation doesn't mention that VALIDATE
>> CONSTRAINT will fail if ONLY is specified and there are descendant tables.
>>  It only mentions that a constraint cannot be renamed unless also renamed
>> in the descendant tables.
>>
>> Attached patch fixes that.
> 
> I did some wordsmithing on the two patches you posted to this thread.
> I suggest the attached version.  What do you think?

Reads much less ambiguous, so +1.

Except in the doc patch:

s/change the type of a column constraint/change the type of a column/g

I fixed that part in the attached.

Thanks,
Amit
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 6f51cbc..e48ccf2 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -1028,11 +1028,15 @@ ALTER TABLE ALL IN TABLESPACE <replaceable class="PARAMETER">name</replaceable>
 
    <para>
     If a table has any descendant tables, it is not permitted to add,
-    rename, or change the type of a column, or rename an inherited constraint
-    in the parent table without doing
-    the same to the descendants.  That is, <command>ALTER TABLE ONLY</command>
-    will be rejected.  This ensures that the descendants always have
-    columns matching the parent.
+    rename, or change the type of a column in the parent table without doing
+    same to the descendants.  This ensures that the descendants always have
+    columns matching the parent.  Similarly, a constraint cannot be renamed
+    in the parent without also renaming it in all descendents, so that
+    constraints also match between the parent and its descendents.
+    Also, because selecting from the parent also selects from its descendents,
+    a constraint on the parent cannot be marked valid unless it is also marked
+    valid for those descendents.  In all of these cases, <command>ALTER TABLE
+    ONLY</command> will be rejected.  
    </para>
 
    <para>
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 86e9814..d312762 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -6908,7 +6908,8 @@ ATExecValidateConstraint(Relation rel, char *constrName, bool recurse,
 
 				/*
 				 * If we are told not to recurse, there had better not be any
-				 * child tables; else the addition would put them out of step.
+				 * child tables, because we can't mark the constraint on the
+				 * parent valid unless it is valid for all child tables.
 				 */
 				if (!recurse)
 					ereport(ERROR,
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to