Enforces NOT NULL constraints to be applied against new PRIMARY KEY
authorTom Lane
Thu, 2 Jan 2003 19:29:22 +0000 (19:29 +0000)
committerTom Lane
Thu, 2 Jan 2003 19:29:22 +0000 (19:29 +0000)
columns in DefineIndex.  So, ALTER TABLE ... PRIMARY KEY will now
automatically add the NOT NULL constraint.  It appeared the alter_table
regression test wanted this to occur, as after the change the regression
test better matched in inline 'fails'/'succeeds' comments.

Rod Taylor

src/backend/commands/indexcmds.c
src/backend/parser/analyze.c
src/test/regress/expected/alter_table.out
src/test/regress/expected/inherit.out
src/test/regress/sql/alter_table.sql
src/test/regress/sql/inherit.sql

index d46b7a389a387d48752f5d966a7d59a7b9b6b926..df7e358b8976ec1dbd0960f8879d3862253adeb0 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/commands/indexcmds.c,v 1.95 2002/12/15 16:17:39 tgl Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/commands/indexcmds.c,v 1.96 2003/01/02 19:29:22 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
 #include "catalog/catalog.h"
 #include "catalog/catname.h"
 #include "catalog/dependency.h"
+#include "catalog/heap.h"
 #include "catalog/index.h"
 #include "catalog/namespace.h"
 #include "catalog/pg_opclass.h"
 #include "catalog/pg_proc.h"
 #include "commands/defrem.h"
+#include "commands/tablecmds.h"
 #include "executor/executor.h"
 #include "miscadmin.h"
 #include "optimizer/clauses.h"
@@ -165,6 +167,50 @@ DefineIndex(RangeVar *heapRelation,
        CheckPredicate(cnfPred, rangetable, relationId);
    }
 
+   /*
+    * Check that all of the attributes in a primary key are marked
+    * as not null, otherwise attempt to ALTER TABLE .. SET NOT NULL
+    */
+   if (primary && !IsFuncIndex(attributeList))
+   {
+       List   *keys;
+
+       foreach(keys, attributeList)
+       {
+           IndexElem   *key = (IndexElem *) lfirst(keys);
+           HeapTuple   atttuple;
+
+           /* System attributes are never null, so no problem */
+           if (SystemAttributeByName(key->name, rel->rd_rel->relhasoids))
+               continue;
+
+           atttuple = SearchSysCacheAttName(relationId, key->name);
+           if (HeapTupleIsValid(atttuple))
+           {
+               if (! ((Form_pg_attribute) GETSTRUCT(atttuple))->attnotnull)
+               {
+                   /*
+                    * Try to make it NOT NULL.
+                    *
+                    * XXX: Shouldn't the ALTER TABLE .. SET NOT NULL cascade
+                    * to child tables?  Currently, since the PRIMARY KEY
+                    * itself doesn't cascade, we don't cascade the notnull
+                    * constraint either; but this is pretty debatable.
+                    */
+                   AlterTableAlterColumnSetNotNull(relationId, false,
+                                                   key->name);
+               }
+               ReleaseSysCache(atttuple);
+           }
+           else
+           {
+               /* This shouldn't happen if parser did its job ... */
+               elog(ERROR, "DefineIndex: column \"%s\" named in key does not exist",
+                    key->name);
+           }
+       }
+   }
+
    /*
     * Prepare arguments for index_create, primarily an IndexInfo
     * structure
@@ -296,7 +342,7 @@ FuncIndexArgs(IndexInfo *indexInfo,
 
        tuple = SearchSysCacheAttName(relId, arg);
        if (!HeapTupleIsValid(tuple))
-           elog(ERROR, "DefineIndex: attribute \"%s\" not found", arg);
+           elog(ERROR, "DefineIndex: column \"%s\" named in key does not exist", arg);
        att = (Form_pg_attribute) GETSTRUCT(tuple);
        indexInfo->ii_KeyAttrNumbers[nargs] = att->attnum;
        argTypes[nargs] = att->atttypid;
index e771d666597727a00acdbfc2e4e12e964ed97c45..2609de18f54656f16a85c3175570fe01fc5c27bc 100644 (file)
@@ -6,7 +6,7 @@
  * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.258 2002/12/17 01:18:29 tgl Exp $
+ * $Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.259 2003/01/02 19:29:22 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1083,7 +1083,9 @@ transformIndexConstraints(ParseState *pstate, CreateStmtContext *cxt)
 
        /*
         * Make sure referenced keys exist.  If we are making a PRIMARY
-        * KEY index, also make sure they are NOT NULL.
+        * KEY index, also make sure they are NOT NULL, if possible.
+        * (Although we could leave it to DefineIndex to mark the columns NOT
+        * NULL, it's more efficient to get it right the first time.)
         */
        foreach(keys, constraint->keys)
        {
@@ -1142,25 +1144,12 @@ transformIndexConstraints(ParseState *pstate, CreateStmtContext *cxt)
                        if (strcmp(key, inhname) == 0)
                        {
                            found = true;
-
                            /*
-                            * If the column is inherited, we currently
-                            * have no easy way to force it to be NOT
-                            * NULL. Only way I can see to fix this would
-                            * be to convert the inherited-column info to
-                            * ColumnDef nodes before we reach this point,
-                            * and then create the table from those nodes
-                            * rather than referencing the parent tables
-                            * later.  That would likely be cleaner, but
-                            * too much work to contemplate right now.
-                            * Instead, raise an error if the inherited
-                            * column won't be NOT NULL. (Would a WARNING
-                            * be more reasonable?)
+                            * We currently have no easy way to force an
+                            * inherited column to be NOT NULL at creation, if
+                            * its parent wasn't so already.  We leave it to
+                            * DefineIndex to fix things up in this case.
                             */
-                           if (constraint->contype == CONSTR_PRIMARY &&
-                               !inhattr->attnotnull)
-                               elog(ERROR, "inherited attribute \"%s\" cannot be a PRIMARY KEY because it is not marked NOT NULL",
-                                    inhname);
                            break;
                        }
                    }
@@ -1178,15 +1167,10 @@ transformIndexConstraints(ParseState *pstate, CreateStmtContext *cxt)
                if (HeapTupleIsValid(atttuple))
                {
                    found = true;
-
                    /*
-                    * We require pre-existing column to be already marked
-                    * NOT NULL.
+                    * If it's not already NOT NULL, leave it to DefineIndex
+                    * to fix later.
                     */
-                   if (constraint->contype == CONSTR_PRIMARY &&
-                       !((Form_pg_attribute) GETSTRUCT(atttuple))->attnotnull)
-                       elog(ERROR, "Existing attribute \"%s\" cannot be a PRIMARY KEY because it is not marked NOT NULL",
-                            key);
                    ReleaseSysCache(atttuple);
                }
            }
index 4d61811045846ff63aecb296058e67f61d965ee3..e0e9b8dc5156dea34d7826c5eaf1de3e783d230b 100644 (file)
@@ -539,16 +539,23 @@ drop table atacc1;
 create table atacc1 ( test int );
 -- add a primary key constraint
 alter table atacc1 add constraint atacc_test1 primary key (test);
-ERROR:  Existing attribute "test" cannot be a PRIMARY KEY because it is not marked NOT NULL
+NOTICE:  ALTER TABLE / ADD PRIMARY KEY will create implicit index 'atacc_test1' for table 'atacc1'
 -- insert first value
 insert into atacc1 (test) values (2);
 -- should fail
 insert into atacc1 (test) values (2);
+ERROR:  Cannot insert a duplicate key into unique index atacc_test1
 -- should succeed
 insert into atacc1 (test) values (4);
 -- inserting NULL should fail
 insert into atacc1 (test) values(NULL);
--- try adding a primary key oid constraint
+ERROR:  ExecInsert: Fail to add null value in not null attribute test
+-- try adding a second primary key (should fail)
+alter table atacc1 add constraint atacc_oid1 primary key(oid);
+ERROR:  ALTER TABLE / PRIMARY KEY multiple primary keys for table 'atacc1' are not allowed
+-- drop first primary key constraint
+alter table atacc1 drop constraint atacc_test1 restrict;
+-- try adding a primary key on oid (should succeed)
 alter table atacc1 add constraint atacc_oid1 primary key(oid);
 NOTICE:  ALTER TABLE / ADD PRIMARY KEY will create implicit index 'atacc_oid1' for table 'atacc1'
 drop table atacc1;
@@ -559,7 +566,8 @@ insert into atacc1 (test) values (2);
 insert into atacc1 (test) values (2);
 -- add a primary key (fails)
 alter table atacc1 add constraint atacc_test1 primary key (test);
-ERROR:  Existing attribute "test" cannot be a PRIMARY KEY because it is not marked NOT NULL
+NOTICE:  ALTER TABLE / ADD PRIMARY KEY will create implicit index 'atacc_test1' for table 'atacc1'
+ERROR:  Cannot create unique index. Table contains non-unique values
 insert into atacc1 (test) values (3);
 drop table atacc1;
 -- let's do another one where the primary key constraint fails when added
@@ -568,7 +576,8 @@ create table atacc1 ( test int );
 insert into atacc1 (test) values (NULL);
 -- add a primary key (fails)
 alter table atacc1 add constraint atacc_test1 primary key (test);
-ERROR:  Existing attribute "test" cannot be a PRIMARY KEY because it is not marked NOT NULL
+NOTICE:  ALTER TABLE / ADD PRIMARY KEY will create implicit index 'atacc_test1' for table 'atacc1'
+ERROR:  ALTER TABLE: Attribute "test" contains NULL values
 insert into atacc1 (test) values (3);
 drop table atacc1;
 -- let's do one where the primary key constraint fails
@@ -582,17 +591,21 @@ drop table atacc1;
 create table atacc1 ( test int, test2 int);
 -- add a primary key constraint
 alter table atacc1 add constraint atacc_test1 primary key (test, test2);
-ERROR:  Existing attribute "test" cannot be a PRIMARY KEY because it is not marked NOT NULL
+NOTICE:  ALTER TABLE / ADD PRIMARY KEY will create implicit index 'atacc_test1' for table 'atacc1'
 -- try adding a second primary key - should fail
 alter table atacc1 add constraint atacc_test2 primary key (test);
-ERROR:  Existing attribute "test" cannot be a PRIMARY KEY because it is not marked NOT NULL
+ERROR:  ALTER TABLE / PRIMARY KEY multiple primary keys for table 'atacc1' are not allowed
 -- insert initial value
 insert into atacc1 (test,test2) values (4,4);
 -- should fail
 insert into atacc1 (test,test2) values (4,4);
+ERROR:  Cannot insert a duplicate key into unique index atacc_test1
 insert into atacc1 (test,test2) values (NULL,3);
+ERROR:  ExecInsert: Fail to add null value in not null attribute test
 insert into atacc1 (test,test2) values (3, NULL);
+ERROR:  ExecInsert: Fail to add null value in not null attribute test2
 insert into atacc1 (test,test2) values (NULL,NULL);
+ERROR:  ExecInsert: Fail to add null value in not null attribute test
 -- should all succeed
 insert into atacc1 (test,test2) values (4,5);
 insert into atacc1 (test,test2) values (5,4);
index b56798124f3bf90bba895a227dd211edd9aba51b..6ca59e799bf35cc3c88295f1b92880573582b0d2 100644 (file)
@@ -534,3 +534,8 @@ SELECT relname, d.* FROM ONLY d, pg_class where d.tableoid = pg_class.oid;
 ---------+----+----+----+----
 (0 rows)
 
+-- Confirm PRIMARY KEY adds NOT NULL constraint to child table
+CREATE TEMP TABLE z (b TEXT, PRIMARY KEY(aa, b)) inherits (a);
+NOTICE:  CREATE TABLE / PRIMARY KEY will create implicit index 'z_pkey' for table 'z'
+INSERT INTO z VALUES (NULL, 'text'); -- should fail
+ERROR:  ExecInsert: Fail to add null value in not null attribute aa
index 7fcfb09df2b4f2246ee7f9f954420c4d873fe4f8..4d20705f53a5a4b8bfdf46a55c3ba19d57d138a2 100644 (file)
@@ -415,7 +415,11 @@ insert into atacc1 (test) values (2);
 insert into atacc1 (test) values (4);
 -- inserting NULL should fail
 insert into atacc1 (test) values(NULL);
--- try adding a primary key oid constraint
+-- try adding a second primary key (should fail)
+alter table atacc1 add constraint atacc_oid1 primary key(oid);
+-- drop first primary key constraint
+alter table atacc1 drop constraint atacc_test1 restrict;
+-- try adding a primary key on oid (should succeed)
 alter table atacc1 add constraint atacc_oid1 primary key(oid);
 drop table atacc1;
 
index c60a89266ae2c9dd74930e12f6aeb5a40c65c27f..d8d73f8860f95264c72e3b6222ed6f790aa912f8 100644 (file)
@@ -92,3 +92,7 @@ SELECT relname, a.* FROM ONLY a, pg_class where a.tableoid = pg_class.oid;
 SELECT relname, b.* FROM ONLY b, pg_class where b.tableoid = pg_class.oid;
 SELECT relname, c.* FROM ONLY c, pg_class where c.tableoid = pg_class.oid;
 SELECT relname, d.* FROM ONLY d, pg_class where d.tableoid = pg_class.oid;
+
+-- Confirm PRIMARY KEY adds NOT NULL constraint to child table
+CREATE TEMP TABLE z (b TEXT, PRIMARY KEY(aa, b)) inherits (a);
+INSERT INTO z VALUES (NULL, 'text'); -- should fail