diff --git a/src/backend/bootstrap/bootparse.y b/src/backend/bootstrap/bootparse.y index fef6e7c..b0d4155 100644 --- a/src/backend/bootstrap/bootparse.y +++ b/src/backend/bootstrap/bootparse.y @@ -323,6 +323,7 @@ Boot_DeclareIndexStmt: false, false, true, /* skip_build */ + false, /* skip_check_not_null */ false); do_end(); } @@ -372,6 +373,7 @@ Boot_DeclareUniqueIndexStmt: false, false, true, /* skip_build */ + false, /* skip_check_not_null */ false); do_end(); } diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index c339a2b..93a4bd4 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -213,6 +213,7 @@ void index_check_primary_key(Relation heapRel, IndexInfo *indexInfo, bool is_alter_table, + bool skip_check_not_null, IndexStmt *stmt) { List *cmds; @@ -283,7 +284,8 @@ index_check_primary_key(Relation heapRel, if (cmds) { EventTriggerAlterTableStart((Node *) stmt); - AlterTableInternal(RelationGetRelid(heapRel), cmds, true); + AlterTableInternal(RelationGetRelid(heapRel), cmds, true, + skip_check_not_null); EventTriggerAlterTableEnd(); } } diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index c3a53d8..842cc07 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -317,6 +317,8 @@ CheckIndexCompatible(Oid oldId, * This should be true unless caller is holding the table open, in which * case the caller had better have checked it earlier. * 'skip_build': make the catalog entries but don't create the index files + * 'skip_check_not_null': do not check whether columns are NOT NULL, + * if phase 3 will do it. * 'quiet': suppress the NOTICE chatter ordinarily provided for constraints. * * Returns the object address of the created index. @@ -331,6 +333,7 @@ DefineIndex(Oid relationId, bool check_rights, bool check_not_in_use, bool skip_build, + bool skip_check_not_null, bool quiet) { char *indexRelationName; @@ -665,7 +668,8 @@ DefineIndex(Oid relationId, * Extra checks when creating a PRIMARY KEY index. */ if (stmt->primary) - index_check_primary_key(rel, indexInfo, is_alter_table, stmt); + index_check_primary_key(rel, indexInfo, is_alter_table, + skip_check_not_null, stmt); /* * If this table is partitioned and we're creating a unique index or a @@ -1037,7 +1041,7 @@ DefineIndex(Oid relationId, indexRelationId, /* this is our child */ createdConstraintId, is_alter_table, check_rights, check_not_in_use, - skip_build, quiet); + skip_build, skip_check_not_null, quiet); } pfree(attmap); diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 515c290..2ca05f6 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -339,15 +339,17 @@ static void validateForeignKeyConstraint(char *conname, Relation rel, Relation pkrel, Oid pkindOid, Oid constraintOid); static void ATController(AlterTableStmt *parsetree, - Relation rel, List *cmds, bool recurse, LOCKMODE lockmode); + Relation rel, List *cmds, bool recurse, + bool skip_check_not_null, LOCKMODE lockmode); static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, bool recurse, bool recursing, LOCKMODE lockmode); static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode); static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel, AlterTableCmd *cmd, LOCKMODE lockmode); static void ATRewriteTables(AlterTableStmt *parsetree, - List **wqueue, LOCKMODE lockmode); -static void ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode); + List **wqueue, bool skip_check_not_null, LOCKMODE lockmode); +static void ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, + bool skip_check_not_null, LOCKMODE lockmode); static AlteredTableInfo *ATGetQueueEntry(List **wqueue, Relation rel); static void ATSimplePermissions(Relation rel, int allowed_targets); static void ATWrongRelkindError(Relation rel, int allowed_targets); @@ -1040,7 +1042,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, InvalidOid, RelationGetRelid(idxRel), constraintOid, - false, false, false, false, false); + false, false, false, false, false, false); index_close(idxRel, AccessShareLock); } @@ -3416,7 +3418,7 @@ AlterTable(Oid relid, LOCKMODE lockmode, AlterTableStmt *stmt) CheckTableNotInUse(rel, "ALTER TABLE"); - ATController(stmt, rel, stmt->cmds, stmt->relation->inh, lockmode); + ATController(stmt, rel, stmt->cmds, stmt->relation->inh, false, lockmode); } /* @@ -3431,7 +3433,7 @@ AlterTable(Oid relid, LOCKMODE lockmode, AlterTableStmt *stmt) * don't have to reject pending AFTER triggers, either. */ void -AlterTableInternal(Oid relid, List *cmds, bool recurse) +AlterTableInternal(Oid relid, List *cmds, bool recurse, bool skip_check_not_null) { Relation rel; LOCKMODE lockmode = AlterTableGetLockLevel(cmds); @@ -3440,7 +3442,7 @@ AlterTableInternal(Oid relid, List *cmds, bool recurse) EventTriggerAlterTableRelid(relid); - ATController(NULL, rel, cmds, recurse, lockmode); + ATController(NULL, rel, cmds, recurse, skip_check_not_null, lockmode); } /* @@ -3728,7 +3730,8 @@ AlterTableGetLockLevel(List *cmds) */ static void ATController(AlterTableStmt *parsetree, - Relation rel, List *cmds, bool recurse, LOCKMODE lockmode) + Relation rel, List *cmds, bool recurse, + bool skip_check_not_null, LOCKMODE lockmode) { List *wqueue = NIL; ListCell *lcmd; @@ -3748,7 +3751,7 @@ ATController(AlterTableStmt *parsetree, ATRewriteCatalogs(&wqueue, lockmode); /* Phase 3: scan/rewrite tables as needed */ - ATRewriteTables(parsetree, &wqueue, lockmode); + ATRewriteTables(parsetree, &wqueue, skip_check_not_null, lockmode); } /* @@ -4389,7 +4392,8 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel, * ATRewriteTables: ALTER TABLE phase 3 */ static void -ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode) +ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, + bool skip_check_not_null, LOCKMODE lockmode) { ListCell *ltab; @@ -4534,7 +4538,7 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode) * modifications, and test the current data within the table * against new constraints generated by ALTER TABLE commands. */ - ATRewriteTable(tab, OIDNewHeap, lockmode); + ATRewriteTable(tab, OIDNewHeap, skip_check_not_null, lockmode); /* * Swap the physical files of the old and new heaps, then rebuild @@ -4560,7 +4564,7 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode) */ if (tab->constraints != NIL || tab->verify_new_notnull || tab->partition_constraint != NULL) - ATRewriteTable(tab, InvalidOid, lockmode); + ATRewriteTable(tab, InvalidOid, skip_check_not_null, lockmode); /* * If we had SET TABLESPACE but no reason to reconstruct tuples, @@ -4625,7 +4629,8 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode) * OIDNewHeap is InvalidOid if we don't need to rewrite */ static void -ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode) +ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, + bool skip_check_not_null, LOCKMODE lockmode) { Relation oldrel; Relation newrel; @@ -4875,19 +4880,22 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode) /* Now check any constraints on the possibly-changed tuple */ econtext->ecxt_scantuple = insertslot; - foreach(l, notnull_attrs) + if (!skip_check_not_null) { - int attn = lfirst_int(l); - - if (slot_attisnull(insertslot, attn + 1)) + foreach(l, notnull_attrs) { - Form_pg_attribute attr = TupleDescAttr(newTupDesc, attn); + int attn = lfirst_int(l); - ereport(ERROR, - (errcode(ERRCODE_NOT_NULL_VIOLATION), - errmsg("column \"%s\" contains null values", + if (slot_attisnull(insertslot, attn + 1)) + { + Form_pg_attribute attr = TupleDescAttr(newTupDesc, attn); + + ereport(ERROR, + (errcode(ERRCODE_NOT_NULL_VIOLATION), + errmsg("column \"%s\" contains null values", NameStr(attr->attname)), - errtablecol(oldrel, attn + 1))); + errtablecol(oldrel, attn + 1))); + } } } @@ -7006,6 +7014,7 @@ ATExecAddIndex(AlteredTableInfo *tab, Relation rel, { bool check_rights; bool skip_build; + bool skip_check_not_null; bool quiet; ObjectAddress address; @@ -7019,6 +7028,8 @@ ATExecAddIndex(AlteredTableInfo *tab, Relation rel, check_rights = !is_rebuild; /* skip index build if phase 3 will do it or we're reusing an old one */ skip_build = tab->rewrite > 0 || OidIsValid(stmt->oldNode); + /* do not check whether columns are NOT NULL,if phase 3 will do it */ + skip_check_not_null = tab->rewrite > 0; /* suppress notices when rebuilding existing index */ quiet = is_rebuild; @@ -7031,6 +7042,7 @@ ATExecAddIndex(AlteredTableInfo *tab, Relation rel, check_rights, false, /* check_not_in_use - we did it already */ skip_build, + skip_check_not_null, quiet); /* @@ -7067,6 +7079,7 @@ ATExecAddIndexConstraint(AlteredTableInfo *tab, Relation rel, char constraintType; ObjectAddress address; bits16 flags; + bool skip_check_not_null; Assert(IsA(stmt, IndexStmt)); Assert(OidIsValid(index_oid)); @@ -7109,9 +7122,12 @@ ATExecAddIndexConstraint(AlteredTableInfo *tab, Relation rel, RenameRelationInternal(index_oid, constraintName, false, true); } + /* do not check whether columns are NOT NULL,if phase 3 will do it */ + skip_check_not_null = tab->rewrite > 0; + /* Extra checks needed if making primary key */ if (stmt->primary) - index_check_primary_key(rel, indexInfo, true, stmt); + index_check_primary_key(rel, indexInfo, true, skip_check_not_null, stmt); /* Note we currently don't support EXCLUSION constraints here */ if (stmt->primary) @@ -11875,7 +11891,7 @@ AlterTableMoveAll(AlterTableMoveAllStmt *stmt) EventTriggerAlterTableStart((Node *) stmt); /* OID is set by AlterTableInternal */ - AlterTableInternal(lfirst_oid(l), cmds, false); + AlterTableInternal(lfirst_oid(l), cmds, false, false); EventTriggerAlterTableEnd(); } @@ -15122,7 +15138,7 @@ AttachPartitionEnsureIndexes(Relation rel, Relation attachrel) DefineIndex(RelationGetRelid(attachrel), stmt, InvalidOid, RelationGetRelid(idxRel), constraintOid, - true, false, false, false, false); + true, false, false, false, false, false); } index_close(idxRel, AccessShareLock); diff --git a/src/backend/commands/view.c b/src/backend/commands/view.c index 87ed453..4bf740d 100644 --- a/src/backend/commands/view.c +++ b/src/backend/commands/view.c @@ -184,7 +184,7 @@ DefineVirtualRelation(RangeVar *relation, List *tlist, bool replace, } /* EventTriggerAlterTableStart called by ProcessUtilitySlow */ - AlterTableInternal(viewOid, atcmds, true); + AlterTableInternal(viewOid, atcmds, true, false); /* Make the new view columns visible */ CommandCounterIncrement(); @@ -216,7 +216,7 @@ DefineVirtualRelation(RangeVar *relation, List *tlist, bool replace, atcmds = list_make1(atcmd); /* EventTriggerAlterTableStart called by ProcessUtilitySlow */ - AlterTableInternal(viewOid, atcmds, true); + AlterTableInternal(viewOid, atcmds, true, false); ObjectAddressSet(address, RelationRelationId, viewOid); diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index 5053ef0..399e473 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -1362,6 +1362,7 @@ ProcessUtilitySlow(ParseState *pstate, true, /* check_rights */ true, /* check_not_in_use */ false, /* skip_build */ + false, /* skip_check_not_null */ false); /* quiet */ /* diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h index 29f7ed6..6b2dd7b 100644 --- a/src/include/catalog/index.h +++ b/src/include/catalog/index.h @@ -41,6 +41,7 @@ typedef enum extern void index_check_primary_key(Relation heapRel, IndexInfo *indexInfo, bool is_alter_table, + bool skip_check_not_null, IndexStmt *stmt); #define INDEX_CREATE_IS_PRIMARY (1 << 0) diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h index 3bc2e8e..89047e3 100644 --- a/src/include/commands/defrem.h +++ b/src/include/commands/defrem.h @@ -33,6 +33,7 @@ extern ObjectAddress DefineIndex(Oid relationId, bool check_rights, bool check_not_in_use, bool skip_build, + bool skip_check_not_null, bool quiet); extern void ReindexIndex(RangeVar *indexRelation, int options); extern Oid ReindexTable(RangeVar *relation, int options); diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h index ec3bb90..41a57a1 100644 --- a/src/include/commands/tablecmds.h +++ b/src/include/commands/tablecmds.h @@ -35,7 +35,7 @@ extern LOCKMODE AlterTableGetLockLevel(List *cmds); extern void ATExecChangeOwner(Oid relationOid, Oid newOwnerId, bool recursing, LOCKMODE lockmode); -extern void AlterTableInternal(Oid relid, List *cmds, bool recurse); +extern void AlterTableInternal(Oid relid, List *cmds, bool recurse, bool skip_check_not_null); extern Oid AlterTableMoveAll(AlterTableMoveAllStmt *stmt); diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 2a26aa3..49492ef 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -1470,6 +1470,17 @@ insert into atacc1(id, value) values (null, 0); ERROR: null value in column "id" violates not-null constraint DETAIL: Failing row contains (null, 0). drop table atacc1; +-- test alter table with ADD COLUMN NOT NULL and ADD PRIMARY KEY +create table atacc1 (a int); +insert into atacc1 values (1); +alter table atacc1 add column b float8 not null default random(), add primary key(a); +drop table atacc1; +-- test alter table with ADD COLUMN NOT NULL add ADD CONSTRAINT USING INDEX +create table atacc1 (a int); +insert into atacc1 values (1); +create unique index uniq_idx on atacc1(a); +alter table atacc1 add column b float8 not null default random(), add primary key using index uniq_idx; +drop table atacc1; -- test inheritance create table parent (a int, b int, c int); insert into parent values (1, 2, 3); diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index 5bda7fe..4939bf4 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -1042,6 +1042,19 @@ insert into atacc1(value) values (100); insert into atacc1(id, value) values (null, 0); drop table atacc1; +-- test alter table with ADD COLUMN NOT NULL and ADD PRIMARY KEY +create table atacc1 (a int); +insert into atacc1 values (1); +alter table atacc1 add column b float8 not null default random(), add primary key(a); +drop table atacc1; + +-- test alter table with ADD COLUMN NOT NULL add ADD CONSTRAINT USING INDEX +create table atacc1 (a int); +insert into atacc1 values (1); +create unique index uniq_idx on atacc1(a); +alter table atacc1 add column b float8 not null default random(), add primary key using index uniq_idx; +drop table atacc1; + -- test inheritance create table parent (a int, b int, c int); insert into parent values (1, 2, 3);