Fix ALTER TABLE to check pre-existing NOT NULL constraints when rewriting

a table.  Otherwise a USING clause that yields NULL can leave the table
violating its constraint (possibly there are other cases too).  Per report
from Alexander Pravking.
This commit is contained in:
Tom Lane 2006-07-10 22:10:57 +00:00
parent acf40f39be
commit 6e37be02e7

View File

@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.142.4.5 2006/01/30 16:19:12 tgl Exp $
* $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.142.4.6 2006/07/10 22:10:57 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@ -122,6 +122,7 @@ typedef struct AlteredTableInfo
/* Information saved by Phases 1/2 for Phase 3: */
List *constraints; /* List of NewConstraint */
List *newvals; /* List of NewColumnValue */
bool new_notnull; /* T if we added new NOT NULL constraints */
Oid newTableSpace; /* new tablespace; 0 means no change */
/* Objects to rebuild after completing ALTER TYPE operations */
List *changedConstraintOids; /* OIDs of constraints to rebuild */
@ -131,11 +132,11 @@ typedef struct AlteredTableInfo
} AlteredTableInfo;
/* Struct describing one new constraint to check in Phase 3 scan */
/* Note: new NOT NULL constraints are handled elsewhere */
typedef struct NewConstraint
{
char *name; /* Constraint name, or NULL if none */
ConstrType contype; /* CHECK, NOT_NULL, or FOREIGN */
AttrNumber attnum; /* only relevant for NOT_NULL */
ConstrType contype; /* CHECK or FOREIGN */
Oid refrelid; /* PK rel, if FOREIGN */
Node *qual; /* Check expr or FkConstraint struct */
List *qualstate; /* Execution state for CHECK */
@ -2292,7 +2293,7 @@ ATRewriteTables(List **wqueue)
* constraints generated by ALTER TABLE commands, but don't
* rebuild data.
*/
if (tab->constraints != NIL)
if (tab->constraints != NIL || tab->new_notnull)
ATRewriteTable(tab, InvalidOid);
/*
@ -2358,6 +2359,7 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
TupleDesc oldTupDesc;
TupleDesc newTupDesc;
bool needscan = false;
List *notnull_attrs;
int i;
ListCell *l;
EState *estate;
@ -2408,9 +2410,6 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
case CONSTR_FOREIGN:
/* Nothing to do here */
break;
case CONSTR_NOTNULL:
needscan = true;
break;
default:
elog(ERROR, "unrecognized constraint type: %d",
(int) con->contype);
@ -2426,6 +2425,25 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
ex->exprstate = ExecPrepareExpr((Expr *) ex->expr, estate);
}
notnull_attrs = NIL;
if (newrel || tab->new_notnull)
{
/*
* If we are rebuilding the tuples OR if we added any new NOT NULL
* constraints, check all not-null constraints. This is a bit of
* overkill but it minimizes risk of bugs, and heap_attisnull is
* a pretty cheap test anyway.
*/
for (i = 0; i < newTupDesc->natts; i++)
{
if (newTupDesc->attrs[i]->attnotnull &&
!newTupDesc->attrs[i]->attisdropped)
notnull_attrs = lappend_int(notnull_attrs, i);
}
if (notnull_attrs)
needscan = true;
}
if (needscan)
{
ExprContext *econtext;
@ -2535,6 +2553,17 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
ExecStoreTuple(tuple, newslot, InvalidBuffer, false);
econtext->ecxt_scantuple = newslot;
foreach(l, notnull_attrs)
{
int attn = lfirst_int(l);
if (heap_attisnull(tuple, attn+1))
ereport(ERROR,
(errcode(ERRCODE_NOT_NULL_VIOLATION),
errmsg("column \"%s\" contains null values",
NameStr(newTupDesc->attrs[attn]->attname))));
}
foreach(l, tab->constraints)
{
NewConstraint *con = lfirst(l);
@ -2548,21 +2577,6 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
errmsg("check constraint \"%s\" is violated by some row",
con->name)));
break;
case CONSTR_NOTNULL:
{
Datum d;
bool isnull;
d = heap_getattr(tuple, con->attnum, newTupDesc,
&isnull);
if (isnull)
ereport(ERROR,
(errcode(ERRCODE_NOT_NULL_VIOLATION),
errmsg("column \"%s\" contains null values",
get_attname(tab->relid,
con->attnum))));
}
break;
case CONSTR_FOREIGN:
/* Nothing to do here */
break;
@ -3231,7 +3245,6 @@ ATExecSetNotNull(AlteredTableInfo *tab, Relation rel,
HeapTuple tuple;
AttrNumber attnum;
Relation attr_rel;
NewConstraint *newcon;
/*
* lookup the attribute
@ -3267,13 +3280,8 @@ ATExecSetNotNull(AlteredTableInfo *tab, Relation rel,
/* keep the system catalog indexes current */
CatalogUpdateIndexes(attr_rel, tuple);
/* Tell Phase 3 to test the constraint */
newcon = (NewConstraint *) palloc0(sizeof(NewConstraint));
newcon->contype = CONSTR_NOTNULL;
newcon->attnum = attnum;
newcon->name = "NOT NULL";
tab->constraints = lappend(tab->constraints, newcon);
/* Tell Phase 3 it needs to test the constraint */
tab->new_notnull = true;
}
heap_close(attr_rel, RowExclusiveLock);
@ -3741,7 +3749,6 @@ ATExecAddConstraint(AlteredTableInfo *tab, Relation rel, Node *newConstraint)
newcon = (NewConstraint *) palloc0(sizeof(NewConstraint));
newcon->name = ccon->name;
newcon->contype = ccon->contype;
newcon->attnum = ccon->attnum;
/* ExecQual wants implicit-AND format */
newcon->qual = (Node *)
make_ands_implicit((Expr *) ccon->expr);