Skip to content

Commit 0a02d47

Browse files
tglsfdctglsfdc
committed
Enforces NOT NULL constraints to be applied against new PRIMARY KEY
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
1 parent 2f86f14 commit 0a02d47

File tree

6 files changed

+91
-35
lines changed

6 files changed

+91
-35
lines changed

‎src/backend/commands/indexcmds.c

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/commands/indexcmds.c,v 1.95 2002/12/15 16:17:39 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/commands/indexcmds.c,v 1.96 2003/01/02 19:29:22 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -19,11 +19,13 @@
1919
#include "catalog/catalog.h"
2020
#include "catalog/catname.h"
2121
#include "catalog/dependency.h"
22+
#include "catalog/heap.h"
2223
#include "catalog/index.h"
2324
#include "catalog/namespace.h"
2425
#include "catalog/pg_opclass.h"
2526
#include "catalog/pg_proc.h"
2627
#include "commands/defrem.h"
28+
#include "commands/tablecmds.h"
2729
#include "executor/executor.h"
2830
#include "miscadmin.h"
2931
#include "optimizer/clauses.h"
@@ -165,6 +167,50 @@ DefineIndex(RangeVar *heapRelation,
165167
CheckPredicate(cnfPred, rangetable, relationId);
166168
}
167169

170+
/*
171+
* Check that all of the attributes in a primary key are marked
172+
* as not null, otherwise attempt to ALTER TABLE .. SET NOT NULL
173+
*/
174+
if (primary && !IsFuncIndex(attributeList))
175+
{
176+
List *keys;
177+
178+
foreach(keys, attributeList)
179+
{
180+
IndexElem *key = (IndexElem *) lfirst(keys);
181+
HeapTuple atttuple;
182+
183+
/* System attributes are never null, so no problem */
184+
if (SystemAttributeByName(key->name, rel->rd_rel->relhasoids))
185+
continue;
186+
187+
atttuple = SearchSysCacheAttName(relationId, key->name);
188+
if (HeapTupleIsValid(atttuple))
189+
{
190+
if (! ((Form_pg_attribute) GETSTRUCT(atttuple))->attnotnull)
191+
{
192+
/*
193+
* Try to make it NOT NULL.
194+
*
195+
* XXX: Shouldn't the ALTER TABLE .. SET NOT NULL cascade
196+
* to child tables? Currently, since the PRIMARY KEY
197+
* itself doesn't cascade, we don't cascade the notnull
198+
* constraint either; but this is pretty debatable.
199+
*/
200+
AlterTableAlterColumnSetNotNull(relationId, false,
201+
key->name);
202+
}
203+
ReleaseSysCache(atttuple);
204+
}
205+
else
206+
{
207+
/* This shouldn't happen if parser did its job ... */
208+
elog(ERROR, "DefineIndex: column \"%s\" named in key does not exist",
209+
key->name);
210+
}
211+
}
212+
}
213+
168214
/*
169215
* Prepare arguments for index_create, primarily an IndexInfo
170216
* structure
@@ -296,7 +342,7 @@ FuncIndexArgs(IndexInfo *indexInfo,
296342

297343
tuple = SearchSysCacheAttName(relId, arg);
298344
if (!HeapTupleIsValid(tuple))
299-
elog(ERROR, "DefineIndex: attribute \"%s\" not found", arg);
345+
elog(ERROR, "DefineIndex: column \"%s\" named in key does not exist", arg);
300346
att = (Form_pg_attribute) GETSTRUCT(tuple);
301347
indexInfo->ii_KeyAttrNumbers[nargs] = att->attnum;
302348
argTypes[nargs] = att->atttypid;

‎src/backend/parser/analyze.c

Lines changed: 10 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
9-
* $Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.258 2002/12/17 01:18:29 tgl Exp $
9+
* $Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.259 2003/01/02 19:29:22 tgl Exp $
1010
*
1111
*-------------------------------------------------------------------------
1212
*/
@@ -1083,7 +1083,9 @@ transformIndexConstraints(ParseState *pstate, CreateStmtContext *cxt)
10831083

10841084
/*
10851085
* Make sure referenced keys exist. If we are making a PRIMARY
1086-
* KEY index, also make sure they are NOT NULL.
1086+
* KEY index, also make sure they are NOT NULL, if possible.
1087+
* (Although we could leave it to DefineIndex to mark the columns NOT
1088+
* NULL, it's more efficient to get it right the first time.)
10871089
*/
10881090
foreach(keys, constraint->keys)
10891091
{
@@ -1142,25 +1144,12 @@ transformIndexConstraints(ParseState *pstate, CreateStmtContext *cxt)
11421144
if (strcmp(key, inhname) == 0)
11431145
{
11441146
found = true;
1145-
11461147
/*
1147-
* If the column is inherited, we currently
1148-
* have no easy way to force it to be NOT
1149-
* NULL. Only way I can see to fix this would
1150-
* be to convert the inherited-column info to
1151-
* ColumnDef nodes before we reach this point,
1152-
* and then create the table from those nodes
1153-
* rather than referencing the parent tables
1154-
* later. That would likely be cleaner, but
1155-
* too much work to contemplate right now.
1156-
* Instead, raise an error if the inherited
1157-
* column won't be NOT NULL. (Would a WARNING
1158-
* be more reasonable?)
1148+
* We currently have no easy way to force an
1149+
* inherited column to be NOT NULL at creation, if
1150+
* its parent wasn't so already. We leave it to
1151+
* DefineIndex to fix things up in this case.
11591152
*/
1160-
if (constraint->contype == CONSTR_PRIMARY &&
1161-
!inhattr->attnotnull)
1162-
elog(ERROR, "inherited attribute \"%s\" cannot be a PRIMARY KEY because it is not marked NOT NULL",
1163-
inhname);
11641153
break;
11651154
}
11661155
}
@@ -1178,15 +1167,10 @@ transformIndexConstraints(ParseState *pstate, CreateStmtContext *cxt)
11781167
if (HeapTupleIsValid(atttuple))
11791168
{
11801169
found = true;
1181-
11821170
/*
1183-
* We require pre-existing column to be already marked
1184-
* NOT NULL.
1171+
* If it's not already NOT NULL, leave it to DefineIndex
1172+
* to fix later.
11851173
*/
1186-
if (constraint->contype == CONSTR_PRIMARY &&
1187-
!((Form_pg_attribute) GETSTRUCT(atttuple))->attnotnull)
1188-
elog(ERROR, "Existing attribute \"%s\" cannot be a PRIMARY KEY because it is not marked NOT NULL",
1189-
key);
11901174
ReleaseSysCache(atttuple);
11911175
}
11921176
}

‎src/test/regress/expected/alter_table.out

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -539,16 +539,23 @@ drop table atacc1;
539539
create table atacc1 ( test int );
540540
-- add a primary key constraint
541541
alter table atacc1 add constraint atacc_test1 primary key (test);
542-
ERROR: Existing attribute "test" cannot be a PRIMARY KEY because it is not marked NOT NULL
542+
NOTICE: ALTER TABLE / ADD PRIMARY KEY will create implicit index 'atacc_test1' for table 'atacc1'
543543
-- insert first value
544544
insert into atacc1 (test) values (2);
545545
-- should fail
546546
insert into atacc1 (test) values (2);
547+
ERROR: Cannot insert a duplicate key into unique index atacc_test1
547548
-- should succeed
548549
insert into atacc1 (test) values (4);
549550
-- inserting NULL should fail
550551
insert into atacc1 (test) values(NULL);
551-
-- try adding a primary key oid constraint
552+
ERROR: ExecInsert: Fail to add null value in not null attribute test
553+
-- try adding a second primary key (should fail)
554+
alter table atacc1 add constraint atacc_oid1 primary key(oid);
555+
ERROR: ALTER TABLE / PRIMARY KEY multiple primary keys for table 'atacc1' are not allowed
556+
-- drop first primary key constraint
557+
alter table atacc1 drop constraint atacc_test1 restrict;
558+
-- try adding a primary key on oid (should succeed)
552559
alter table atacc1 add constraint atacc_oid1 primary key(oid);
553560
NOTICE: ALTER TABLE / ADD PRIMARY KEY will create implicit index 'atacc_oid1' for table 'atacc1'
554561
drop table atacc1;
@@ -559,7 +566,8 @@ insert into atacc1 (test) values (2);
559566
insert into atacc1 (test) values (2);
560567
-- add a primary key (fails)
561568
alter table atacc1 add constraint atacc_test1 primary key (test);
562-
ERROR: Existing attribute "test" cannot be a PRIMARY KEY because it is not marked NOT NULL
569+
NOTICE: ALTER TABLE / ADD PRIMARY KEY will create implicit index 'atacc_test1' for table 'atacc1'
570+
ERROR: Cannot create unique index. Table contains non-unique values
563571
insert into atacc1 (test) values (3);
564572
drop table atacc1;
565573
-- let's do another one where the primary key constraint fails when added
@@ -568,7 +576,8 @@ create table atacc1 ( test int );
568576
insert into atacc1 (test) values (NULL);
569577
-- add a primary key (fails)
570578
alter table atacc1 add constraint atacc_test1 primary key (test);
571-
ERROR: Existing attribute "test" cannot be a PRIMARY KEY because it is not marked NOT NULL
579+
NOTICE: ALTER TABLE / ADD PRIMARY KEY will create implicit index 'atacc_test1' for table 'atacc1'
580+
ERROR: ALTER TABLE: Attribute "test" contains NULL values
572581
insert into atacc1 (test) values (3);
573582
drop table atacc1;
574583
-- let's do one where the primary key constraint fails
@@ -582,17 +591,21 @@ drop table atacc1;
582591
create table atacc1 ( test int, test2 int);
583592
-- add a primary key constraint
584593
alter table atacc1 add constraint atacc_test1 primary key (test, test2);
585-
ERROR: Existing attribute "test" cannot be a PRIMARY KEY because it is not marked NOT NULL
594+
NOTICE: ALTER TABLE / ADD PRIMARY KEY will create implicit index 'atacc_test1' for table 'atacc1'
586595
-- try adding a second primary key - should fail
587596
alter table atacc1 add constraint atacc_test2 primary key (test);
588-
ERROR: Existing attribute "test" cannot be a PRIMARY KEY because it is not marked NOT NULL
597+
ERROR: ALTER TABLE / PRIMARY KEY multiple primary keys for table 'atacc1' are not allowed
589598
-- insert initial value
590599
insert into atacc1 (test,test2) values (4,4);
591600
-- should fail
592601
insert into atacc1 (test,test2) values (4,4);
602+
ERROR: Cannot insert a duplicate key into unique index atacc_test1
593603
insert into atacc1 (test,test2) values (NULL,3);
604+
ERROR: ExecInsert: Fail to add null value in not null attribute test
594605
insert into atacc1 (test,test2) values (3, NULL);
606+
ERROR: ExecInsert: Fail to add null value in not null attribute test2
595607
insert into atacc1 (test,test2) values (NULL,NULL);
608+
ERROR: ExecInsert: Fail to add null value in not null attribute test
596609
-- should all succeed
597610
insert into atacc1 (test,test2) values (4,5);
598611
insert into atacc1 (test,test2) values (5,4);

‎src/test/regress/expected/inherit.out

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -534,3 +534,8 @@ SELECT relname, d.* FROM ONLY d, pg_class where d.tableoid = pg_class.oid;
534534
---------+----+----+----+----
535535
(0 rows)
536536

537+
-- Confirm PRIMARY KEY adds NOT NULL constraint to child table
538+
CREATE TEMP TABLE z (b TEXT, PRIMARY KEY(aa, b)) inherits (a);
539+
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index 'z_pkey' for table 'z'
540+
INSERT INTO z VALUES (NULL, 'text'); -- should fail
541+
ERROR: ExecInsert: Fail to add null value in not null attribute aa

‎src/test/regress/sql/alter_table.sql

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,11 @@ insert into atacc1 (test) values (2);
415415
insert into atacc1 (test) values (4);
416416
-- inserting NULL should fail
417417
insert into atacc1 (test) values(NULL);
418-
-- try adding a primary key oid constraint
418+
-- try adding a second primary key (should fail)
419+
alter table atacc1 add constraint atacc_oid1 primary key(oid);
420+
-- drop first primary key constraint
421+
alter table atacc1 drop constraint atacc_test1 restrict;
422+
-- try adding a primary key on oid (should succeed)
419423
alter table atacc1 add constraint atacc_oid1 primary key(oid);
420424
drop table atacc1;
421425

‎src/test/regress/sql/inherit.sql

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,3 +92,7 @@ SELECT relname, a.* FROM ONLY a, pg_class where a.tableoid = pg_class.oid;
9292
SELECT relname, b.* FROM ONLY b, pg_class where b.tableoid = pg_class.oid;
9393
SELECT relname, c.* FROM ONLY c, pg_class where c.tableoid = pg_class.oid;
9494
SELECT relname, d.* FROM ONLY d, pg_class where d.tableoid = pg_class.oid;
95+
96+
-- Confirm PRIMARY KEY adds NOT NULL constraint to child table
97+
CREATE TEMP TABLE z (b TEXT, PRIMARY KEY(aa, b)) inherits (a);
98+
INSERT INTO z VALUES (NULL, 'text'); -- should fail

0 commit comments

Comments
 (0)