patch for 9.2: enhanced errors

On 11-06-08 04:14 PM, Pavel Stehule wrote:

Hello

Attached patch implements a new erros's fields that describes table,
colums related to error. This enhanced info is limited to constraints
and RI.

Here is my review of this patch

Submission Review:
------------------------
The patch applies cleanly against master
The patch does not include any documentation updates (see note below to
update config.sgml)
The patch does not include any unit tests. At a minimum it should add a
few tests with verbosity set to verbose

Usability Review
--------------------
The patch adds the ability to get more information about the reasons a
query failed. Pavel indicates that this is a building block for a later
patch. This sounds like a worthwhile goal, without this patch I don't
see another good way of getting the details on what columns make up the
constraint that fails, other than making additional queries into the
catalog.

This patch should not impact pg_dump or pg_upgrade.

Pavel has submitted a related patch that adds support for this feature
to plpgsql, in theory other pl's might want to use the information this
patch exposes.

Feature Test
-------------------

The error messages behave as described with \set verbosity verbose.

I tried this using both the 8.3 and 9.0 versions of psql (against a
postgresql server with this patch) and things worked as expected (the
extra error messages did not show). I also tried the patched psql
against an 8.3 backend and verified that we don't get strange behaviour
going against an older backend with verbosity set.

I tried adding multiple constraints to a table and inserting a row that
violates them, only one of the constraints showed up in the error
message, this is fine and consistent with the existing behaviour

Consider this example of an error that gets generated

ERROR: 23505: duplicate key value violates unique constraint "A_pkey"
DETAIL: Key (a)=(1) already exists.
LOCATION: _bt_check_unique, nbtinsert.c:433
CONSTRAINT: A_pkey
SCHEMA: public
TABLE: A
COLUMN: a
STATEMENT: insert into "A" values (1);

I think that both the CONSTRAINT, and TABLE name should be double quoted
like "A_pkey" is above. When doing this make sure you don't break the
quoting in the CSV format log.

Performance Review
-----------------------------
I don't see this patch impacting performance, I did not conduct any
performance tests.

Coding Review
-----------------

In tupdesc.c

line 202 the existing code is performing a deep copy of ConstrCheck. Do
you need to copy nkeys and conkey here as well?

Then at line 250 ccname is freed but not conkey

postgres_ext.h line 55
+ #define PG_DIAG_SCHEMA_NAME    's'
+ #define PG_DIAG_TABLE_NAME    't'
+ #define PG_DIAG_COLUMN_NAMES    'c'
+ #define PG_DIAG_CONSTRAINT_NAME 'n'

The assignment of letters to parameters seems arbitrary to me, I don't
have a different non-arbitrary mapping in mind but if anyone else does
they should speak up. I think it will be difficult to change this after
9.2 goes out.

elog.c:
***************
*** 2197,2202 ****
--- 2299,2319 ----
       if (application_name)
           appendCSVLiteral(&buf, application_name);

+     /* constraint_name */
+     appendCSVLiteral(&buf, edata->constraint_name);
+     appendStringInfoChar(&buf, ',');
+
+     /* schema name */
+     appendCSVLiteral(&buf, edata->schema_name);
+     appendStringInfoChar(&buf, ',');

You need to update config.sgml at the same time you update this format.
You need to append a "," after application name but before
constraintName. As it stands the CSV log has something like:
.....nbtinsert.c:433","psql""a_pkey","public","a","a"

nbtinsert.c

pg_get_indrelation is named differently than everything else in this
file (ie _bt...). My guess is that this function belongs somewhere else
but I don't know the code well enough to say where you should move it too.

Everything I've mentioned above is a minor issue, I will move the patch
to 'waiting for author' and wait for you to release an updated patch.

Steve Singer

On 11-06-18 06:36 PM, Steve Singer wrote:

On 11-06-08 04:14 PM, Pavel Stehule wrote:

Here is my review of this patch

Submission Review:
------------------------
The patch applies cleanly against master
The patch does not include any documentation updates (see note below
to update config.sgml)
The patch does not include any unit tests. At a minimum it should add
a few tests with verbosity set to verbose

On second thought tests might not work. Is the only way to get the
constraint details are in verbose mode where line numbers from the c
file are also included? If so then this won't work for the regression
tests. Having the diff comparison fail every time someone makes an
unrelated change to a source file isn't what we want.

Hello

2011/6/19 Steve Singer <ssinger_pg@sympatico.ca>:

On 11-06-08 04:14 PM, Pavel Stehule wrote:

Hello

Attached patch implements a new erros's fields that describes table,
colums related to error. This enhanced info is limited to constraints
and RI.

...

I think that both the CONSTRAINT, and TABLE name should be double quoted
like "A_pkey" is above.  When doing this make sure you don't break the
quoting in the CSV format log.

I agree so quoting must be used in CSV log - the result have to be
valid CSV and I'll ensure this. I am not sure about implicit quoting
and using some quote_ident operation early. This is result of some
operation - not input. Quoting in message is used not like SQL
quoting, but as plain text quoting - it is just border between human
readable text and data. But fields like TABLE_NAME or COLUMN_NAME
contains just data - so quoting is useless.

Next argument - the quoting is more simple than remove quoting. If
somebody needs to quoting, then can use a quoting_ident function, but
there are no inverse function - so I prefer a names in raw format. It
is more simply and usual to add quoting than remove quoting.

What do you think about?

Performance Review
-----------------------------
I don't see this patch impacting performance, I did not conduct any
performance tests.

Coding Review
-----------------

In tupdesc.c

line 202 the existing code is performing a deep copy of ConstrCheck.  Do you
need to copy nkeys and conkey here as well?

Then at line 250 ccname is freed but not conkey

I have to look on this

postgres_ext.h line 55
+ #define PG_DIAG_SCHEMA_NAME    's'
+ #define PG_DIAG_TABLE_NAME    't'
+ #define PG_DIAG_COLUMN_NAMES    'c'
+ #define PG_DIAG_CONSTRAINT_NAME 'n'

The assignment of letters to parameters seems arbitrary to me, I don't have
a different non-arbitrary mapping in mind but if anyone else does they
should speak up.  I think it will be difficult to change this after 9.2 goes
out.

elog.c:
***************
*** 2197,2202 ****
--- 2299,2319 ----
     if (application_name)
         appendCSVLiteral(&buf, application_name);

+     /* constraint_name */
+     appendCSVLiteral(&buf, edata->constraint_name);
+     appendStringInfoChar(&buf, ',');
+
+     /* schema name */
+     appendCSVLiteral(&buf, edata->schema_name);
+     appendStringInfoChar(&buf, ',');

You need to update config.sgml at the same time you update this format.
You need to append a "," after application name but before constraintName.
As it stands the CSV log has something like:
.....nbtinsert.c:433","psql""a_pkey","public","a","a"

ok

nbtinsert.c

pg_get_indrelation is named differently than everything else in this file
(ie _bt...).  My guess is that this function belongs somewhere else but I
don't know the code well enough to say where you should move it too.

I'll try to get better name, but I would not use a public name like _bt

Everything I've mentioned above is a minor issue, I will move the patch to
'waiting for author' and wait for you to release an updated patch.

Steve Singer

ok

Thank you very much

Pavel Stehule

2011/6/19 Steve Singer <ssinger_pg@sympatico.ca>:

On 11-06-18 06:36 PM, Steve Singer wrote:

On 11-06-08 04:14 PM, Pavel Stehule wrote:

Here is my review of this patch

Submission Review:
------------------------
The patch applies cleanly against master
The patch does not include any documentation updates (see note below to
update config.sgml)
The patch does not include any unit tests. At a minimum it should add a
few tests with verbosity set to verbose

On second thought tests might not work. Is the only way to get the
constraint details are in verbose mode where line numbers from the c file
are also included? If so then this won't work for the regression tests.
Having the diff comparison fail every time someone makes an unrelated change
to a source file isn't what we want.

it is reason why patch doesn't any regress test changes. I have to
look, if verbose mode is documented somewhere.

Regards

Pavel Stehule

On Jun19, 2011, at 05:10 , Steve Singer wrote:

On 11-06-18 06:36 PM, Steve Singer wrote:

On 11-06-08 04:14 PM, Pavel Stehule wrote:

Here is my review of this patch

Submission Review:
------------------------
The patch applies cleanly against master
The patch does not include any documentation updates (see note below to update config.sgml)
The patch does not include any unit tests. At a minimum it should add a few tests with verbosity set to verbose

On second thought tests might not work. Is the only way to get the constraint details are in verbose mode where line numbers from the c file are also included? If so then this won't work for the regression tests. Having the diff comparison fail every time someone makes an unrelated change to a source file isn't what we want.

Speaking as someone who's wished for the feature that Pavel's patch provides
many times in the past - shouldn't there also be a field containing the
offending value? If we had that, it'd finally be possible to translate
constraint-related error messages to informative messages for the user.

best regards,
Florian Pflug

2011/6/19 Florian Pflug <fgp@phlo.org>:

On Jun19, 2011, at 05:10 , Steve Singer wrote:

On 11-06-18 06:36 PM, Steve Singer wrote:

On 11-06-08 04:14 PM, Pavel Stehule wrote:

Here is my review of this patch

Submission Review:
------------------------
The patch applies cleanly against master
The patch does not include any documentation updates (see note below to update config.sgml)
The patch does not include any unit tests. At a minimum it should add a few tests with verbosity set to verbose

On second thought tests might not work. Is the only way to get the constraint details are in verbose mode where line numbers from the c file are also included? If so then this won't work for the regression tests.   Having the diff comparison fail every time someone makes an unrelated change to a source file isn't what we want.

Speaking as someone who's wished for the feature that Pavel's patch provides
many times in the past - shouldn't there also be a field containing the
offending value? If we had that, it'd finally be possible to translate
constraint-related error messages to informative messages for the user.

The value is available in almost cases. There is only one issue - this
should not be only one value - it could be list of values - so basic
question is about format and property name. PostgreSQL doesn't hold
relation between column and column constraint - all column constraints
are transformed to table constrains. All column informations are
derived from constraint - so when constraint is a > b and this
constraint is false, we have two values.

Maybe there is second issue (little bit - performance - you have to
call a output function), But I agree, so this information is very
interesting and can help.

I am open for any ideas in this direction.

Regards

Pavel

On Jun19, 2011, at 14:03 , Pavel Stehule wrote:

2011/6/19 Florian Pflug <fgp@phlo.org>:

Speaking as someone who's wished for the feature that Pavel's patch provides
many times in the past - shouldn't there also be a field containing the
offending value? If we had that, it'd finally be possible to translate
constraint-related error messages to informative messages for the user.

The value is available in almost cases. There is only one issue - this
should not be only one value - it could be list of values - so basic
question is about format and property name. PostgreSQL doesn't hold
relation between column and column constraint - all column constraints
are transformed to table constrains. All column informations are
derived from constraint - so when constraint is a > b and this
constraint is false, we have two values.

Hm, you could rename COLUMN to VALUE, make it include the value,
and repeat it for every column in the constraint or index that caused
the error. For example, you'd get

VALUE: "a":5
VALUE: "b":3

if you violated a CHECK constraint asserting that "a < b".

You could also use that in custom constraint enforcement triggers -
i.e. I'm maintaining an application that enforces foreign key
constraints for arrays. With VALUE fields available, I could emit
one value field for every offending array member.

If repeating the same field multiple times is undesirable, the
information could of course be packed into one field, giving

VALUES: ("a":5, "b":3)

for the example from above. My array FK constraint trigger would
the presumably report

VALUES: ("array_field":42, "array_field":23)

if array members 42 and 23 lacked a corresponding row in the
referenced table.

That'd also work work for foreign keys and unique constraints. Exclusion
constraints are harder, because there the conflicting value might also
be of interest. (Hm, actually it might even be for unique indices if
some columns are NULL - not sure right now if there's a mode where we
treat NULL as a kind of wildcard...).

best regards,
Florian Pflug

2011/6/19 Florian Pflug <fgp@phlo.org>:

On Jun19, 2011, at 14:03 , Pavel Stehule wrote:

2011/6/19 Florian Pflug <fgp@phlo.org>:

Speaking as someone who's wished for the feature that Pavel's patch provides
many times in the past - shouldn't there also be a field containing the
offending value? If we had that, it'd finally be possible to translate
constraint-related error messages to informative messages for the user.

The value is available in almost cases. There is only one issue - this
should not be only one value - it could be list of values - so basic
question is about format and property name. PostgreSQL doesn't hold
relation between column and column constraint - all column constraints
are transformed to table constrains. All column informations are
derived from constraint - so when constraint is a > b and this
constraint is false, we have two values.

Hm, you could rename COLUMN to VALUE, make it include the value,
and repeat it for every column in the constraint or index that caused
the error. For example, you'd get

VALUE: "a":5
VALUE: "b":3

I don't have a idea. These data should be available via GET
DIAGNOSTICS statement, so you can't use a repeated properties. I would
to use a simple access to column names because it is in ANSI SQL.

if you violated a CHECK constraint asserting that "a < b".

You could also use that in custom constraint enforcement triggers -
i.e. I'm maintaining an application that enforces foreign key
constraints for arrays. With VALUE fields available, I could emit
one value field for every offending array member.

If repeating the same field multiple times is undesirable, the
information could of course be packed into one field, giving

VALUES: ("a":5, "b":3)

for the example from above. My array FK constraint trigger would
the presumably report

VALUES: ("array_field":42, "array_field":23)

there should be some similar, but probably we need to have some
dictionary type in core before. If we are too hurry, then we can have
a problem with backing compatibility :(. Theoretically we have a know
columns in COLUMNS property, so we can serialize values in same order
in serialized array format.

COLUMNS: a, b, c
VALUES: some, else, "some with \" or , "

Regards

Pavel

On Sun, 19 Jun 2011, Pavel Stehule wrote:

Maybe there is second issue (little bit - performance - you have to
call a output function), But I agree, so this information is very
interesting and can help.

I am concerned about the performance impact of doing that. Not all
constraints are on int4 columns. Some constraints might be on a geometry
type that is megabytes in side taking a substantial chunk of CPU and
bandwith to convert it into a text representation and then send it back to
the client.

Excerpts from Pavel Stehule's message of dom jun 19 06:51:13 -0400 2011:

Hello

2011/6/19 Steve Singer <ssinger_pg@sympatico.ca>:

On 11-06-08 04:14 PM, Pavel Stehule wrote:

nbtinsert.c

pg_get_indrelation is named differently than everything else in this file
(ie _bt...).  My guess is that this function belongs somewhere else but I
don't know the code well enough to say where you should move it too.

I'll try to get better name, but I would not use a public name like _bt

lsyscache.c?

--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Hello

I am sending a updated patch

Coding Review
-----------------

In tupdesc.c

line 202 the existing code is performing a deep copy of ConstrCheck.  Do you
need to copy nkeys and conkey here as well?

Then at line 250 ccname is freed but not conkey

fixed

postgres_ext.h line 55
+ #define PG_DIAG_SCHEMA_NAME    's'
+ #define PG_DIAG_TABLE_NAME    't'
+ #define PG_DIAG_COLUMN_NAMES    'c'
+ #define PG_DIAG_CONSTRAINT_NAME 'n'

The assignment of letters to parameters seems arbitrary to me, I don't have
a different non-arbitrary mapping in mind but if anyone else does they
should speak up.  I think it will be difficult to change this after 9.2 goes
out.

elog.c:
***************
*** 2197,2202 ****
--- 2299,2319 ----
     if (application_name)
         appendCSVLiteral(&buf, application_name);

+     /* constraint_name */
+     appendCSVLiteral(&buf, edata->constraint_name);
+     appendStringInfoChar(&buf, ',');
+
+     /* schema name */
+     appendCSVLiteral(&buf, edata->schema_name);
+     appendStringInfoChar(&buf, ',');

You need to update config.sgml at the same time you update this format.
You need to append a "," after application name but before constraintName.
As it stands the CSV log has something like:
.....nbtinsert.c:433","psql""a_pkey","public","a","a"

fixed

nbtinsert.c

pg_get_indrelation is named differently than everything else in this file
(ie _bt...).  My guess is that this function belongs somewhere else but I
don't know the code well enough to say where you should move it too.

I renamed this function to IndexRelationGetParentRelation and muved to
relcache.c

I don't call a quote_identifier on only data error properties like
table_name or schema_name (but I am open to arguments for it or
against it). The quote_identifier is used for column names, because
there should be a more names and comma should be used inside name -
and this is consistent with pg_get_indexdef_columns.

Regards

Pavel Stehule

On 11-06-20 03:44 PM, Pavel Stehule wrote:

Hello

You need to update config.sgml at the same time you update this format.
You need to append a "," after application name but before constraintName.
As it stands the CSV log has something like:
.....nbtinsert.c:433","psql""a_pkey","public","a","a"

fixed

The CSV log seems fine now.

nbtinsert.c

pg_get_indrelation is named differently than everything else in this file
(ie _bt...). My guess is that this function belongs somewhere else but I
don't know the code well enough to say where you should move it too.

I renamed this function to IndexRelationGetParentRelation and muved to
relcache.c

Thanks, it looks less out of place there than it did in nbtinsert.c

I don't call a quote_identifier on only data error properties like
table_name or schema_name (but I am open to arguments for it or
against it). The quote_identifier is used for column names, because
there should be a more names and comma should be used inside name -
and this is consistent with pg_get_indexdef_columns.

Regards

Okay.

Pavel Stehule

I'm going to mark this as ready for a committer.

2011/6/21 Steve Singer <ssinger_pg@sympatico.ca>:

On 11-06-20 03:44 PM, Pavel Stehule wrote:

Hello

You need to update config.sgml at the same time you update this format.
You need to append a "," after application name but before constraintName.
As it stands the CSV log has something like:
.....nbtinsert.c:433","psql""a_pkey","public","a","a"

fixed

The CSV log seems fine now.

nbtinsert.c

pg_get_indrelation is named differently than everything else in this file
(ie _bt...).  My guess is that this function belongs somewhere else but I
don't know the code well enough to say where you should move it too.

I renamed this function to IndexRelationGetParentRelation and muved to
relcache.c

Thanks, it looks less out of place there than it did in nbtinsert.c

I don't call a quote_identifier on only data error properties like
table_name or schema_name (but I am open to arguments for it or
against it). The quote_identifier is used for column names, because
there should be a more names and comma should be used inside name -
and this is consistent with pg_get_indexdef_columns.

Regards

Okay.

Pavel Stehule

I'm going to mark this as ready for a committer.

Thank you very much

Regards

Pavel Stehule

Pavel Stehule <pavel.stehule@gmail.com> writes:

I am sending a updated patch

I looked over this patch a bit. I guess my main concern about it
is that the set of items to be reported seems to have been made up on
a whim. I think that we ought to follow the SQL standard, which has a
pretty clearly defined set of additional information items --- look at
the spec for the GET DIAGNOSTICS statement. (In SQL:2008, this is
section 23.1 <get diagnostics statement>.) I don't feel that we need to
implement every field the standard calls for, at least not right away,
but we ought to have their list in mind. Conversely, implementing items
that *aren't* listed in the spec has to meet a considerably higher bar
than somebody just submitting a patch that does it.

The standard information items that seem reasonable for us to implement
in the near future are

COLUMN_NAME
CONSTRAINT_NAME
CONSTRAINT_SCHEMA
SCHEMA_NAME
TABLE_NAME
TRIGGER_NAME
TRIGGER_SCHEMA

So I'd like to see the patch revised to use this terminology. We
probably also need to think a bit harder about the PG_DIAG_XXX code
letters to be used --- we're already just about at the limit of what
fields can have reasonably-mnemonic code letters, and not all of the
above have obvious choices, let alone the rest of what's in the spec
that we might someday want to implement. What assignment rule should
we use when we can't choose a mnemonic letter?

Some other specific comments on the patch follow:

1. It's way short in the documentation department. protocol.sgml
certainly needs additions (see "Error and Notice Message Fields"),
also libpq.sgml's discussion of PQresultErrorField(), also
sources.sgml's "Reporting Errors Within the Server", and I'm not
sure where else.

2. I think you could drop the tuple-descriptor changes, because they're
only needed in service of an information item that is not found in the
standard and doesn't seem very necessary. The standard says to report
the name of the constraint, not what columns it involves.

3. errrel() is extremely poorly considered. The fact that it requires
utils/relcache.h to be #included by elog.h (and therefore by *every*
*single* *file* in the backend) is a symptom of that, but expecting
elog.c to do catalog lookups is as bad or worse from a modularity
standpoint. I think all the added elog functions should not take
anything higher-level than a C string.

4. Actually, it would probably be a good idea to avoid inventing a new
elog API function for each individual new information item; something
along the lines of "erritem(PG_DIAG_WHATEVER, string_value)" would be
more appropriate to cover the inevitable future expansions.

5. I don't think IndexRelationGetParentRelation is very appropriate
either --- in the use cases you have, the parent table's OID is easily
accessible, as is its namespace (which'll be the same as the index's)
and so you could just have the callers do get_rel_name(tableoid).
Doing a relcache open in an error reporting path seems like overkill.

I'm going to mark this patch Returned With Feedback.

regards, tom lane

Hello Tom,

Thank you for review

I am thinking, so your comment is clean and I'll respect it in new version.

There is only one issue, that should be solved first. I introduced non
standard diagnostics field "column_names", because there is not
possible get "column_name" value for check constraints now. A correct
implementation of COLUMN_NAME field needs a explicit relation between
pg_constraint and pg_attribute - maybe implemented as new column to
pg_constraint. Do you agree?

Regards

Pavel

2011/7/16 Tom Lane <tgl@sss.pgh.pa.us>:

Pavel Stehule <pavel.stehule@gmail.com> writes:

I am sending a updated patch

I looked over this patch a bit.  I guess my main concern about it
is that the set of items to be reported seems to have been made up on
a whim.  I think that we ought to follow the SQL standard, which has a
pretty clearly defined set of additional information items --- look at
the spec for the GET DIAGNOSTICS statement.  (In SQL:2008, this is
section 23.1 <get diagnostics statement>.)  I don't feel that we need to
implement every field the standard calls for, at least not right away,
but we ought to have their list in mind.  Conversely, implementing items
that *aren't* listed in the spec has to meet a considerably higher bar
than somebody just submitting a patch that does it.

The standard information items that seem reasonable for us to implement
in the near future are

       COLUMN_NAME
       CONSTRAINT_NAME
       CONSTRAINT_SCHEMA
       SCHEMA_NAME
       TABLE_NAME
       TRIGGER_NAME
       TRIGGER_SCHEMA

So I'd like to see the patch revised to use this terminology.  We
probably also need to think a bit harder about the PG_DIAG_XXX code
letters to be used --- we're already just about at the limit of what
fields can have reasonably-mnemonic code letters, and not all of the
above have obvious choices, let alone the rest of what's in the spec
that we might someday want to implement.  What assignment rule should
we use when we can't choose a mnemonic letter?

Some other specific comments on the patch follow:

1. It's way short in the documentation department.  protocol.sgml
certainly needs additions (see "Error and Notice Message Fields"),
also libpq.sgml's discussion of PQresultErrorField(), also
sources.sgml's "Reporting Errors Within the Server", and I'm not
sure where else.

ok

Pavel Stehule <pavel.stehule@gmail.com> writes:

There is only one issue, that should be solved first. I introduced non
standard diagnostics field "column_names", because there is not
possible get "column_name" value for check constraints now. A correct
implementation of COLUMN_NAME field needs a explicit relation between
pg_constraint and pg_attribute - maybe implemented as new column to
pg_constraint. Do you agree?

No, I don't. You're adding complication to solve a problem that doesn't
need to be solved. The standard says to return the name of the
constraint for a constraint-violation failure. It does not say anything
about naming the associated column(s). COLUMN_NAME is only supposed to
be defined for certain kinds of errors, and this isn't one of them.

regards, tom lane

Tom,

No, I don't. You're adding complication to solve a problem that doesn't
need to be solved. The standard says to return the name of the
constraint for a constraint-violation failure. It does not say anything
about naming the associated column(s). COLUMN_NAME is only supposed to
be defined for certain kinds of errors, and this isn't one of them.

Are we talking about FK constraints here, or CHECK contstraints?

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

Josh Berkus <josh@agliodbs.com> writes:

Tom,

No, I don't. You're adding complication to solve a problem that doesn't
need to be solved. The standard says to return the name of the
constraint for a constraint-violation failure. It does not say anything
about naming the associated column(s). COLUMN_NAME is only supposed to
be defined for certain kinds of errors, and this isn't one of them.

Are we talking about FK constraints here, or CHECK contstraints?

Either one. They both have the potential to reference more than one
column, so if the committee had meant errors to try to identify the
referenced columns, they'd have put something other than COLUMN_NAME
into the standard. They didn't.

regards, tom lane

2011/7/18 Tom Lane <tgl@sss.pgh.pa.us>:

Josh Berkus <josh@agliodbs.com> writes:

Tom,

No, I don't.  You're adding complication to solve a problem that doesn't
need to be solved.  The standard says to return the name of the
constraint for a constraint-violation failure.  It does not say anything
about naming the associated column(s).  COLUMN_NAME is only supposed to
be defined for certain kinds of errors, and this isn't one of them.

Are we talking about FK constraints here, or CHECK contstraints?

Either one.  They both have the potential to reference more than one
column, so if the committee had meant errors to try to identify the
referenced columns, they'd have put something other than COLUMN_NAME
into the standard.  They didn't.

Personally, I see a sense for COLUMN_NAME field only with relation to
CHECK_CONSTRAINT - for any other constraint using a COLUMN_NAME is
based on parsing a constraint rule - and I don't believe so the
standard is based in it. Column check constraint is attached
explicitly to one column - but this relation should not be based on
semantic.

We can check DB2 implementation.

Regards
Pavel