On Tue, 21 Jan 2020 at 10:51, Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Mon, Jan 6, 2020 at 6:31 PM Mahendra Singh Thalor <mahi6...@gmail.com> wrote: > > > > On Sat, 6 Jul 2019 at 09:53, Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > > > On Mon, Jul 1, 2019 at 10:05 PM Alvaro Herrera < alvhe...@2ndquadrant.com> wrote: > > > > > > > > Do we have an actual patch here? > > > > > > > > > > We have a patch, but it needs some more work like finding similar > > > places and change all of them at the same time and then change the > > > tests to adapt the same. > > > > > > > Hi all, > > Based on above discussion, I tried to find out all the places where we need to change error for "not null constraint". As Amit Kapila pointed out 1 place, I changed the error and adding modified patch. > > > > It seems you have not used the two error codes > (ERRCODE_NOT_NULL_VIOLATION and ERRCODE_CHECK_VIOLATION) pointed by me > above.
Thanks Amit and Beena for reviewing patch. Yes, you are correct. I searched using error messages not error code. That was my mistake. Now, I grepped using above error codes and found that these error codes are used in 19 places. Below is the code parts of 19 places. 1. src/backend/utils/adt/domains.c - 146 if (isnull) - 147 ereport(ERROR, - 148 (errcode(ERRCODE_NOT_NULL_VIOLATION), - 149 errmsg("domain %s does not allow null values", - 150 format_type_be(my_extra->domain_type)), - 151 errdatatype(my_extra->domain_type))); - 152 break; I think, above error is for domain, so there is no need to add anything in error message. ----------------------------------------------------------------------------------------------------- 2. src/backend/utils/adt/domains.c - 181 if (!ExecCheck(con->check_exprstate, econtext)) - 182 ereport(ERROR, - 183 (errcode(ERRCODE_CHECK_VIOLATION), - 184 errmsg("value for domain %s violates check constraint \"%s\"", - 185 format_type_be(my_extra->domain_type), - 186 con->name), - 187 errdomainconstraint(my_extra->domain_type, - 188 con->name))); I think, above error is for domain, so there is no need to add anything in error message. ----------------------------------------------------------------------------------------------------- 3. src/backend/partitioning/partbounds.c - 1330 if (part_rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE) - 1331 ereport(WARNING, - 1332 (errcode(ERRCODE_CHECK_VIOLATION), - 1333 errmsg("skipped scanning foreign table \"%s\" which is a partition of default partition \"%s\"", - 1334 RelationGetRelationName(part_rel), - 1335 RelationGetRelationName(default_rel)))); Relation name is already appended in error messgae. ----------------------------------------------------------------------------------------------------- 4. src/backend/partitioning/partbounds.c - 1363 if (!ExecCheck(partqualstate, econtext)) - 1364 ereport(ERROR, - 1365 (errcode(ERRCODE_CHECK_VIOLATION), - 1366 errmsg("updated partition constraint for default partition \"%s\" would be violated by some row", - 1367 RelationGetRelationName(default_rel)))); Relation name is already appended in error messgae. ----------------------------------------------------------------------------------------------------- 5. src/backend/executor/execPartition.c - 342 ereport(ERROR, - 343 (errcode(ERRCODE_CHECK_VIOLATION), - 344 errmsg("no partition of relation \"%s\" found for row", - 345 RelationGetRelationName(rel)), - 346 val_desc ? - 347 errdetail("Partition key of the failing row contains %s.", - 348 val_desc) : 0)); Relation name is already appended in error messgae. ----------------------------------------------------------------------------------------------------- 6. src/backend/executor/execMain.c - 1877 ereport(ERROR, - 1878 (errcode(ERRCODE_CHECK_VIOLATION), - 1879 errmsg("new row for relation \"%s\" violates partition constraint", - 1880 RelationGetRelationName(resultRelInfo->ri_RelationDesc)), - 1881 val_desc ? errdetail("Failing row contains %s.", val_desc) : 0)); Relation name is already appended in error messgae. ----------------------------------------------------------------------------------------------------- 7. src/backend/executor/execMain.c - 1958 ereport(ERROR, - 1959 (errcode(ERRCODE_NOT_NULL_VIOLATION), - 1960 errmsg("null value in column \"%s\" violates not-null constraint", - 1961 NameStr(att->attname)), - 1962 val_desc ? errdetail("Failing row contains %s.", val_desc) : 0, - 1963 errtablecol(orig_rel, attrChk))); Added relation name for this error. This can be verified by below example: *Ex:* CREATE TABLE test (a int PRIMARY KEY, b int GENERATED ALWAYS AS (nullif(a, 0)) STORED NOT NULL); INSERT INTO test (a) VALUES (1); INSERT INTO test (a) VALUES (0); *Without patch:* ERROR: null value in column "b" violates not-null constraint DETAIL: Failing row contains (0, null). *With patch:* ERROR: null value in column "b" of relation "test" violates not-null constraint DETAIL: Failing row contains (0, null). ----------------------------------------------------------------------------------------------------- 8. src/backend/executor/execMain.c - 2006 ereport(ERROR, - 2007 (errcode(ERRCODE_CHECK_VIOLATION), - 2008 errmsg("new row for relation \"%s\" violates check constraint \"%s\"", - 2009 RelationGetRelationName(orig_rel), failed), - 2010 val_desc ? errdetail("Failing row contains %s.", val_desc) : 0, - 2011 errtableconstraint(orig_rel, failed))); Relation name is already appended in error messgae. ----------------------------------------------------------------------------------------------------- 9. src/backend/executor/execExprInterp.c - 3600 ereport(ERROR, - 3601 (errcode(ERRCODE_NOT_NULL_VIOLATION), - 3602 errmsg("domain %s does not allow null values", - 3603 format_type_be(op->d.domaincheck.resulttype)), - 3604 errdatatype(op->d.domaincheck.resulttype))); I think, above error is for domain, so there is no need to add anything in error message. ----------------------------------------------------------------------------------------------------- 10. src/backend/executor/execExprInterp.c - 3615 ereport(ERROR, - 3616 (errcode(ERRCODE_CHECK_VIOLATION), - 3617 errmsg("value for domain %s violates check constraint \"%s\"", - 3618 format_type_be(op->d.domaincheck.resulttype), - 3619 op->d.domaincheck.constraintname), - 3620 errdomainconstraint(op->d.domaincheck.resulttype, - 3621 op->d.domaincheck.constraintname))); I think, above error is for domain, so there is no need to add anything in error message. ----------------------------------------------------------------------------------------------------- 11. src/backend/commands/tablecmds.c - 5273 ereport(ERROR, - 5274 (errcode(ERRCODE_NOT_NULL_VIOLATION), - 5275 errmsg("column \"%s\" contains null values", - 5276 NameStr(attr->attname)), - 5277 errtablecol(oldrel, attn + 1))); Added relation name for this error. This can be verified by below example: *Ex:* CREATE TABLE test (a int); INSERT INTO test VALUES (0), (1); ALTER TABLE test ADD COLUMN b int NOT NULL, ALTER COLUMN b ADD GENERATED ALWAYS AS IDENTITY; *Without patch:* ERROR: column "b" contains null values *With patch*: ERROR: column "b" of relation "test" contains null values ----------------------------------------------------------------------------------------------------- 12. src/backend/commands/tablecmds.c - 5288 if (!ExecCheck(con->qualstate, econtext)) - 5289 ereport(ERROR, - 5290 (errcode(ERRCODE_CHECK_VIOLATION), - 5291 errmsg("check constraint \"%s\" is violated by some row", - 5292 con->name), - 5293 errtableconstraint(oldrel, con->name))); Added relation name for this error. This can be verified by below example: *Ex:* CREATE TABLE test (a int PRIMARY KEY, b int GENERATED ALWAYS AS (a * 2) STORED); INSERT INTO test (a) VALUES (10), (30); ALTER TABLE test ADD CHECK (b < 50); *Without patch:* ERROR: check constraint "test_b_check" is violated by some row *With patch:* ERROR: check constraint "test_b_check" of relation "test" is violated by some row ----------------------------------------------------------------------------------------------------- 13. src/backend/commands/tablecmds.c - 5306 if (tab->validate_default) - 5307 ereport(ERROR, - 5308 (errcode(ERRCODE_CHECK_VIOLATION), - 5309 errmsg("updated partition constraint for default partition would be violated by some row"))); Added relation name for this error. This can be verified by below example: *Ex:* CREATE TABLE list_parted ( a int, b char ) PARTITION BY LIST (a); CREATE TABLE list_parted_def PARTITION OF list_parted DEFAULT; INSERT INTO list_parted_def VALUES (11, 'z'); CREATE TABLE part_1 (LIKE list_parted); ALTER TABLE list_parted ATTACH PARTITION part_1 FOR VALUES IN (11); *Without patch:* ERROR: updated partition constraint for default partition would be violated by some row *With patch:* ERROR: updated partition constraint for default partition "list_parted_def" would be violated by some row ----------------------------------------------------------------------------------------------------- 14. src/backend/commands/tablecmds.c - 5310 else - 5311 ereport(ERROR, - 5312 (errcode(ERRCODE_CHECK_VIOLATION), - 5313 errmsg("partition constraint is violated by some row"))); Added relation name for this error. This can be verified by below example: *Ex:* CREATE TABLE list_parted (a int,b char)PARTITION BY LIST (a); CREATE TABLE part_1 (LIKE list_parted); INSERT INTO part_1 VALUES (3, 'a'); ALTER TABLE list_parted ATTACH PARTITION part_1 FOR VALUES IN (2); *Without patch:* ERROR: partition constraint is violated by some row *With patch:* ERROR: partition constraint "part_1" is violated by some row --------------------------------------------------------------------------- 15. src/backend/commands/tablecmds.c - 10141 ereport(ERROR, - 10142 (errcode(ERRCODE_CHECK_VIOLATION), - 10143 errmsg("check constraint \"%s\" is violated by some row", - 10144 NameStr(constrForm->conname)), - 10145 errtableconstraint(rel, NameStr(constrForm->conname)))); Added relation name for this error. This can be verified by below example: *Ex:* CREATE TABLE test (a int PRIMARY KEY, b int GENERATED ALWAYS AS (a * 2) STORED); INSERT INTO test (a) VALUES (10), (30); ALTER TABLE test ADD CONSTRAINT chk CHECK (b < 50) NOT VALID; ALTER TABLE test VALIDATE CONSTRAINT chk; *Without patch:* ERROR: check constraint "chk" is violated by some row *With patch:* ERROR: check constraint "chk" of relation "test" is violated by some row ----------------------------------------------------------------------------------------------------- 16. src/backend/commands/typecmds.c - 2396 ereport(ERROR, - 2397 (errcode(ERRCODE_NOT_NULL_VIOLATION), - 2398 errmsg("column \"%s\" of table \"%s\" contains null values", - 2399 NameStr(attr->attname), - 2400 RelationGetRelationName(testrel)), - 2401 errtablecol(testrel, attnum))); Relation name is already appended in error messgae. ----------------------------------------------------------------------------------------------------- 17. src/backend/commands/typecmds.c - 2824 ereport(ERROR, - 2825 (errcode(ERRCODE_CHECK_VIOLATION), - 2826 errmsg("column \"%s\" of table \"%s\" contains values that violate the new constraint", - 2827 NameStr(attr->attname), - 2828 RelationGetRelationName(testrel)), - 2829 errtablecol(testrel, attnum))); Relation name is already appended in error messgae. ----------------------------------------------------------------------------------------------------- 18. src/backend/commands/typecmds.c - 2396 ereport(ERROR, - 2397 (errcode(ERRCODE_NOT_NULL_VIOLATION), - 2398 errmsg("column \"%s\" of table \"%s\" contains null values", - 2399 NameStr(attr->attname), - 2400 RelationGetRelationName(testrel)), - 2401 errtablecol(testrel, attnum))); Relation name is already appended in error messgae. ----------------------------------------------------------------------------------------------------- 19. src/backend/commands/typecmds.c - 2824 ereport(ERROR, - 2825 (errcode(ERRCODE_CHECK_VIOLATION), - 2826 errmsg("column \"%s\" of table \"%s\" contains values that violate the new constraint", - 2827 NameStr(attr->attname), - 2828 RelationGetRelationName(testrel)), - 2829 errtablecol(testrel, attnum))) Relation name is already appended in error messgae. ----------------------------------------------------------------------------------------------------- > > > What does this patch? > > Before this patch, to display error of "not-null constraint", we were not displaying relation name in some cases so attached patch is adding relation name with the "not-null constraint" error in 2 places. I didn't changed out files of test suite as we haven't finalized error messages. > > > > I verified Robert's point of for partition tables also. With the error, we are adding relation name of "child table" and i think, it is correct. > > > > Can you show the same with the help of an example? Okay. Below is the example: create table parent (a int, b int not null) partition by range (a); create table ch1 partition of parent for values from ( 10 ) to (20); postgres=# insert into parent values (9); ERROR: no partition of relation "parent" found for row DETAIL: Partition key of the failing row contains (a) = (9). postgres=# insert into parent values (11); *ERROR: null value in column "b" of relation "ch1" violates not-null constraint* DETAIL: Failing row contains (11, null). Attaching a patch for review. In this patch, total 6 places I added relation name in error message and verifyed same with above mentioned examples. Please review attahced patch and let me know your feedback. I haven't modifed .out files because we haven't finalied patch. -- Thanks and Regards Mahendra Singh Thalor EnterpriseDB: http://www.enterprisedb.com
rationalize_constraint_error_messages_v3.patch
Description: Binary data