Re: enhanced error fields

On Fri, Dec 28, 2012 at 1:21 PM, Peter Geoghegan <peter(at)2ndquadrant(dot)com> wrote:
> 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.

I don't really agree with this. To be honest, my biggest concern
about this patch is that it will make it take longer to report an
error. At least in the cases where these additional fields are
included, it will take CPU time to populate those fields, and maybe
there will be some overhead even in the cases where they aren't even
used (although I'd expect that to be too little to measure). Now,
maybe that doesn't matter in the case where the error is being
reported back to the client, because the overhead of shipping packets
across even a local socket likely dwarfs the overhead, but I think it
matters a lot where you are running a big exception-catching loop.
That is already painfully slow, and -1 from me on anything that makes
it significantly slower. I have a feeling this isn't the first time
I'm ranting about this topic in relation to this patch, so apologies
if this is old news.

But if we decide that there is no performance issue or that we don't
care about the hit there, then I think Stephen and Pavel are right to
want a large amount of very precise detail. What's the use case for
this feature? Presumably, it's for people for whom parsing the error
message is not a practical option, so either they textual error
message doesn't identify the target object sufficiently precisely, and
they want to make sure they know what it applies to; or else it's for
people who want any error that applies to a table to be handled the
same way (without worrying about exactly which error they have got).
Imagine, for example, someone writing a framework that will be used as
a basis for many different applications. It might want to do
something, like, I don't know, update the comment on a table every
time an error involving that table occurs. Clearly, precise
identification of the table is a must. In a particular application
development environment, it's reasonable to rely on users to name
things sensibly, but if you're shipping a tool that might be dropped
into any arbitrary environment, that's significantly more dangerous.

Similarly, for a function-related error, you'd need something like
that looks like the output of pg_proc.oid::regprocedure, or individual
fields with the various components of that output. That sounds like
routine_name et. al.

I'm not happy about the idea of shipping OIDs back to the client.
OIDs are too user-visible as it is; we should try to make them less
not moreso.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company