subject:"Re\: \[HACKERS\] enhanced error fields"

On 29 December 2012 05:04, Pavel Stehule pavel.steh...@gmail.com wrote:
 again - it is reason why I propose ROUTINE_NAME and TRIGGER_NAME - it
 can be useful for some use cases, where developer should to handle
 exception when they coming from known functions/triggers and he would
 to raise exception, when it was raised elsewhere. Is possible working
 with CONTEXT, but it needs little bit more work and situation is very
 similar to fields COLUMN_NAME, where I can parse message and I can get
 this information. But then it depends on message format.

That seems very thin. Again, I have to point out that a precedent for
this doesn't really appear to exist in any major programming language,
even though ISTM it would be just as useful in a wide variety of
programming environments as it would be within Postgres. As I've said,
the DB2 GET DIAGNOSTIC stuff isn't anything like what you've proposed.

 So I'm with Peter G on this: the existing CONTEXT mechanism is good
 enough, we do not need to split out selected sub-parts of that as
 separate error fields.  Moreover, doing so would provide only an utterly
 arbitrary subset of the context stack: who's to say whether it would be
 more important to report the most closely nested function, or the
 outermost one?

 I don't agree with this argument - you can say too COLUMN_NAME,
 TABLE_NAME is not necessary, because MESSAGE is good enough. I don't
 see any difference - and I would to use these variables for error
 handling (not for logging) without dependency on current format of
 MESSAGE or CONTEXT.

In my judgement, COLUMN_NAME and TABLE_NAME can be used without having
an unreasonable degree of coupling between client and server-side
code. They are at least easily understood, and not at all astonishing,
unlike ROUTINE_NAME. That said, CONSTRAINT_NAME clearly is by far the
most compelling new field, and I actually wouldn't mind losing the
other ones too much. I might have suggested losing COLUMN_NAME and
TABLE_NAME myself if we could reliably get something like a constraint
name for NOT NULL violations.

 Second question - what should be context is important. I am thinking
 so standard and implementation in other databases is relative clean -
 it is name of custom routine, that doesn't handle exception. Moving to
 pg - it is name of routine on the top on error callback stack, because
 builtin functions doesn't set it - I don't need on top of CONTEX -
 div_int or other name of builtin function - but I need there function
 foo(b): a := 10/b.

I don't think the fact that built-in functions don't set ROUTINE_NAME
supports your position. In fact, it seems pretty broken to me that one
pl handler sets the value, while others may not. Furthermore, the
distinction between built-in and not built-in is fairly small within
Postgres - who is to say that even if a person thinks the proposed
semantics are useful, they'll continue to when they find out that
ROUTINE_NAME isn't set to the name of their C function?

 other point - theoretically I can get ROUTINE_NAME from CONTEXT, but I
 cannot get TRIGGER_NAME - this information is lost.

I don't think there should be a TRIGGER_NAME either - I think that we
should make interfaces easy to use correctly, and hard to use
incorrectly, and a mechanised response to a TRIGGER_NAME seems
incorrect. If you think that there should be a trigger name within
CONTEXT, there might be a case to be made for that, but I would prefer
to have that reviewed separately.

-- 
Peter Geoghegan   http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

2012-12-29 Thread Tom Lane

Peter Geoghegan pe...@2ndquadrant.com writes:
 On 29 December 2012 05:04, Pavel Stehule pavel.steh...@gmail.com wrote:
 So I'm with Peter G on this: the existing CONTEXT mechanism is good
 enough, we do not need to split out selected sub-parts of that as
 separate error fields.  Moreover, doing so would provide only an utterly
 arbitrary subset of the context stack: who's to say whether it would be
 more important to report the most closely nested function, or the
 outermost one?

 I don't agree with this argument - you can say too COLUMN_NAME,
 TABLE_NAME is not necessary, because MESSAGE is good enough. I don't
 see any difference - and I would to use these variables for error
 handling (not for logging) without dependency on current format of
 MESSAGE or CONTEXT.

 In my judgement, COLUMN_NAME and TABLE_NAME can be used without having
 an unreasonable degree of coupling between client and server-side
 code.

Yeah, I was about to reply in almost exactly those words.  Table and
column names are, almost by definition, part of the shared understanding
of the client-side and server-side portions of any application, because
both sides have to manipulate those in order to do anything.  However,
if client-side code were to rely on something like ROUTINE_NAME for
error processing, it would become closely tied to the *code structure*
of the server-side functions, which is a bad idea.

Basically the value proposition here is let's contort the backend code
in order to support very bad programming practices in applications.
I'm not attracted by either part of that.

 I don't think there should be a TRIGGER_NAME either - I think that we
 should make interfaces easy to use correctly, and hard to use
 incorrectly, and a mechanised response to a TRIGGER_NAME seems
 incorrect. If you think that there should be a trigger name within
 CONTEXT, there might be a case to be made for that, but I would prefer
 to have that reviewed separately.

If the calling of a trigger isn't visible in the CONTEXT stack then
that's something we should address --- but in practice, wouldn't it be
visible anyway as an ordinary function call?  At least if the trigger
is written in a reasonable PL.  If the trigger is written in C, then
I'm outright against adding any such overhead.  I don't think this patch
should be adding any cycles whatsoever to non-error code paths.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

2012/12/29 Peter Geoghegan pe...@2ndquadrant.com:
 On 29 December 2012 05:04, Pavel Stehule pavel.steh...@gmail.com wrote:
 again - it is reason why I propose ROUTINE_NAME and TRIGGER_NAME - it
 can be useful for some use cases, where developer should to handle
 exception when they coming from known functions/triggers and he would
 to raise exception, when it was raised elsewhere. Is possible working
 with CONTEXT, but it needs little bit more work and situation is very
 similar to fields COLUMN_NAME, where I can parse message and I can get
 this information. But then it depends on message format.

 That seems very thin. Again, I have to point out that a precedent for
 this doesn't really appear to exist in any major programming language,
 even though ISTM it would be just as useful in a wide variety of
 programming environments as it would be within Postgres. As I've said,
 the DB2 GET DIAGNOSTIC stuff isn't anything like what you've proposed.

 So I'm with Peter G on this: the existing CONTEXT mechanism is good
 enough, we do not need to split out selected sub-parts of that as
 separate error fields.  Moreover, doing so would provide only an utterly
 arbitrary subset of the context stack: who's to say whether it would be
 more important to report the most closely nested function, or the
 outermost one?

 I don't agree with this argument - you can say too COLUMN_NAME,
 TABLE_NAME is not necessary, because MESSAGE is good enough. I don't
 see any difference - and I would to use these variables for error
 handling (not for logging) without dependency on current format of
 MESSAGE or CONTEXT.

 In my judgement, COLUMN_NAME and TABLE_NAME can be used without having
 an unreasonable degree of coupling between client and server-side
 code. They are at least easily understood, and not at all astonishing,
 unlike ROUTINE_NAME. That said, CONSTRAINT_NAME clearly is by far the
 most compelling new field, and I actually wouldn't mind losing the
 other ones too much. I might have suggested losing COLUMN_NAME and
 TABLE_NAME myself if we could reliably get something like a constraint
 name for NOT NULL violations.


Maybe this is main difference of our views. In my mind I never
thinking about using ROUTINE_NAME on client side part. My motivation
and reason why I push this feature is a using ROUTINE_NAME inside
PL/pgSQL - or other PL, where it can helps with more precious
exception processing - one real example - handling exception in
business process routines

BEGIN
  ...
  ... business logic
EXCEPTION WHEN OTHERS THEN
  .. all is returned back
GET STACKED DIAGNOSTICS _message = MESSAGE, _routine_name =
ROUTINE_NAME, ...
IF verbosity  'verbose' THEN
INSERT INTO log(...) VALUES(_message, _routine_name)
ELSE
-- log CONTEXT too
INSERT INTO log(...) VALUES(_message, _routine_name, _context);
END IF;
END;

ROUTINE_NAME is information, that has different character from MESSAGE
-- and it is simply (not too detailed) information, where exception
coming from. Usually I don't know what check or builtin function
failed, because it is described by MESSAGE. And I don't would to use
CONTEXT everywhere, because it is too detailed.


 Second question - what should be context is important. I am thinking
 so standard and implementation in other databases is relative clean -
 it is name of custom routine, that doesn't handle exception. Moving to
 pg - it is name of routine on the top on error callback stack, because
 builtin functions doesn't set it - I don't need on top of CONTEX -
 div_int or other name of builtin function - but I need there function
 foo(b): a := 10/b.

 I don't think the fact that built-in functions don't set ROUTINE_NAME
 supports your position. In fact, it seems pretty broken to me that one
 pl handler sets the value, while others may not. Furthermore, the
 distinction between built-in and not built-in is fairly small within
 Postgres - who is to say that even if a person thinks the proposed
 semantics are useful, they'll continue to when they find out that
 ROUTINE_NAME isn't set to the name of their C function?

If C function sets error callback then it can fill ROUTINE_NAME.

yes, now errors callback is used only for PL languages - probably it
is more intuitive design than explicit design, but it is working. It
is different design than in generic languages, where you can track
exception to really last point. But I don't think so it is necessary
in PL - typical customer is not interested in PostgreSQL internals.


 other point - theoretically I can get ROUTINE_NAME from CONTEXT, but I
 cannot get TRIGGER_NAME - this information is lost.

 I don't think there should be a TRIGGER_NAME either - I think that we
 should make interfaces easy to use correctly, and hard to use
 incorrectly, and a mechanised response to a TRIGGER_NAME seems
 incorrect. If you think that there should be a trigger name within
 CONTEXT, there might be a case to be made for that, but I would prefer

Re: [HACKERS] enhanced error fields

2012/12/29 Tom Lane t...@sss.pgh.pa.us:
 Peter Geoghegan pe...@2ndquadrant.com writes:
 On 29 December 2012 05:04, Pavel Stehule pavel.steh...@gmail.com wrote:
 So I'm with Peter G on this: the existing CONTEXT mechanism is good
 enough, we do not need to split out selected sub-parts of that as
 separate error fields.  Moreover, doing so would provide only an utterly
 arbitrary subset of the context stack: who's to say whether it would be
 more important to report the most closely nested function, or the
 outermost one?

 I don't agree with this argument - you can say too COLUMN_NAME,
 TABLE_NAME is not necessary, because MESSAGE is good enough. I don't
 see any difference - and I would to use these variables for error
 handling (not for logging) without dependency on current format of
 MESSAGE or CONTEXT.

 In my judgement, COLUMN_NAME and TABLE_NAME can be used without having
 an unreasonable degree of coupling between client and server-side
 code.

 Yeah, I was about to reply in almost exactly those words.  Table and
 column names are, almost by definition, part of the shared understanding
 of the client-side and server-side portions of any application, because
 both sides have to manipulate those in order to do anything.  However,
 if client-side code were to rely on something like ROUTINE_NAME for
 error processing, it would become closely tied to the *code structure*
 of the server-side functions, which is a bad idea.

 Basically the value proposition here is let's contort the backend code
 in order to support very bad programming practices in applications.
 I'm not attracted by either part of that.

I don't think so it is necessary bad practice.

first - my motivation is related primary for usage in PL/pgSQL.

we can handle exceptions very near to place where exception was
raised. And then I can take same information like ROUTINE_NAME. Later
I can forward exception. A disadvantage of this design is higher
number of subtransactions.

Other design, I talked about it - is based on one relative high
positioned subtransaction, where I can handle lot of types of
exceptions or can raise final exception. I can do same work as in
previous mentioned design but just with one subtransaction. But for
design I need a data. And if is possible in simple form - because it
is better for PL/pgSQL.


 I don't think there should be a TRIGGER_NAME either - I think that we
 should make interfaces easy to use correctly, and hard to use
 incorrectly, and a mechanised response to a TRIGGER_NAME seems
 incorrect. If you think that there should be a trigger name within
 CONTEXT, there might be a case to be made for that, but I would prefer
 to have that reviewed separately.

 If the calling of a trigger isn't visible in the CONTEXT stack then
 that's something we should address --- but in practice, wouldn't it be
 visible anyway as an ordinary function call?  At least if the trigger
 is written in a reasonable PL.  If the trigger is written in C, then
 I'm outright against adding any such overhead.  I don't think this patch
 should be adding any cycles whatsoever to non-error code paths.


visibility in CONTEXT depends on creating and registering error
callbacks - I don't would to change it.

Regards

Pavel
 regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

* Peter Geoghegan (pe...@2ndquadrant.com) wrote:
 On 28 December 2012 15:57, Tom Lane t...@sss.pgh.pa.us wrote:
  I don't think that part's been agreed to at all; it seems entirely
  possible that it'll get dropped if/when the patch gets committed.
  I'm not convinced that having these fields in the log is worth
  the compatibility hit of changing the CSV column set.
 
 I am willing to let that go, because it just doesn't seem important to
 me. I just thought that if we were going to break compatibility, we
 might as well not hold back on the inclusion of fields. I'm not sure
 where this leaves the regular log. Pavel wants to remove that, too. I
 defer to others.

I'd like to see more options for what is logged, as I've mentioned in
the past, and I think all of these would be good candidates for logging
in some circumstances.  The insistence on having one CSV format to rule
them all and which doesn't change (except that we've been regularly
changing it across major releases anyway..) doesn't strike me as the
right approach.

 Now, as to the question of whether we need to make sure that
 everything is always fully qualified - I respectfully disagree with
 Stephen, and maintain my position set out in the last round of
 feedback [1], which is that we don't need to fully qualify everything,
 because *for the purposes of error handling*, which is what I think we
 should care about, these fields are sufficient:

It's not entirely clear to me what distinction you're making here or if
we're simply in agreement about what the necessary fields are.
Having the schema name, table name, column name and constraint name
seems like it's sufficient to fully qualify a specific constraint..?
Where I see this being useful would be something along these lines:

COMMENT ON my_constraint ON my_schema.my_table
IS 'Please update XYZ first.';

Application runs, gets back a constraint violation, then:

SELECT
  description,
  consrc
FROM pg_description a
JOIN pg_constraint b ON (a.objid = b.oid)
JOIN pg_namespace c ON (b.connamespace = c.oid)
JOIN pg_class d ON (b.connrelid = d.oid)
WHERE classoid =
(select oid from pg_class x
join pg_namespace y on (x.relnamespace = y.oid)
where y.nspname = 'pg_catalog'
  and x.relname = 'pg_constraint')
  AND c.nspname = 'my_schema'
  AND d.relname = 'my_table'
;

and have that information available to return to the client.

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] enhanced error fields

On 29 December 2012 17:43, Stephen Frost sfr...@snowman.net wrote:
 I'd like to see more options for what is logged, as I've mentioned in
 the past, and I think all of these would be good candidates for logging
 in some circumstances.  The insistence on having one CSV format to rule
 them all and which doesn't change (except that we've been regularly
 changing it across major releases anyway..) doesn't strike me as the
 right approach.

Yeah, you're probably right about that. I'm just not sure where it
leaves this patch.

 It's not entirely clear to me what distinction you're making here or if
 we're simply in agreement about what the necessary fields are.

I think we might be in agreement.

 Having the schema name, table name, column name and constraint name
 seems like it's sufficient to fully qualify a specific constraint..?

Not necessarily, strictly speaking. It's my position that we should
not care about the theoretical edge-cases that this presents. For
example, I don't have any sympathy for the idea that we need to fully
qualify that we're talking about a constraint in a particular schema,
in case there are two distinct constraints in different schemas *that
have the same name but don't do exactly the same thing anyway*. In
cases where there are two distinct constraints with the same name in
the same code path, it seems very likely that the custom error message
to be displayed (or whatever) should not need to differ for each (this
could come up if you were using schemas for multi-tenancy, for
example, and each schema contained the same objects).

So, in my latest revision of the patch, the only thing that isn't
fully-qualified is constraint_name (because routine_name and so on are
no longer included). It just seems unnecessary, given that only
ERRCODE_CHECK_VIOLATION errors will lack a schema name and table name
(and even then, only sometimes). Pavel originally included a
constraint_schema field, because he figured that the way constraints
are catalogued necessitated such a field.

-- 
Peter Geoghegan   http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

2012/12/29 Peter Geoghegan pe...@2ndquadrant.com:
 On 29 December 2012 17:43, Stephen Frost sfr...@snowman.net wrote:
 I'd like to see more options for what is logged, as I've mentioned in
 the past, and I think all of these would be good candidates for logging
 in some circumstances.  The insistence on having one CSV format to rule
 them all and which doesn't change (except that we've been regularly
 changing it across major releases anyway..) doesn't strike me as the
 right approach.

 Yeah, you're probably right about that. I'm just not sure where it
 leaves this patch.

 It's not entirely clear to me what distinction you're making here or if
 we're simply in agreement about what the necessary fields are.

 I think we might be in agreement.

 Having the schema name, table name, column name and constraint name
 seems like it's sufficient to fully qualify a specific constraint..?

 Not necessarily, strictly speaking. It's my position that we should
 not care about the theoretical edge-cases that this presents. For
 example, I don't have any sympathy for the idea that we need to fully
 qualify that we're talking about a constraint in a particular schema,
 in case there are two distinct constraints in different schemas *that
 have the same name but don't do exactly the same thing anyway*. In
 cases where there are two distinct constraints with the same name in
 the same code path, it seems very likely that the custom error message
 to be displayed (or whatever) should not need to differ for each (this
 could come up if you were using schemas for multi-tenancy, for
 example, and each schema contained the same objects).

 So, in my latest revision of the patch, the only thing that isn't
 fully-qualified is constraint_name (because routine_name and so on are
 no longer included). It just seems unnecessary, given that only
 ERRCODE_CHECK_VIOLATION errors will lack a schema name and table name
 (and even then, only sometimes). Pavel originally included a
 constraint_schema field, because he figured that the way constraints
 are catalogued necessitated such a field.

Design of constraints is little bit different between ANSI SQL and
PostgreSQL. And then some fields proposed by standard has no sense in
pg - TRIGGER_SCHEMA and probably CONSTRAINT_SCHEMA. Ours constraints
are related to some relation - everywhere. You cannot create
constraint without relation - so everywhere where CONSTRAINT_NAME is
not empty, then TABLE_NAME and TABLE_SCHEMA should be defined or
CONSTRAINT_NAME should be unique in database. In my first patch used
for these field some expected generated value, but I agree, so it is
not necessary and better to drop it, although it can help with
readability of some queries.

Regards

Pavel


 --
 Peter Geoghegan   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training and Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

Peter,

* Peter Geoghegan (pe...@2ndquadrant.com) wrote:
 Pavel originally included a
 constraint_schema field, because he figured that the way constraints
 are catalogued necessitated such a field.

That's exactly what I was getting at also- in order to do a lookup in
the catalog, you need to know certain information to avoid potentially
getting multiple records back (which would be an error...).

If it isn't possible to uniquely identify a constraint using the
information returned, that's a problem.  It's not a question about what
information is necessary- the catalog dictates what the definition of a
constraint is and it includes schema, table, and costraint name.  In my
view, we shouldn't be returing a constraint name without returning the
other information to identify that constraint.

How is that different from the schema_name and table_name fields that
you mentioned were going to be included..?  Are they not always filled
out for constraint violations and if not, why not?  Is there a situation
where we could be getting an error back about a constraint where the
table returned isn't the table which the constraint is on?

If you review the use-case from my last email, I'm hopeful that you'll
see and understand where I think returning this information would be
useful and the information which would be necessary to make that query
work.  Even without looking up the comment and instead just trying to
return the source of the constraint which was violated, you need the
information whcih will allow you to uniquely identify the constraint.

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] enhanced error fields

On 29 December 2012 18:37, Stephen Frost sfr...@snowman.net wrote:
 That's exactly what I was getting at also- in order to do a lookup in
 the catalog, you need to know certain information to avoid potentially
 getting multiple records back (which would be an error...).

Well, Pavel said that since a constraint is necessarily associated
with another object, the constraint name doesn't need to be separately
qualified. That isn't quite the truth, but I think it's close enough.

Note that I've documented a new set of requirements for various errcodes:

  Section: Class 23 - Integrity Constraint Violation
! Requirement: unused
  23000EERRCODE_INTEGRITY_CONSTRAINT_VIOLATION
integrity_constraint_violation
+ Requirement: unused
  23001EERRCODE_RESTRICT_VIOLATION
restrict_violation
+ # Note that requirements for ERRCODE_NOT_NULL do not apply to domains:
+ Requirement: schema_name, table_name
  23502EERRCODE_NOT_NULL_VIOLATION
not_null_violation
+ Requirement: schema_name, table_name, constraint_name
  23503EERRCODE_FOREIGN_KEY_VIOLATION
foreign_key_violation
+ Requirement: schema_name, table_name, constraint_name
  23505EERRCODE_UNIQUE_VIOLATION
unique_violation
+ Requirement: constraint_name
  23514EERRCODE_CHECK_VIOLATION
check_violation
+ Requirement: schema_name, table_name, constraint_name
  23P01EERRCODE_EXCLUSION_VIOLATION
exclusion_violation

So, unless someone adds a constraint name outside of these errcodes (I
doubt that would make sense), there is exactly one case where a
constraint_name could not have a schema_name (which, as I've said, is
almost the same thing as constraint_schema, the exception being when
referencing FKs on *other* tables are involved) - that case is
ERRCODE_CHECK_VIOLATION.

That's because this SQL could cause ERRCODE_CHECK_VIOLATION:

select '123'::upc_barcode;

What should schema_name be set to now? Surely not the schema of the
type upc_barcode, since that would be inconsistent with a few other
ERRCODE_CHECK_VIOLATION sites where we do know schema_name +
table_name (those two are always either available together or not at
all).

The bottom line is that I'm not promising that you can reliably look
up the constraint, and I don't think that that should be a blocker, or
even that it's all that important. You could do it reliably with the
schema_name + table_name, though I'm not strongly encouraging that you
do.

So I guess we disagree on that, though I'm not *that* strongly opposed
to adding back in a constraint_schema field if the extra code is
deemed worth it.

Does anyone else have an opinion? Tom?

-- 
Peter Geoghegan   http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

2012/12/29 Peter Geoghegan pe...@2ndquadrant.com:
 On 29 December 2012 18:37, Stephen Frost sfr...@snowman.net wrote:
 That's exactly what I was getting at also- in order to do a lookup in
 the catalog, you need to know certain information to avoid potentially
 getting multiple records back (which would be an error...).

 Well, Pavel said that since a constraint is necessarily associated
 with another object, the constraint name doesn't need to be separately
 qualified. That isn't quite the truth, but I think it's close enough.

 Note that I've documented a new set of requirements for various errcodes:

   Section: Class 23 - Integrity Constraint Violation
 ! Requirement: unused
   23000EERRCODE_INTEGRITY_CONSTRAINT_VIOLATION
 integrity_constraint_violation
 + Requirement: unused
   23001EERRCODE_RESTRICT_VIOLATION
 restrict_violation
 + # Note that requirements for ERRCODE_NOT_NULL do not apply to domains:
 + Requirement: schema_name, table_name
   23502EERRCODE_NOT_NULL_VIOLATION
 not_null_violation
 + Requirement: schema_name, table_name, constraint_name
   23503EERRCODE_FOREIGN_KEY_VIOLATION
 foreign_key_violation
 + Requirement: schema_name, table_name, constraint_name
   23505EERRCODE_UNIQUE_VIOLATION
 unique_violation
 + Requirement: constraint_name
   23514EERRCODE_CHECK_VIOLATION
 check_violation
 + Requirement: schema_name, table_name, constraint_name
   23P01EERRCODE_EXCLUSION_VIOLATION
 exclusion_violation

 So, unless someone adds a constraint name outside of these errcodes (I
 doubt that would make sense), there is exactly one case where a
 constraint_name could not have a schema_name (which, as I've said, is
 almost the same thing as constraint_schema, the exception being when
 referencing FKs on *other* tables are involved) - that case is
 ERRCODE_CHECK_VIOLATION.

 That's because this SQL could cause ERRCODE_CHECK_VIOLATION:

 select '123'::upc_barcode;

 What should schema_name be set to now? Surely not the schema of the
 type upc_barcode, since that would be inconsistent with a few other
 ERRCODE_CHECK_VIOLATION sites where we do know schema_name +
 table_name (those two are always either available together or not at
 all).

I forgot on domain :(

this is use case, where CONSTRAINT_SCHEMA has sense


 The bottom line is that I'm not promising that you can reliably look
 up the constraint, and I don't think that that should be a blocker, or
 even that it's all that important. You could do it reliably with the
 schema_name + table_name, though I'm not strongly encouraging that you
 do.

so then we probably need a CONSTRAINT_SCHEMA


 So I guess we disagree on that, though I'm not *that* strongly opposed
 to adding back in a constraint_schema field if the extra code is
 deemed worth it.

 Does anyone else have an opinion? Tom?



 --
 Peter Geoghegan   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training and Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

* Peter Geoghegan (pe...@2ndquadrant.com) wrote:
 So, unless someone adds a constraint name outside of these errcodes (I
 doubt that would make sense), there is exactly one case where a
 constraint_name could not have a schema_name (which, as I've said, is
 almost the same thing as constraint_schema, the exception being when
 referencing FKs on *other* tables are involved)

To be honest, I expected the concern to be about FKs and RESTRICT-type
relationships, which I think we do need to figure out an answer for.  Is
there a distinction between the errors thrown for violating an FK on an
insert vs. violating a FK on a delete?  Perhaps with that we could
identify referring table vs referred table and provide all of that
information to the application in a structured way?

 - that case is
 ERRCODE_CHECK_VIOLATION.
 
 That's because this SQL could cause ERRCODE_CHECK_VIOLATION:
 
 select '123'::upc_barcode;

I'm surprised to see that as a constraint violation rather than a domain
violation..?  ala:

=* select '30'::int;
ERROR:  value 30 is out of range for type integer

 What should schema_name be set to now? Surely not the schema of the
 type upc_barcode, since that would be inconsistent with a few other
 ERRCODE_CHECK_VIOLATION sites where we do know schema_name +
 table_name (those two are always either available together or not at
 all).

I'm not sure that the schema of the type would be entirely wrong in that
specific case, along with the table name being set to the name of the
domain.  I still think a domain violation-type error would be more
appropriate than calling it a constraint violation though.

 The bottom line is that I'm not promising that you can reliably look
 up the constraint, and I don't think that that should be a blocker, or
 even that it's all that important. You could do it reliably with the
 schema_name + table_name, though I'm not strongly encouraging that you
 do.
 
 So I guess we disagree on that, though I'm not *that* strongly opposed
 to adding back in a constraint_schema field if the extra code is
 deemed worth it.
 
 Does anyone else have an opinion? Tom?

Having just constraint_schema and constraint_name feels horribly wrong
as the definition of a constraint also includes a pg_class oid.

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] enhanced error fields

2012/12/29 Stephen Frost sfr...@snowman.net:
 * Peter Geoghegan (pe...@2ndquadrant.com) wrote:
 So, unless someone adds a constraint name outside of these errcodes (I
 doubt that would make sense), there is exactly one case where a
 constraint_name could not have a schema_name (which, as I've said, is
 almost the same thing as constraint_schema, the exception being when
 referencing FKs on *other* tables are involved)

 To be honest, I expected the concern to be about FKs and RESTRICT-type
 relationships, which I think we do need to figure out an answer for.  Is
 there a distinction between the errors thrown for violating an FK on an
 insert vs. violating a FK on a delete?  Perhaps with that we could
 identify referring table vs referred table and provide all of that
 information to the application in a structured way?

 - that case is
 ERRCODE_CHECK_VIOLATION.

 That's because this SQL could cause ERRCODE_CHECK_VIOLATION:

 select '123'::upc_barcode;

 I'm surprised to see that as a constraint violation rather than a domain
 violation..?  ala:

 =* select '30'::int;
 ERROR:  value 30 is out of range for type integer

 What should schema_name be set to now? Surely not the schema of the
 type upc_barcode, since that would be inconsistent with a few other
 ERRCODE_CHECK_VIOLATION sites where we do know schema_name +
 table_name (those two are always either available together or not at
 all).

 I'm not sure that the schema of the type would be entirely wrong in that
 specific case, along with the table name being set to the name of the
 domain.  I still think a domain violation-type error would be more
 appropriate than calling it a constraint violation though.

 The bottom line is that I'm not promising that you can reliably look
 up the constraint, and I don't think that that should be a blocker, or
 even that it's all that important. You could do it reliably with the
 schema_name + table_name, though I'm not strongly encouraging that you
 do.

 So I guess we disagree on that, though I'm not *that* strongly opposed
 to adding back in a constraint_schema field if the extra code is
 deemed worth it.

 Does anyone else have an opinion? Tom?

 Having just constraint_schema and constraint_name feels horribly wrong
 as the definition of a constraint also includes a pg_class oid.

but then TABLE_NAME and TABLE_SCHEMA will be defined.

Pavel


 Thanks,

 Stephen


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

* Pavel Stehule (pavel.steh...@gmail.com) wrote:
  Having just constraint_schema and constraint_name feels horribly wrong
  as the definition of a constraint also includes a pg_class oid.
 
 but then TABLE_NAME and TABLE_SCHEMA will be defined.

How are you going to look up the constraint?  Using constraint_schema,
table_name, and constraint_name?  Or table_schema, table_name and
constraint_name?  When do you use constraint_schema instead of
table_schema?

None of those options is exactly clear or understandable...

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] enhanced error fields

2012/12/29 Stephen Frost sfr...@snowman.net:
 * Pavel Stehule (pavel.steh...@gmail.com) wrote:
  Having just constraint_schema and constraint_name feels horribly wrong
  as the definition of a constraint also includes a pg_class oid.

 but then TABLE_NAME and TABLE_SCHEMA will be defined.

 How are you going to look up the constraint?  Using constraint_schema,
 table_name, and constraint_name?  Or table_schema, table_name and
 constraint_name?  When do you use constraint_schema instead of
 table_schema?

 None of those options is exactly clear or understandable...

probably there will be situation when TABLE_SCHEMA and
CONSTRAINT_SCHEMA same values

Hypothetically - if we define CONSTRAINT_TABLE - what is difference
from TABLE_NAME ?

Pavel




 Thanks,

 Stephen


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

On 29 December 2012 19:56, Stephen Frost sfr...@snowman.net wrote:
 - that case is
 ERRCODE_CHECK_VIOLATION.

 That's because this SQL could cause ERRCODE_CHECK_VIOLATION:

 select '123'::upc_barcode;

 I'm surprised to see that as a constraint violation rather than a domain
 violation..?

I was trying to convey that upc_barcode is a domain with a check
constraint (i.e. that the checkdigit on UPC barcode domains must be
correct). So yes, that would be an ERRCODE_CHECK_VIOLATION. See
ExecEvalCoerceToDomain().

 What should schema_name be set to now? Surely not the schema of the
 type upc_barcode, since that would be inconsistent with a few other
 ERRCODE_CHECK_VIOLATION sites where we do know schema_name +
 table_name (those two are always either available together or not at
 all).

 I'm not sure that the schema of the type would be entirely wrong in that
 specific case, along with the table name being set to the name of the
 domain.  I still think a domain violation-type error would be more
 appropriate than calling it a constraint violation though.

Well, it is what it is. We can't change it now.

 Having just constraint_schema and constraint_name feels horribly wrong
 as the definition of a constraint also includes a pg_class oid.

I think that it's probably sufficient *for error handling purposes*.
Since it is non-trivial to get the schema of a constraint, and since
we have that jarring inconsistency (the schema of the type or the
schema of the table on which a check constraint is defined?) we might
well be better off just not addressing it.

It isn't as simple as you make out. Not all constraints appear within
pg_constraint (consider NOT NULL constraints), and besides,
pg_constraint.conrelid can be zero for non-table constraints. What's
more, pg_constraint actually has three pg_class oid columns; conrelid,
conindid and confrelid.

That is all.

-- 
Peter Geoghegan   http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

* Peter Geoghegan (pe...@2ndquadrant.com) wrote:
  Having just constraint_schema and constraint_name feels horribly wrong
  as the definition of a constraint also includes a pg_class oid.
 
 I think that it's probably sufficient *for error handling purposes*.
 Since it is non-trivial to get the schema of a constraint, and since
 we have that jarring inconsistency (the schema of the type or the
 schema of the table on which a check constraint is defined?) we might
 well be better off just not addressing it.

I actually really like the idea of being able to programatically figure
out what constraint caused a given error and then go look up that
constraint definition and the comment associated with it, to be able to
pass back a meaningful error to the user.  Reducing the cases where
users end up implementing their own application-level error checking
before sending things to the DB is a worthwhile goal, as they often end
up implementing slightly different checking that what the DB does and
get upset when the DB throws an error that they can't do anything useful
with.

 It isn't as simple as you make out. Not all constraints appear within
 pg_constraint (consider NOT NULL constraints), and besides,
 pg_constraint.conrelid can be zero for non-table constraints. What's
 more, pg_constraint actually has three pg_class oid columns; conrelid,
 conindid and confrelid.

Perhaps we can provide a bit more help to our application developers
then by coming up with something which will work consistently- eg: we
provide the data in a structured way to the client and then a function
(or a few of them) which the application can then use to get the
details.  I understand that it's complicated and I'd hope that we can do
something better.  In general, I wouldn't recommend developers to query
the catalogs directly, but I don't think there's really a better option
currently.  If we fix that, great.

In the end, I may agree with you- the patch is a nice idea, but it needs
more to be a complete and working solution and providing something that
only gets people half-way there would result in developers hacking
things together which will quitely break when they least expect it.

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] enhanced error fields

2012/12/29 Stephen Frost sfr...@snowman.net:
 * Peter Geoghegan (pe...@2ndquadrant.com) wrote:
  Having just constraint_schema and constraint_name feels horribly wrong
  as the definition of a constraint also includes a pg_class oid.

 I think that it's probably sufficient *for error handling purposes*.
 Since it is non-trivial to get the schema of a constraint, and since
 we have that jarring inconsistency (the schema of the type or the
 schema of the table on which a check constraint is defined?) we might
 well be better off just not addressing it.

 I actually really like the idea of being able to programatically figure
 out what constraint caused a given error and then go look up that
 constraint definition and the comment associated with it, to be able to
 pass back a meaningful error to the user.  Reducing the cases where
 users end up implementing their own application-level error checking
 before sending things to the DB is a worthwhile goal, as they often end
 up implementing slightly different checking that what the DB does and
 get upset when the DB throws an error that they can't do anything useful
 with.

 It isn't as simple as you make out. Not all constraints appear within
 pg_constraint (consider NOT NULL constraints), and besides,
 pg_constraint.conrelid can be zero for non-table constraints. What's
 more, pg_constraint actually has three pg_class oid columns; conrelid,
 conindid and confrelid.

 Perhaps we can provide a bit more help to our application developers
 then by coming up with something which will work consistently- eg: we
 provide the data in a structured way to the client and then a function
 (or a few of them) which the application can then use to get the
 details.  I understand that it's complicated and I'd hope that we can do
 something better.  In general, I wouldn't recommend developers to query
 the catalogs directly, but I don't think there's really a better option
 currently.  If we fix that, great.

 In the end, I may agree with you- the patch is a nice idea, but it needs
 more to be a complete and working solution and providing something that
 only gets people half-way there would result in developers hacking
 things together which will quitely break when they least expect it.

it is a problem of this patch or not consistent constraints implementation ?

Regards

Pavel


 Thanks,

 Stephen


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

* Pavel Stehule (pavel.steh...@gmail.com) wrote:
 it is a problem of this patch or not consistent constraints implementation ?

Not sure, but I don't think it matters.  You can blame the constraint
implementation, but that doesn't change my feelings about what we need
before we can accept a patch like this.  Providing something which works
only part of the time and then doesn't work for very unclear reasons
isn't a good idea.  Perhaps we need to fix the constraint implementation
and perhaps we need to fix the error information being returned, or most
likely we have to fix both, it doesn't change that we need to do
something more than just ignore this problem.

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] enhanced error fields

2012/12/29 Stephen Frost sfr...@snowman.net:
 * Pavel Stehule (pavel.steh...@gmail.com) wrote:
 it is a problem of this patch or not consistent constraints implementation ?

 Not sure, but I don't think it matters.  You can blame the constraint
 implementation, but that doesn't change my feelings about what we need
 before we can accept a patch like this.  Providing something which works
 only part of the time and then doesn't work for very unclear reasons
 isn't a good idea.  Perhaps we need to fix the constraint implementation
 and perhaps we need to fix the error information being returned, or most
 likely we have to fix both, it doesn't change that we need to do
 something more than just ignore this problem.

can we remove CONSTRAINT_NAME from this patch? Minimally TABLE_SCHEMA,
TABLE_NAME and COLUMN_NAME works as expected.

CONSTRAINT_NAME can be implemented after constraints refactoring

Pavel


 Thanks,

 Stephen


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

On 29 December 2012 20:49, Stephen Frost sfr...@snowman.net wrote:
 In the end, I may agree with you- the patch is a nice idea, but it needs
 more to be a complete and working solution and providing something that
 only gets people half-way there would result in developers hacking
 things together which will quitely break when they least expect it.

I certainly wouldn't go that far. I think that 95% of the value that
could be delivered by this sort of thing will be realised by
committing something that is along the lines of what we have here
already. All I really want is to give users a well-principled way of
getting a constraint name within their error handler, so that they can
go do something in their application along the lines of displaying a
custom error message in the domain of the application and/or error
handler, not the domain of the database. That's it.

Ascertaining the identity of the object in question perfectly
unambiguously, so that you can safely do something like lookup a
comment on the object, seems like something way beyond what I'd
envisioned for this feature. Why should the comment be useful in an
error handler anyway? At best, that seems like a nice-to-have extra to
me. The vast majority are not even going to think about the ambiguity
that may exist. They'll just write:

if (constraint_name == upc)
MessageBox(That is not a valid barcode.);

If it isn't the same constraint object as expected - if they have
multiple schemas with the same object definitions, are they likely to
care? We'll never promise that constraint_name is unambiguous, and in
fact will warn that it may be ambiguous.

It seems like what you're really looking for is a way of effectively
changing the error message to a user-defined error message using some
kind of DDL against a constraint object, without doing anything
special in a handler. That would be less flexible, but more automated,
and would probably entail using placeholders in our custom message
that get expanded (as with FK constraint violations - *what* key was
violated?). That has its appeal, but I don't see that it's something
that we need to do first.

Don't get me wrong - I think it's quite possible to pin down a way of
unambiguously identifying constraint objects as they appear here. I
just think that it isn't worth the effort to maintain the code in
Postgres, and in particular, I think people will not care about any
ambiguity, and even if they do, they could easily misunderstand the
exact rules. The right thing to do is not have multiple constraints
with the same name in flight within the same place that do different
things.

-- 
Peter Geoghegan   http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

On 29 December 2012 21:24, Pavel Stehule pavel.steh...@gmail.com wrote:
 can we remove CONSTRAINT_NAME from this patch? Minimally TABLE_SCHEMA,
 TABLE_NAME and COLUMN_NAME works as expected.

 CONSTRAINT_NAME can be implemented after constraints refactoring

This patch is almost completely pointless without a CONSTRAINT_NAME field.

-- 
Peter Geoghegan   http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

Peter,

* Peter Geoghegan (pe...@2ndquadrant.com) wrote:
 if (constraint_name == upc)
 MessageBox(That is not a valid barcode.);

So they'll quickly realize that a lookup-table based on constraint name
would be useful, create it, and then have a primary key on it to make
sure that they don't have any duplicates.  Of course, this is all
duplicating what we're *already* keeping track of, except that method
won't be consistent with the catalog and they could end up with a single
entry in their table which corresponds to multiple actual constraints in
the system and begin subtly returning errors that don't make any sense
to the end user.

That's exactly the kind of subtly broken situation that I would hope
we'd try to keep them from getting into.

I'd almost rather return the OID and provide some lookup functions.

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] enhanced error fields

On 29 December 2012 22:57, Stephen Frost sfr...@snowman.net wrote:
 So they'll quickly realize that a lookup-table based on constraint name
 would be useful, create it, and then have a primary key on it to make
 sure that they don't have any duplicates.

I don't find that terribly likely. There is nothing broken about the
example. It's possible to misuse almost anything.

In order for the problem you describe to happen, the user would have
to ignore the warning in the documentation about constraint_name's
ability to uniquely identify something, and then have two constraints
in play at the same time with the same name but substantively
different. That seems incredibly unlikely.

Maybe you think that users cannot be trusted to take that warning on
board, but then the same user could not be trusted to heed another
warning about using a constraint_schema in the lookup table primary
key.

This whole lookup table idea presupposes that there'll only ever be
one error message per constraint in the entire application. That
usually isn't true for all sorts of reasons, in my experience.

-- 
Peter Geoghegan   http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

Peter,

* Peter Geoghegan (pe...@2ndquadrant.com) wrote:
 In order for the problem you describe to happen, the user would have
 to ignore the warning in the documentation about constraint_name's
 ability to uniquely identify something, and then have two constraints
 in play at the same time with the same name but substantively
 different. That seems incredibly unlikely.

I really don't think what I sketched out or something similar would
happen.  I do think it's incredibly frustrating as a user who is trying
to develop an application which behaves correctly to be given only half
the information.

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] enhanced error fields

On 30 December 2012 02:01, Stephen Frost sfr...@snowman.net wrote:
 I really don't think what I sketched out or something similar would
 happen.  I do think it's incredibly frustrating as a user who is trying
 to develop an application which behaves correctly to be given only half
 the information.

I don't know what to say to that. Sometimes, when deciding how to
address problems like this, it's possible to arrive at slightly
surprising answers by process of elimination.

-- 
Peter Geoghegan   http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

* Peter Geoghegan (pe...@2ndquadrant.com) wrote:
 On 30 December 2012 02:01, Stephen Frost sfr...@snowman.net wrote:
  I really don't think what I sketched out or something similar would
  happen.  I do think it's incredibly frustrating as a user who is trying
  to develop an application which behaves correctly to be given only half
  the information.
 
 I don't know what to say to that. Sometimes, when deciding how to
 address problems like this, it's possible to arrive at slightly
 surprising answers by process of elimination.

Err.  I intended to say I really don't think what I sketched out, or
something similar, would be that unlikely to happen, or something along
those lines.  Apologies for the confusion.

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] enhanced error fields

On 30 December 2012 03:32, Stephen Frost sfr...@snowman.net wrote:
 Err.  I intended to say I really don't think what I sketched out, or
 something similar, would be that unlikely to happen, or something along
 those lines.  Apologies for the confusion.

Almost anything can be misused.

If you're going to insist that I hack a bunch of mechanism into this
patch so that the user can unambiguously identify each constraint
object, I'll do that. However, that's more code, and more complexity,
that will have to be documented, for just next to no practical benefit
that I can see.

-- 
Peter Geoghegan   http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

* Peter Geoghegan (pe...@2ndquadrant.com) wrote:
 On 30 December 2012 03:32, Stephen Frost sfr...@snowman.net wrote:
  Err.  I intended to say I really don't think what I sketched out, or
  something similar, would be that unlikely to happen, or something along
  those lines.  Apologies for the confusion.
 
 Almost anything can be misused.

I agree, almost anything can be.  The 'misuse' in this case, however, is
in expecting the information returned to be useful in a deterministic
and consistent manner, as it's being returned in a programmatic fashion.

 If you're going to insist that I hack a bunch of mechanism into this
 patch so that the user can unambiguously identify each constraint
 object, I'll do that.

This is the part that I'm having trouble wrapping my head around- what's
the additional complexity here?  If we have the OID of the constraint,
we should be able to unambiguoulsy return information which allows a
user to get back to that specific constraint and OID.

 However, that's more code, and more complexity,
 that will have to be documented, for just next to no practical benefit
 that I can see.

I've certainly seen cases where constraints are duplicated by name
across schemas.  I would imagine it could happen across tables in the
same schema also.  I just don't like this notion of returning something
ambiguous to the user and worry that they'd accept it as deterministic
and dependable, regardless of the documentation.  There's a certain
amount of read the docs, but also look at what information you really
get back, which I think is, in general, a good thing, but it does mean
individuals might come to believe that they can depend on the constraint
name being unique in all cases.

As a side-note, I've recently run into more cases than I care to think
about where the 63-character limit on constraint names has been a
problem- because we're trying to ensure that we create an unambiguous
constraint name, and that's *with* various name shortening techniques
being used.  The discussion about having longer values possible for the
'name' data type was on a different thread and should probably stay
there, but I bring it up because it has some impact on the possibility
of name collisions which is relevent to this discussion.

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] enhanced error fields

2012/12/30 Stephen Frost sfr...@snowman.net:
 * Peter Geoghegan (pe...@2ndquadrant.com) wrote:
 On 30 December 2012 03:32, Stephen Frost sfr...@snowman.net wrote:
  Err.  I intended to say I really don't think what I sketched out, or
  something similar, would be that unlikely to happen, or something along
  those lines.  Apologies for the confusion.

 Almost anything can be misused.

 I agree, almost anything can be.  The 'misuse' in this case, however, is
 in expecting the information returned to be useful in a deterministic
 and consistent manner, as it's being returned in a programmatic fashion.

 If you're going to insist that I hack a bunch of mechanism into this
 patch so that the user can unambiguously identify each constraint
 object, I'll do that.

 This is the part that I'm having trouble wrapping my head around- what's
 the additional complexity here?  If we have the OID of the constraint,
 we should be able to unambiguoulsy return information which allows a
 user to get back to that specific constraint and OID.

 However, that's more code, and more complexity,
 that will have to be documented, for just next to no practical benefit
 that I can see.

 I've certainly seen cases where constraints are duplicated by name
 across schemas.  I would imagine it could happen across tables in the
 same schema also.  I just don't like this notion of returning something
 ambiguous to the user and worry that they'd accept it as deterministic
 and dependable, regardless of the documentation.  There's a certain
 amount of read the docs, but also look at what information you really
 get back, which I think is, in general, a good thing, but it does mean
 individuals might come to believe that they can depend on the constraint
 name being unique in all cases.

 As a side-note, I've recently run into more cases than I care to think
 about where the 63-character limit on constraint names has been a
 problem- because we're trying to ensure that we create an unambiguous
 constraint name, and that's *with* various name shortening techniques
 being used.  The discussion about having longer values possible for the
 'name' data type was on a different thread and should probably stay
 there, but I bring it up because it has some impact on the possibility
 of name collisions which is relevent to this discussion.

so - cannot be a solution define CONSTRAINT_TABLE field - constaint
names in table are unique.

sure there is a problem with long names, but I am thinking so it has
solution - when constraint has no name, then we can try to generate
name, and when this name is longer than 63 chars, then CREATE
STATEMENT fails and users should be define name manually - this
feature should be disabled by guc due compatibility issues.

Regards

Pavel

 Thanks,

 Stephen


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

2012-12-28 Thread Stephen Frost

Pavel, Peter,

To be honest, I haven't been following this thread at all, but I'm kind
of amazed that this patch seems to be progressing.  I spent quite a bit
of time previously trying to change the CSV log structure to add a
single column and this patch is adding a whole slew of them.  My prior
patch also allowed customization of the CSV log output, which seems like
it would be even more valuable if we're going to be adding a bunch more
fields.  I know there are dissenting viewpoints on that, however, as
application authors would prefer to not have to deal with variable CSV
output formats, which I can understand.

As regards this specific patch, I trust that the protocol error response
changes won't have any impact on existing clients?  Do we anticipate
something like the JDBC driver having any issues?  If there's any chance
that we're going to break a client by sending it things which it won't
understand, it seems we'd need to bump the protocol (which hasn't been
done in quite some time and would likely needs its own discussion...).

Regarding qualifying what we're returning- I tend to agree with Pavel
that if we're going to provide this information, we should do it in a
fully qualified manner.  I can certainly see situations where constraint
names are repeated in environments and it may not be clear what schema
it's in (think application development with a dev and a test schema in
the same database), and the same could hold for constraints against
tables (though having those repeated would certainly be more rare, since
you can't repeat a named constraint in a given schema if it has an index
behind it, though you could with simple CHECK constraints).  Again, my
feeling is that if we're going to provide such information, it should be
fully-qualified.

There are some additional concerns regarding the patch itself that I
have (do we really want to modify ereport() in this way?  How can we
implement something which scales better than just adding more and more
parameters?) but I think we need to figure out exactly what we're agreed
to be doing with this patch and get buy-in from everyone first.

Thanks,

Stephen

* Pavel Stehule (pavel.steh...@gmail.com) wrote:
 I rechecked your version eelog4 and I am thinking so it is ok. From my
 view it can be enough for application developer and I am not against
 to commit this (or little bit reduced version) as first step.
 
 As plpgsql developer I really miss a fields routine_name,
 routine_schema and trigger_name. These fields can be simply
 implemented without any impact on performance. Because routine_name
 and routine_schema is not unique in postgres, I propose third field
 routine_oid. My patch eelog5 is based on your eelog4 with enhancing
 for these mentioned fields - 5KB more - but only PL/pgSQL is supported
 now. I would to find a acceptable design now.
 
 Second - I don't see any value for forwarding these new fields
 (column_name, table_name, constraint_name, schema_name, routine_name,
 routine_schema, routine_id, trigger_name) to log or to csvlog and I
 propose don't push it to log. We can drop logging in next iteration if
 you agree.
 
 What do you thinking about new version?
 
 Regards
 
 Pavel
 
 
 
 2012/12/10 Peter Geoghegan pe...@2ndquadrant.com:
  So, I took a look at this, and made some more changes.
 
  I have a hard time seeing the utility of some fields that were in this
  patch, and so they have been removed from this revision.
 
  Consider, for example:
 
  +   constraint_table text,
  +   constraint_schema text,
 
  While constraint_name remains, I find it really hard to believe that
  applications need a perfectly unambiguous idea of what constraint
  they're dealing with. If applications have a constraint that has a
  different domain-specific meaning depending on what schema it is in,
  while a table in one schema uses that constraint in another, well,
  that's a fairly awful design. Is it something that we really need to
  care about? You mentioned the SQL standard, but as far as I know the
  SQL standard has nothing to say about these fields within something
  like errdata - their names are lifted from information_schema, but
  being unambiguous is hardly so critical here. We want to identify an
  object for the purposes of displaying an error message only. Caring
  deeply about ambiguity seems wrong-headed to me in this context.
 
  +   routine_name text,
  +   trigger_name text,
  +   trigger_table text,
  +   trigger_schema text,
 
  The whole point of an exception (which ereport() is very similar to)
  is to decouple errors from error handling as much as possible - any
  application that magically takes a different course of action based on
  where that error occurred as reported by the error itself (and not the
  location of where handling the error occurs) seems kind of
  wrong-headed to me. Sure, a backtrace may be supplied, but that's only
  for diagnostic purposes, and a human can just as easily get that

Re: [HACKERS] enhanced error fields

Hello

2012/12/28 Stephen Frost sfr...@snowman.net:
 Pavel, Peter,

 To be honest, I haven't been following this thread at all, but I'm kind
 of amazed that this patch seems to be progressing.  I spent quite a bit
 of time previously trying to change the CSV log structure to add a
 single column and this patch is adding a whole slew of them.  My prior
 patch also allowed customization of the CSV log output, which seems like
 it would be even more valuable if we're going to be adding a bunch more
 fields.  I know there are dissenting viewpoints on that, however, as
 application authors would prefer to not have to deal with variable CSV
 output formats, which I can understand.

Some smarter design of decision what will be stored to CSV and what
now can be great. I am not thinking so these enhanced fields has value
pro typical DBA and should be stored to CSV only when somebody need
it. But it is different story - although it can simplify our work
because then we don't need to solve this issue.


 As regards this specific patch, I trust that the protocol error response
 changes won't have any impact on existing clients?  Do we anticipate
 something like the JDBC driver having any issues?  If there's any chance
 that we're going to break a client by sending it things which it won't
 understand, it seems we'd need to bump the protocol (which hasn't been
 done in quite some time and would likely needs its own discussion...).


Depends on implementation. Implementations designed similar to libpq
should not have a problems.


 Regarding qualifying what we're returning- I tend to agree with Pavel
 that if we're going to provide this information, we should do it in a
 fully qualified manner.  I can certainly see situations where constraint
 names are repeated in environments and it may not be clear what schema
 it's in (think application development with a dev and a test schema in
 the same database), and the same could hold for constraints against
 tables (though having those repeated would certainly be more rare, since
 you can't repeat a named constraint in a given schema if it has an index
 behind it, though you could with simple CHECK constraints).  Again, my
 feeling is that if we're going to provide such information, it should be
 fully-qualified.

 There are some additional concerns regarding the patch itself that I
 have (do we really want to modify ereport() in this way?  How can we
 implement something which scales better than just adding more and more
 parameters?) but I think we need to figure out exactly what we're agreed
 to be doing with this patch and get buy-in from everyone first.

Related fields are fundamental - and I am thinking so is good to
optimize it - it has semantic tag, and tag has one char size. Some set
of fields is given by ANSI - we can select some subset because not all
fields described by ANSI has sense in pg. I don't would to enhance
range of this patch too much and I don't would to redesign current
concept of error handling. I am thinking so it works well and I have
no negative feedback from PostgreSQL users that I know.

But probably only reserved memory for ErrorData is limit for
serialized custom fields - I believe so we will have a associative
array - and one field can be of this type for any custom fields.

Regards

Pavel


 Thanks,

 Stephen

 * Pavel Stehule (pavel.steh...@gmail.com) wrote:
 I rechecked your version eelog4 and I am thinking so it is ok. From my
 view it can be enough for application developer and I am not against
 to commit this (or little bit reduced version) as first step.

 As plpgsql developer I really miss a fields routine_name,
 routine_schema and trigger_name. These fields can be simply
 implemented without any impact on performance. Because routine_name
 and routine_schema is not unique in postgres, I propose third field
 routine_oid. My patch eelog5 is based on your eelog4 with enhancing
 for these mentioned fields - 5KB more - but only PL/pgSQL is supported
 now. I would to find a acceptable design now.

 Second - I don't see any value for forwarding these new fields
 (column_name, table_name, constraint_name, schema_name, routine_name,
 routine_schema, routine_id, trigger_name) to log or to csvlog and I
 propose don't push it to log. We can drop logging in next iteration if
 you agree.

 What do you thinking about new version?

 Regards

 Pavel



 2012/12/10 Peter Geoghegan pe...@2ndquadrant.com:
  So, I took a look at this, and made some more changes.
 
  I have a hard time seeing the utility of some fields that were in this
  patch, and so they have been removed from this revision.
 
  Consider, for example:
 
  +   constraint_table text,
  +   constraint_schema text,
 
  While constraint_name remains, I find it really hard to believe that
  applications need a perfectly unambiguous idea of what constraint
  they're dealing with. If applications have a constraint that has a
  different domain-specific meaning depending

Re: [HACKERS] enhanced error fields

2012-12-28 Thread Tom Lane

Stephen Frost sfr...@snowman.net writes:
 To be honest, I haven't been following this thread at all, but I'm kind
 of amazed that this patch seems to be progressing.  I spent quite a bit
 of time previously trying to change the CSV log structure to add a
 single column and this patch is adding a whole slew of them.

I don't think that part's been agreed to at all; it seems entirely
possible that it'll get dropped if/when the patch gets committed.
I'm not convinced that having these fields in the log is worth
the compatibility hit of changing the CSV column set.

 As regards this specific patch, I trust that the protocol error response
 changes won't have any impact on existing clients?  Do we anticipate
 something like the JDBC driver having any issues?  If there's any chance
 that we're going to break a client by sending it things which it won't
 understand, it seems we'd need to bump the protocol (which hasn't been
 done in quite some time and would likely needs its own discussion...).

I think we are okay on this.  The protocol definition has said from the
very beginning Since more field types might be added in future,
frontends should silently ignore fields of unrecognized type.  A client
that fails to do that is buggy, not grounds for bumping the protocol.

I haven't been following the patch so have no thoughts about your
other comments.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

On 28 December 2012 14:00, Stephen Frost sfr...@snowman.net wrote:
 There are some additional concerns regarding the patch itself that I
 have (do we really want to modify ereport() in this way?  How can we
 implement something which scales better than just adding more and more
 parameters?) but I think we need to figure out exactly what we're agreed
 to be doing with this patch and get buy-in from everyone first.

I don't think that the need to scale beyond what we have in my
revision really exists. Some of the ereport sites are a bit unwieldy,
but I don't see that there is much that can be done about that - you
need to specify the information somewhere, and it makes sense to do it
at that point. The field names are frequently expanded in the error
message presented to the user anyway.

-- 
Peter Geoghegan   http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

On 28 December 2012 15:57, Tom Lane t...@sss.pgh.pa.us wrote:
I don't think that part's been agreed to at all; it seems entirely
possible that it'll get dropped if/when the patch gets committed.
I'm not convinced that having these fields in the log is worth
the compatibility hit of changing the CSV column set.

I am willing to let that go, because it just doesn't seem important to
me. I just thought that if we were going to break compatibility, we
might as well not hold back on the inclusion of fields. I'm not sure
where this leaves the regular log. Pavel wants to remove that, too. I
defer to others.

Now, as to the question of whether we need to make sure that
everything is always fully qualified - I respectfully disagree with
Stephen, and maintain my position set out in the last round of
feedback [1], which is that we don't need to fully qualify everything,
because *for the purposes of error handling*, which is what I think we
should care about, these fields are sufficient:

+ column_name text,
+ table_name text,
+ constraint_name text,
+ schema_name text,

If you use the same constraint name everywhere, you might get the same
error message. The situations in which this scheme might fall down are
too implausible for me to want to bloat up all those ereport sites any
further (something that Stephen also complained about).

I think that the major outstanding issues are concerning whether or
not I have the API here right. I make explicit guarantees as to the
availability of certain fields for certain errcodes (a small number of
Class 23 - Integrity Constraint Violation codes). No one has really
said anything about that, though I think it's important.

I also continue to think that we shouldn't have routine_name,
routine_schema and trigger_name fields - I think it's wrong-headed
to have an exception handler magically act on knowledge about where
the exception came from that has been baked into the exception - is
there any sort of precedent for this? Pavel disagrees here. Again, I
defer to others.

[1] Post of revision eelog4.diff:
http://archives.postgresql.org/message-id/caeylb_um9z8vitjckaogg2shab1n-71doznhj2pjm2ls96q...@mail.gmail.com
--
Peter Geoghegan http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

On 28 December 2012 18:40, Pavel Stehule pavel.steh...@gmail.com wrote:
 If
 I understand you, we have a fields that has behave that you expected -
 filename and funcname. And I have not used these fields for
 application programming.

No, that's not what I mean. What I mean is that it seems questionable
to want to do anything *within* an exception handler on the basis of
where an exception was raised from. You should just place exception
handlers more carefully instead.

Are you aware of any popular programming language that provides this
kind of information? Can you tell in a well-principled way what
function a Python exception originated from, for example? These are
the built-in Python 2 exception classes:

http://docs.python.org/2/library/exceptions.html

None of the Python built-in exception types have this kind of
information available from fields or anything. Now, you might say that
Python isn't Postgres, and you'd be right. I'm fairly confident that
you'll find no precedent for a routine_name field in any code that is
even roughly analogous to the elog infrastructure, though, because
acting on the basis of what particular function the exception was
raised from seems quite hazardous - it breaks any kind of
encapsulation that might have existed.

If one person slightly refactors their code, it could break the code
of another person who has never even met or talked to the first.

-- 
Peter Geoghegan   http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

2012/12/28 Peter Geoghegan pe...@2ndquadrant.com:
 On 28 December 2012 18:40, Pavel Stehule pavel.steh...@gmail.com wrote:
 If
 I understand you, we have a fields that has behave that you expected -
 filename and funcname. And I have not used these fields for
 application programming.

 No, that's not what I mean. What I mean is that it seems questionable
 to want to do anything *within* an exception handler on the basis of
 where an exception was raised from. You should just place exception
 handlers more carefully instead.

 Are you aware of any popular programming language that provides this
 kind of information? Can you tell in a well-principled way what
 function a Python exception originated from, for example? These are
 the built-in Python 2 exception classes:

 http://docs.python.org/2/library/exceptions.html


for this subject ANSI SQL is more relevant source or manual for DB2 or
Oracle. Design of Python and native PL languages are different. Python
can use complex nested structures. PL - PL/pgSQL or PL/PSM is designed
for work with simply scalar types. So these environments are not
comparable.

 None of the Python built-in exception types have this kind of
 information available from fields or anything. Now, you might say that
 Python isn't Postgres, and you'd be right. I'm fairly confident that
 you'll find no precedent for a routine_name field in any code that is
 even roughly analogous to the elog infrastructure, though, because
 acting on the basis of what particular function the exception was
 raised from seems quite hazardous - it breaks any kind of
 encapsulation that might have existed.

 If one person slightly refactors their code, it could break the code
 of another person who has never even met or talked to the first.

yes, it is not valid argument, I am sorry. Lot of error fields doesn't
work if developer doesn't respect some coding standards. It is not
just routine_name. Only when your implementation is correct, then code
works.

Best regards

Pavel


 --
 Peter Geoghegan   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training and Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

On 28 December 2012 19:23, Pavel Stehule pavel.steh...@gmail.com wrote:
 for this subject ANSI SQL is more relevant source or manual for DB2 or
 Oracle. Design of Python and native PL languages are different. Python
 can use complex nested structures. PL - PL/pgSQL or PL/PSM is designed
 for work with simply scalar types. So these environments are not
 comparable.

I don't see how the fact that Python can use nested data structures
has any bearing (you could argue that plpgsql does too, fwiw).

Please direct me towards the manual of DB2 or Oracle where it says
that something like routine_name is exposed for error handling
purposes. Correct me if I'm mistaken, but I don't believe that ANSI
SQL has anything to say about any of these error fields. You've just
lifted the names of the fields from various information_schema
catalogs, which is hardly the same thing.

-- 
Peter Geoghegan   http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

2012/12/28 Peter Geoghegan pe...@2ndquadrant.com:
On 28 December 2012 19:23, Pavel Stehule pavel.steh...@gmail.com wrote:
for this subject ANSI SQL is more relevant source or manual for DB2 or
Oracle. Design of Python and native PL languages are different. Python
can use complex nested structures. PL - PL/pgSQL or PL/PSM is designed
for work with simply scalar types. So these environments are not
comparable.

I don't see how the fact that Python can use nested data structures
has any bearing (you could argue that plpgsql does too, fwiw).

Please direct me towards the manual of DB2 or Oracle where it says
that something like routine_name is exposed for error handling
purposes. Correct me if I'm mistaken, but I don't believe that ANSI
SQL has anything to say about any of these error fields. You've just
lifted the names of the fields from various information_schema
catalogs, which is hardly the same thing.

http://publib.boulder.ibm.com/infocenter/iseries/v5r3/index.jsp?topic=%2Fdb2%2Frbafzmstgetdiag.htm
http://savage.net.au/SQL/sql-2003-2.bnf - GET DIAGNOSTICS statement

SQL: 1999, Jim Melton,Alan R. Simon - description of GET DIAGNOSTICS statement
ANSI SQL - Feature F122, Enhanced diagnostics management - table
identifiers for use with get diagnostics statement - and related
description

Regards

Pavel

please, search

--
Peter Geoghegan http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

2012-12-28 Thread Peter Eisentraut

On 12/10/12 4:23 PM, Peter Geoghegan wrote:
 Well, this is an area that the standard doesn't have anything to say
 about. The standard defines errcodes, but not what fields they make
 available. I suppose you could say that the patch proposes that they
 become a Postgres extension to the standard.

The standard certainly does define extra fields for errors; see get
diagnostics statement.  We have a GET DIAGNOSTICS statement in
PL/pgSQL, so there is certainly precedent for all of this.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

2012-12-28 Thread Peter Eisentraut

On 12/28/12 2:03 PM, Peter Geoghegan wrote:
 No, that's not what I mean. What I mean is that it seems questionable
 to want to do anything *within* an exception handler on the basis of
 where an exception was raised from.

Isn't that the whole point of this patch?  The only purpose of this
feature is to make the exception information available in a
machine-readable way.  That functionality has been requested many
times over the years.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

On 28 December 2012 19:55, Pavel Stehule pavel.steh...@gmail.com wrote:
http://publib.boulder.ibm.com/infocenter/iseries/v5r3/index.jsp?topic=%2Fdb2%2Frbafzmstgetdiag.htm

I'm unconvinced by this. First of all, it only applies to the GET
DIAGNOSTICS statement, and the only implementation that actually
currently uses that is DB2, AFAICT. Secondly, DB2 only provides it for
very specific errcode classes that relate to problems that are
peculiar to routines/functions, so in general you cannot rely on the
information being available in the same way as you intend. Clearly,
DB2 provides it for completeness, and not because you can rely on it
being available for error handling purposes. On the other hand, your
latest revision of the patch (eelog5.patch) sees plpgsql jury-rigged
to set the fields itself, which seems like a whole other proposition
entirely.

What's more, you're changing ErrorData to make this happen, rather
than having the user interrogate the server for this information as
GET DIAGNOSTICS does. So I don't see that this supports your case at
all. I maintain that having an exception handler's behaviour vary
based on a field that describes a routine that originally raised the
function is a really bad idea, and that we should not facilitate it.

--
Peter Geoghegan http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

2012-12-28 Thread Peter Eisentraut

On 12/28/12 2:03 PM, Peter Geoghegan wrote:
 Are you aware of any popular programming language that provides this
 kind of information? Can you tell in a well-principled way what
 function a Python exception originated from, for example? These are
 the built-in Python 2 exception classes:
 
 http://docs.python.org/2/library/exceptions.html
 
 None of the Python built-in exception types have this kind of
 information available from fields or anything.

Sure, OSError has a filename attribute (which I'm sure is qualified by a
directory name if necessary), SyntaxError has filename, lineno, etc.

OSError.filename is essentially the equivalent of what is being proposed
here.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

On 28 December 2012 20:34, Peter Eisentraut pete...@gmx.net wrote:
 Isn't that the whole point of this patch?  The only purpose of this
 feature is to make the exception information available in a
 machine-readable way.  That functionality has been requested many
 times over the years.

Right, and I agree that it's very useful for some fields (if you can
actually have a reasonable set of guarantees about where each becomes
available). I just don't think that it's worth including fields like
routine_name within ErrorData, and in fact it may be harmful to do so.

-- 
Peter Geoghegan   http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

2012/12/28 Peter Geoghegan pe...@2ndquadrant.com:
On 28 December 2012 19:55, Pavel Stehule pavel.steh...@gmail.com wrote:
http://publib.boulder.ibm.com/infocenter/iseries/v5r3/index.jsp?topic=%2Fdb2%2Frbafzmstgetdiag.htm

It cannot to wait to GET DIAGNOSTICS request - because when GET
DIAGNOSTICS is called, then all unsupported info about exception is
lost, all used memory will be released. So if we would to support
ROUTINE_NAME or similar fields like CONTEXT or MESSAGE, we have to
store these values to ErrorData without knowledge if this value will
be used or not.

Pavel

--
Peter Geoghegan http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

On 28 December 2012 20:40, Pavel Stehule pavel.steh...@gmail.com wrote:
 It cannot to wait to GET DIAGNOSTICS request - because when GET
 DIAGNOSTICS is called, then all unsupported info about exception is
 lost, all used memory will be released.

I'm not suggesting that you do. I'm only suggesting that the two are
not comparable.

-- 
Peter Geoghegan   http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

2012/12/28 Peter Geoghegan pe...@2ndquadrant.com:
 On 28 December 2012 20:40, Pavel Stehule pavel.steh...@gmail.com wrote:
 It cannot to wait to GET DIAGNOSTICS request - because when GET
 DIAGNOSTICS is called, then all unsupported info about exception is
 lost, all used memory will be released.

 I'm not suggesting that you do. I'm only suggesting that the two are
 not comparable

MESSAGE and similar yes. But CONTEXT is comparable. And there is
interesting precedent. Some years PL/Python or PL/Perl doesn't support
CONTEXT and PL/pgSQL yes. And it was working.

Regards

Pavel


 --
 Peter Geoghegan   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training and Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

On 28 December 2012 20:40, Peter Eisentraut pete...@gmx.net wrote:
 Sure, OSError has a filename attribute (which I'm sure is qualified by a
 directory name if necessary), SyntaxError has filename, lineno, etc.

 OSError.filename is essentially the equivalent of what is being proposed
 here.

I disagree. That isn't the same as what's being proposed here, because
that's something you're only going to get for those particular
exception classes, and I'm guessing that the fields only exist to
facilitate refactoring tools, IDEs and the like.

If BaseException, Exception or StandardError had a function_name
field, and it was idiomatic to change the behaviour of an exception
handler on the basis of the field's value, that would be equivalent.
Obviously that is not the case.

-- 
Peter Geoghegan   http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields

2012-12-28 Thread Tom Lane

Peter Eisentraut pete...@gmx.net writes:
 On 12/28/12 2:03 PM, Peter Geoghegan wrote:
 None of the Python built-in exception types have this kind of
 information available from fields or anything.

 Sure, OSError has a filename attribute (which I'm sure is qualified by a
 directory name if necessary), SyntaxError has filename, lineno, etc.

No no no ... that's completely unrelated.  Those fields report
information about the user-visible object that the error is about.

The point Peter G is making is that reporting the source of the error is
like having the kernel report the name of the function inside the kernel
that threw the error.  Or perhaps the name of the kernel source file
containing that function.

Now, these things are quite useful *to a kernel developer* who's trying
to debug a problem involving an error report.  But they're pretty
useless to an application developer, and certainly any application
developer who relied on such information to control his error handling
code would receive no sympathy when (not if) a new kernel version broke
it.

In the same way, the filename/lineno stuff that we include in ereports
is sometimes useful to Postgres developers --- but it is very hard to
see a reason why application code would do anything else with it except
possibly print it for human reference.

And, in the same way, a CONTEXT traceback is sometimes useful for
debugging a collection of server-side functions --- but it's hard to see
any client-side code using that information in a mechanized way, at
least not while respecting a sane degree of separation between
server-side and client-side code.

So I'm with Peter G on this: the existing CONTEXT mechanism is good
enough, we do not need to split out selected sub-parts of that as
separate error fields.  Moreover, doing so would provide only an utterly
arbitrary subset of the context stack: who's to say whether it would be
more important to report the most closely nested function, or the
outermost one?

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] enhanced error fields