diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index d46b7a389a..df7e358b89 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -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 $ * *------------------------------------------------------------------------- */ @@ -19,11 +19,13 @@ #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; diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index e771d66659..2609de18f5 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -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); } } diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 4d61811045..e0e9b8dc51 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -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); diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out index b56798124f..6ca59e799b 100644 --- a/src/test/regress/expected/inherit.out +++ b/src/test/regress/expected/inherit.out @@ -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 diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index 7fcfb09df2..4d20705f53 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -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; diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql index c60a89266a..d8d73f8860 100644 --- a/src/test/regress/sql/inherit.sql +++ b/src/test/regress/sql/inherit.sql @@ -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