Postgres Pro
Downloads
Home   >   mailing lists

Re: Error message inconsistency - Mailing list pgsql-hackers

From Mahendra Singh Thalor
Subject Re: Error message inconsistency
Date
Msg-id CAKYtNAqYgdh1JhZ51cdEVHvg=1FzTeos3TQidHo8K2FnoNMBDA@mail.gmail.com
Whole thread Raw
In response to Re: Error message inconsistency  (Amit Kapila <amit.kapila16@gmail.com>)
Responses Re: Error message inconsistency  (MBeena Emerson <mbeena.emerson@gmail.com>)
List pgsql-hackers
Tree view
On Tue, 21 Jan 2020 at 10:51, Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Mon, Jan 6, 2020 at 6:31 PM Mahendra Singh Thalor <mahi6run@gmail.com> wrote:
> >
> > On Sat, 6 Jul 2019 at 09:53, Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > > On Mon, Jul 1, 2019 at 10:05 PM Alvaro Herrera <alvherre@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
Attachment

pgsql-hackers by date:

Previous
From: Amit Langote
Date:
Subject: Re: table partitioning and access privileges
Next
From: Daniel Gustafsson
Date:
Subject: Re: Increase psql's password buffer size