From 8bd5a6af629141e0de1798570ea51db4bdabf6fe Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 7 Nov 2013 14:41:43 -0500 Subject: [PATCH] Prevent display of dropped columns in row constraint violation messages. ExecBuildSlotValueDescription() printed "null" for each dropped column in a row being complained of by ExecConstraints(). This has some sanity in terms of the underlying implementation, but is of course pretty surprising to users. To fix, we must pass the target relation's descriptor to ExecBuildSlotValueDescription(), because the slot descriptor it had been using doesn't get labeled with attisdropped markers. Per bug #8408 from Maxim Boguk. Back-patch to 9.2 where the feature of printing row values in NOT NULL and CHECK constraint violation messages was introduced. Michael Paquier and Tom Lane --- src/backend/executor/execMain.c | 43 ++++++++++++++++------- src/test/regress/expected/alter_table.out | 16 +++++++++ src/test/regress/sql/alter_table.sql | 9 +++++ 3 files changed, 56 insertions(+), 12 deletions(-) diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 56be2a78d8..8ffd92f27a 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -80,6 +80,7 @@ static void ExecutePlan(EState *estate, PlanState *planstate, static bool ExecCheckRTEPerms(RangeTblEntry *rte); static void ExecCheckXactReadOnly(PlannedStmt *plannedstmt); static char *ExecBuildSlotValueDescription(TupleTableSlot *slot, + TupleDesc tupdesc, int maxfieldlen); static void EvalPlanQualStart(EPQState *epqstate, EState *parentestate, Plan *planTree); @@ -1506,25 +1507,28 @@ ExecConstraints(ResultRelInfo *resultRelInfo, TupleTableSlot *slot, EState *estate) { Relation rel = resultRelInfo->ri_RelationDesc; - TupleConstr *constr = rel->rd_att->constr; + TupleDesc tupdesc = RelationGetDescr(rel); + TupleConstr *constr = tupdesc->constr; Assert(constr); if (constr->has_not_null) { - int natts = rel->rd_att->natts; + int natts = tupdesc->natts; int attrChk; for (attrChk = 1; attrChk <= natts; attrChk++) { - if (rel->rd_att->attrs[attrChk - 1]->attnotnull && + if (tupdesc->attrs[attrChk - 1]->attnotnull && slot_attisnull(slot, attrChk)) ereport(ERROR, (errcode(ERRCODE_NOT_NULL_VIOLATION), errmsg("null value in column \"%s\" violates not-null constraint", - NameStr(rel->rd_att->attrs[attrChk - 1]->attname)), + NameStr(tupdesc->attrs[attrChk - 1]->attname)), errdetail("Failing row contains %s.", - ExecBuildSlotValueDescription(slot, 64)))); + ExecBuildSlotValueDescription(slot, + tupdesc, + 64)))); } } @@ -1538,7 +1542,9 @@ ExecConstraints(ResultRelInfo *resultRelInfo, errmsg("new row for relation \"%s\" violates check constraint \"%s\"", RelationGetRelationName(rel), failed), errdetail("Failing row contains %s.", - ExecBuildSlotValueDescription(slot, 64)))); + ExecBuildSlotValueDescription(slot, + tupdesc, + 64)))); } } @@ -1546,15 +1552,22 @@ ExecConstraints(ResultRelInfo *resultRelInfo, * ExecBuildSlotValueDescription -- construct a string representing a tuple * * This is intentionally very similar to BuildIndexValueDescription, but - * unlike that function, we truncate long field values. That seems necessary - * here since heap field values could be very long, whereas index entries - * typically aren't so wide. + * unlike that function, we truncate long field values (to at most maxfieldlen + * bytes). That seems necessary here since heap field values could be very + * long, whereas index entries typically aren't so wide. + * + * Also, unlike the case with index entries, we need to be prepared to ignore + * dropped columns. We used to use the slot's tuple descriptor to decode the + * data, but the slot's descriptor doesn't identify dropped columns, so we + * now need to be passed the relation's descriptor. */ static char * -ExecBuildSlotValueDescription(TupleTableSlot *slot, int maxfieldlen) +ExecBuildSlotValueDescription(TupleTableSlot *slot, + TupleDesc tupdesc, + int maxfieldlen) { StringInfoData buf; - TupleDesc tupdesc = slot->tts_tupleDescriptor; + bool write_comma = false; int i; /* Make sure the tuple is fully deconstructed */ @@ -1569,6 +1582,10 @@ ExecBuildSlotValueDescription(TupleTableSlot *slot, int maxfieldlen) char *val; int vallen; + /* ignore dropped columns */ + if (tupdesc->attrs[i]->attisdropped) + continue; + if (slot->tts_isnull[i]) val = "null"; else @@ -1581,8 +1598,10 @@ ExecBuildSlotValueDescription(TupleTableSlot *slot, int maxfieldlen) val = OidOutputFunctionCall(foutoid, slot->tts_values[i]); } - if (i > 0) + if (write_comma) appendStringInfoString(&buf, ", "); + else + write_comma = true; /* truncate if needed */ vallen = strlen(val); diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 8e9d22836a..68d30acc1e 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -1223,6 +1223,22 @@ select * from atacc1; -- (1 row) +drop table atacc1; +-- test constraint error reporting in presence of dropped columns +create table atacc1 (id serial primary key, value int check (value < 10)); +NOTICE: CREATE TABLE will create implicit sequence "atacc1_id_seq" for serial column "atacc1.id" +NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "atacc1_pkey" for table "atacc1" +insert into atacc1(value) values (100); +ERROR: new row for relation "atacc1" violates check constraint "atacc1_value_check" +DETAIL: Failing row contains (1, 100). +alter table atacc1 drop column value; +alter table atacc1 add column value int check (value < 10); +insert into atacc1(value) values (100); +ERROR: new row for relation "atacc1" violates check constraint "atacc1_value_check" +DETAIL: Failing row contains (2, 100). +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 inheritance create table parent (a int, b int, c int); diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index dcf8121d70..404fa95e3f 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -874,6 +874,15 @@ select * from atacc1; drop table atacc1; +-- test constraint error reporting in presence of dropped columns +create table atacc1 (id serial primary key, value int check (value < 10)); +insert into atacc1(value) values (100); +alter table atacc1 drop column value; +alter table atacc1 add column value int check (value < 10); +insert into atacc1(value) values (100); +insert into atacc1(id, value) values (null, 0); +drop table atacc1; + -- test inheritance create table parent (a int, b int, c int); insert into parent values (1, 2, 3); -- 2.40.0