Обсуждение: Add schema-qualified relnames in constraint error messages.
Hackers,
At the moment schemas used as single-level namespaces.
It's quite convenient in large databases.
But error messages doesn’t contain schema names.
I have done a little patch with schema-qualified relation names in error messages that produced by foreign key constraints and triggers.
Patch and usage example are attached.
Based on real bug hunting.
Вложения
Petr Korobeinikov <pkorobeinikov@gmail.com> writes:
> At the moment schemas used as single-level namespaces.
> It's quite convenient in large databases.
> But error messages doesn’t contain schema names.
> I have done a little patch with schema-qualified relation names in error
> messages that produced by foreign key constraints and triggers.
We have gone around on this before and pretty much concluded we didn't
want to do it; the demand is too small and the risk of breaking things
too large. In particular, your patch as presented *would* break every
application that is still parsing primary error messages to extract
object names from them.
What seems more likely to be useful and to not break anything is to push
forward on adding PG_DIAG_SCHEMA_NAME and similar auxiliary fields to more
error messages (see errtable() and siblings). That would allow
applications to extract this information automatically and reliably.
Only after that work is complete and there's been a reasonable amount of
time for clients to start relying on it can we really start thinking about
whacking the message texts around.
Aside from those points, it's quite unacceptable to format qualified names
as you propose here; they would really have to look more like "foo"."bar"
to prevent confusion.
regards, tom lane
On 1/5/16 7:21 PM, Tom Lane wrote: > What seems more likely to be useful and to not break anything is to push > forward on adding PG_DIAG_SCHEMA_NAME and similar auxiliary fields to more > error messages (see errtable() and siblings). That would allow > applications to extract this information automatically and reliably. > Only after that work is complete and there's been a reasonable amount of > time for clients to start relying on it can we really start thinking about > whacking the message texts around. +1, but... does psql do anything with those fields? ISTM the biggest use for this info is someone sitting at psql or pgAdmin. Maybe schema info could be presented in HINT or DETAIL messages as well? -- Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX Experts in Analytics, Data Architecture and PostgreSQL Data in Trouble? Get it in Treble! http://BlueTreble.com
Jim Nasby <Jim.Nasby@BlueTreble.com> writes:
> does psql do anything with those fields? ISTM the biggest use for this
> info is someone sitting at psql or pgAdmin.
Sure, if you turn up the error verbosity.
regression=# create table foo (f1 int primary key);
CREATE TABLE
regression=# create table bar (f1 int references foo);
CREATE TABLE
regression=# insert into bar values(1);
ERROR: insert or update on table "bar" violates foreign key constraint "bar_f1_fkey"
DETAIL: Key (f1)=(1) is not present in table "foo".
regression=# \set VERBOSITY verbose
regression=# insert into bar values(1);
ERROR: 23503: insert or update on table "bar" violates foreign key constraint "bar_f1_fkey"
DETAIL: Key (f1)=(1) is not present in table "foo".
SCHEMA NAME: public
TABLE NAME: bar
CONSTRAINT NAME: bar_f1_fkey
LOCATION: ri_ReportViolation, ri_triggers.c:3326
I can't speak to pgAdmin, but if it doesn't make this info available
the answer is to fix pgAdmin ...
regards, tom lane
On 1/5/16 8:41 PM, Tom Lane wrote: > Jim Nasby<Jim.Nasby@BlueTreble.com> writes: >> >does psql do anything with those fields? ISTM the biggest use for this >> >info is someone sitting at psql or pgAdmin. > Sure, if you turn up the error verbosity. FWIW, I suspect very few people know about the verbosity setting (I didn't until a few months ago...) Maybe psql should hint about it the first time an error is reported in a session. -- Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX Experts in Analytics, Data Architecture and PostgreSQL Data in Trouble? Get it in Treble! http://BlueTreble.com
Jim Nasby <Jim.Nasby@bluetreble.com> writes:
> FWIW, I suspect very few people know about the verbosity setting (I
> didn't until a few months ago...) Maybe psql should hint about it the
> first time an error is reported in a session.
Actually, what'd be really handy IMO is something to regurgitate the
most recent error in verbose mode, without making a permanent session
state change. Something like
regression=# insert into bar values(1);
ERROR: insert or update on table "bar" violates foreign key constraint "bar_f1_fkey"
DETAIL: Key (f1)=(1) is not present in table "foo".
regression=# \saywhat
ERROR: 23503: insert or update on table "bar" violates foreign key constraint "bar_f1_fkey"
DETAIL: Key (f1)=(1) is not present in table "foo".
SCHEMA NAME: public
TABLE NAME: bar
CONSTRAINT NAME: bar_f1_fkey
LOCATION: ri_ReportViolation, ri_triggers.c:3326
regression=#
Not sure how hard that would be to do within psql's current structure.
regards, tom lane
On 1/5/16 9:16 PM, Tom Lane wrote: > Jim Nasby <Jim.Nasby@bluetreble.com> writes: >> FWIW, I suspect very few people know about the verbosity setting (I >> didn't until a few months ago...) Maybe psql should hint about it the >> first time an error is reported in a session. > > Actually, what'd be really handy IMO is something to regurgitate the > most recent error in verbose mode, without making a permanent session > state change. Something like > > regression=# insert into bar values(1); > ERROR: insert or update on table "bar" violates foreign key constraint "bar_f1_fkey" > DETAIL: Key (f1)=(1) is not present in table "foo". > regression=# \saywhat > ERROR: 23503: insert or update on table "bar" violates foreign key constraint "bar_f1_fkey" > DETAIL: Key (f1)=(1) is not present in table "foo". > SCHEMA NAME: public > TABLE NAME: bar > CONSTRAINT NAME: bar_f1_fkey > LOCATION: ri_ReportViolation, ri_triggers.c:3326 > regression=# > > Not sure how hard that would be to do within psql's current structure. At first glance, it looks like it just means changing AcceptResult() to use PQresultErrorField in addition to PQresultErrorMessage, and stuffing the results somewhere. And of course adding \saywhat to the psql parser, but maybe someone more versed in psql code can verify that. If it is that simple, looks like another good beginner hacker task. :) -- Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX Experts in Analytics, Data Architecture and PostgreSQL Data in Trouble? Get it in Treble! http://BlueTreble.com
On Wed, Jan 6, 2016 at 5:06 AM, Jim Nasby <Jim.Nasby@bluetreble.com> wrote:
On 1/5/16 9:16 PM, Tom Lane wrote:Jim Nasby <Jim.Nasby@bluetreble.com> writes:FWIW, I suspect very few people know about the verbosity setting (I
didn't until a few months ago...) Maybe psql should hint about it the
first time an error is reported in a session.
Actually, what'd be really handy IMO is something to regurgitate the
most recent error in verbose mode, without making a permanent session
state change. Something like
regression=# insert into bar values(1);
ERROR: insert or update on table "bar" violates foreign key constraint "bar_f1_fkey"
DETAIL: Key (f1)=(1) is not present in table "foo".
regression=# \saywhat
ERROR: 23503: insert or update on table "bar" violates foreign key constraint "bar_f1_fkey"
DETAIL: Key (f1)=(1) is not present in table "foo".
SCHEMA NAME: public
TABLE NAME: bar
CONSTRAINT NAME: bar_f1_fkey
LOCATION: ri_ReportViolation, ri_triggers.c:3326
regression=#
Not sure how hard that would be to do within psql's current structure.
At first glance, it looks like it just means changing AcceptResult() to use PQresultErrorField in addition to PQresultErrorMessage, and stuffing the results somewhere. And of course adding \saywhat to the psql parser, but maybe someone more versed in psql code can verify that.
If it is that simple, looks like another good beginner hacker task. :)
Sorry, I couldn't resist it: I was too excited to learn such option existed. :-)
Please find attached a POC patch, using \errverbose for the command name. Unfortunately, I didn't see a good way to contain the change in psql only and had to change libpq, adding new interface PQresultBuildErrorMessage(). Also, what I don't like very much is that we need to keep track of the last result from last SendQuery() in psql's pset. So in my view this is sort of a dirty hack that works nonetheless.
Cheers!
--
Alex
Вложения
On Wed, Jan 6, 2016 at 3:02 PM, Shulgin, Oleksandr <oleksandr.shulgin@zalando.de> wrote:
Please find attached a POC patch, using \errverbose for the command name. Unfortunately, I didn't see a good way to contain the change in psql only and had to change libpq, adding new interface PQresultBuildErrorMessage(). Also, what I don't like very much is that we need to keep track of the last result from last SendQuery() in psql's pset. So in my view this is sort of a dirty hack that works nonetheless.
Added to the Open commitfest: https://commitfest.postgresql.org/9/475/
Shulgin, Oleksandr wrote: > Added to the Open commitfest: https://commitfest.postgresql.org/9/475/ Here's a review. Note that the patch tested and submitted is not the initial one in the thread, so it doesn't exactly match $subject now. What's tested here is a client-side approach, suggested by Tom upthread as an alternative, and implemented by Oleksandr in 0001-POC-errverbose-in-psql.patch The patch has two parts: one part in libpq exposing a new function named PQresultBuildErrorMessage, and a second part implementing an \errverbose command in psql, essentially displaying the result of the function. The separation makes sense if we consider that other clients might benefit from the function, or that libpq is a better place than psql to maintain this in the future, as the list of error fields available in a PGresult might grow. OTOH maybe adding a new libpq function just for that is overkill, but this is subjective. psql part ========= Compiles and works as intended. For instance with \set VERBOSITY default, an FK violation produces: # insert into table2 values(10); ERROR: insert or update on table "table2" violates foreign key constraint "table2_id_tbl1_fkey"DETAIL: Key (id_tbl1)=(10) is not present in table "table1". Then \errverbose just displays the verbose form of the error: # \errverbose ERROR: 23503: insert or update on table "table2"violates foreign key constraint "table2_id_tbl1_fkey" DETAIL: Key (id_tbl1)=(10) is not present in table "table1". SCHEMA NAME: public TABLE NAME: table2 CONSTRAINT NAME: table2_id_tbl1_fkey LOCATION: ri_ReportViolation,ri_triggers.c:3326 - make check passes - valgrind test is OK (no obvious leak after using the command). Missing bits: - the command is not mentioned in the help (\? command, see help.c) - it should be added to tab completions (see tab-complete.c) - it needs to be described in the SGML documentation libpq part ========== The patch implements and exports a new PQresultBuildErrorMessage() function. AFAICS its purpose is to produce a result similar to what PQresultErrorMessage() would have produced, if PQERRORS_VERBOSE was the verbosity in effect at execution time. My comments: - the name of the function does not really hint at what it does. Maybe something like PQresultVerboseErrorMessage() would be more explicit? I would expect the new function to have the same interface than PQresultErrorMessage(), but it's not the case. - it takes a PGconn* and PGresult* as input parameters, but PQresultErrorMessage takes only a <const PGresult*> as input. It's not clear why PGconn* is necessary. - it returns a pointer to a strdup'ed() buffer, which has to be freed separately by the caller. That differs from PQresultErrorMessage() which keeps this managed inside the PGresult struct. - if protocol version < 3, an error message is returned. It's not clear to the caller that this error is emitted by the function itself rather than the query. Besides, are we sure it's necessary? Maybe the function could just produce an output with whatever error fields it has, even minimally with older protocol versions, rather than failing? - if the fixed error message is kept, it should pass through libpq_gettext() for translation. - a description of the function should be added to the SGML doc. There's a sentence in PQsetErrorVerbosity() that says, currently: "Changing the verbosity does not affect the messages available from already-existing PGresult objects, only subsequently-createdones." As it's precisely the point of that new function, that bit could be altered to refer to it. Best regards, -- Daniel Vérité PostgreSQL-powered mailer: http://www.manitou-mail.org Twitter: @DanielVerite
On Mon, Feb 8, 2016 at 5:24 PM, Daniel Verite <daniel@manitou-mail.org> wrote:
I've edited the CF entry title to read: Add \errverbose to psql (and support in libpq)
Shulgin, Oleksandr wrote:
> Added to the Open commitfest: https://commitfest.postgresql.org/9/475/
Here's a review. Note that the patch tested and submitted
is not the initial one in the thread, so it doesn't exactly
match $subject now.
What's tested here is a client-side approach, suggested by Tom
upthread as an alternative, and implemented by Oleksandr in
0001-POC-errverbose-in-psql.patch
The patch has two parts: one part in libpq exposing a new function
named PQresultBuildErrorMessage, and a second part implementing an
\errverbose command in psql, essentially displaying the result of the
function.
The separation makes sense if we consider that other clients might benefit
from the function, or that libpq is a better place than psql to maintain
this in the future, as the list of error fields available in a PGresult
might grow.
OTOH maybe adding a new libpq function just for that is overkill, but this
is subjective.
psql part
=========
Compiles and works as intended.
For instance with \set VERBOSITY default, an FK violation produces:
# insert into table2 values(10);
ERROR: insert or update on table "table2" violates foreign key constraint
"table2_id_tbl1_fkey"
DETAIL: Key (id_tbl1)=(10) is not present in table "table1".
Then \errverbose just displays the verbose form of the error:
# \errverbose
ERROR: 23503: insert or update on table "table2" violates foreign
key constraint "table2_id_tbl1_fkey"
DETAIL: Key (id_tbl1)=(10) is not present in table "table1".
SCHEMA NAME: public
TABLE NAME: table2
CONSTRAINT NAME: table2_id_tbl1_fkey
LOCATION: ri_ReportViolation, ri_triggers.c:3326
- make check passes
- valgrind test is OK (no obvious leak after using the command).
Missing bits:
- the command is not mentioned in the help (\? command, see help.c)
- it should be added to tab completions (see tab-complete.c)
- it needs to be described in the SGML documentation
libpq part
==========
The patch implements and exports a new PQresultBuildErrorMessage()
function.
AFAICS its purpose is to produce a result similar to what
PQresultErrorMessage() would have produced, if PQERRORS_VERBOSE
was the verbosity in effect at execution time.
My comments:
- the name of the function does not really hint at what it does.
Maybe something like PQresultVerboseErrorMessage() would be more
explicit?
Not exactly, the verbosity setting might or might not be in effect when this function is called. Another external part of the state that might affect the result is conn->show_context field.
I would expect the new function to have the same interface than
PQresultErrorMessage(), but it's not the case.
- it takes a PGconn* and PGresult* as input parameters, but
PQresultErrorMessage takes only a <const PGresult*> as input.
It's not clear why PGconn* is necessary.
This is because PQresultErrorMessage() returns a stored error message: res->errMsg (or "").
- it returns a pointer to a strdup'ed() buffer, which
has to be freed separately by the caller. That differs from
PQresultErrorMessage() which keeps this managed inside the
PGresult struct.
For the same reason: this function actually produces a new error message, as opposed to returning a stored one.
- if protocol version < 3, an error message is returned. It's not
clear to the caller that this error is emitted by the function itself
rather than the query. Besides, are we sure it's necessary?
Maybe the function could just produce an output with whatever
error fields it has, even minimally with older protocol versions,
rather than failing?
Hm, probably we could just copy the message from conn->errorMessage, in case of protocol v2, but I don't see a point in passing PGresult to the func or naming it PQresult<Something> in the case of v2.
- if the fixed error message is kept, it should pass through
libpq_gettext() for translation.
Good point.
- a description of the function should be added to the SGML doc.
There's a sentence in PQsetErrorVerbosity() that says, currently:
"Changing the verbosity does not affect the messages available from
already-existing PGresult objects, only subsequently-created ones."
As it's precisely the point of that new function, that bit could
be altered to refer to it.
I didn't touch the documentation specifically, because this is just a proof-of-concept, but it's good that you notice this detail. Most importantly, I'd like to learn of better options than storing the whole last_result in psql's pset structure.
Thanks for the review!
--
Alex
Shulgin, Oleksandr wrote: > Most importantly, I'd like to learn of better options than storing the > whole last_result in psql's pset structure. I guess that you could, each time a query fails, gather silently the result of \errverbose, store it in a buffer, discard the PGresult, and in case the user does \errverbose before running another query, output what was in that buffer. Best regards, -- Daniel Vérité PostgreSQL-powered mailer: http://www.manitou-mail.org Twitter: @DanielVerite
On Tue, Jan 5, 2016 at 10:16 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Jim Nasby <Jim.Nasby@bluetreble.com> writes: >> FWIW, I suspect very few people know about the verbosity setting (I >> didn't until a few months ago...) Maybe psql should hint about it the >> first time an error is reported in a session. > > Actually, what'd be really handy IMO is something to regurgitate the > most recent error in verbose mode, without making a permanent session > state change. Something like > > regression=# insert into bar values(1); > ERROR: insert or update on table "bar" violates foreign key constraint "bar_f1_fkey" > DETAIL: Key (f1)=(1) is not present in table "foo". > regression=# \saywhat > ERROR: 23503: insert or update on table "bar" violates foreign key constraint "bar_f1_fkey" > DETAIL: Key (f1)=(1) is not present in table "foo". > SCHEMA NAME: public > TABLE NAME: bar > CONSTRAINT NAME: bar_f1_fkey > LOCATION: ri_ReportViolation, ri_triggers.c:3326 > regression=# Wow, that's a fabulous idea. I see Oleksandr has tried to implement it, although I haven't looked at the patch. But I think this would be REALLY helpful. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
> most recent error in verbose mode, without making a permanent session
> state change. Something like
>
> regression=# insert into bar values(1);
> ERROR: insert or update on table "bar" violates foreign key constraint "bar_f1_fkey"
> DETAIL: Key (f1)=(1) is not present in table "foo".
> regression=# \saywhat
> ERROR: 23503: insert or update on table "bar" violates foreign key constraint "bar_f1_fkey"
> DETAIL: Key (f1)=(1) is not present in table "foo".
> SCHEMA NAME: public
> TABLE NAME: bar
> CONSTRAINT NAME: bar_f1_fkey
> LOCATION: ri_ReportViolation, ri_triggers.c:3326
> regression=#
Wow, that's a fabulous idea. I see Oleksandr has tried to implement
it, although I haven't looked at the patch. But I think this would be
REALLY helpful.
yes
+1
+1
Pavel
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
> most recent error in verbose mode, without making a permanent session> state change. Something like
>
> regression=# insert into bar values(1);
> ERROR: insert or update on table "bar" violates foreign key constraint "bar_f1_fkey"
> DETAIL: Key (f1)=(1) is not present in table "foo".
> regression=# \saywhat
Maybe its too cute but I'll give a +1 for this alone.
David J.
On Wed, Feb 10, 2016 at 12:33 AM, Daniel Verite <daniel@manitou-mail.org> wrote:
Shulgin, Oleksandr wrote:
> Most importantly, I'd like to learn of better options than storing the
> whole last_result in psql's pset structure.
I guess that you could, each time a query fails, gather silently the
result of \errverbose, store it in a buffer, discard the PGresult,
and in case the user does \errverbose before running another query,
output what was in that buffer.
That's a neat idea. I also think that we could only store last PGresult when the query fails actually and discard it otherwise: the PGresult holding only the error doesn't occupy too much space.
What I dislike about this POC is all the disruption in libpq, to be honest. It would be much neater if we could form the verbose message every time and let the client decide where to cut it. Maybe a bit "too clever" would be to put a \0 char between short message and it's verbose continuation. The client could then reach the verbose part like this (assuming that libpq did put a verbose part there): msg + strlen(msg) + 1.
--
Alex
"Shulgin, Oleksandr" <oleksandr.shulgin@zalando.de> writes:
> What I dislike about this POC is all the disruption in libpq, to be
> honest.
Yeah, I don't much like that either. But I don't think we can avoid
some refactoring there; as designed, conversion of an error message into
user-visible form is too tightly tied to receipt of the message.
> It would be much neater if we could form the verbose message every
> time and let the client decide where to cut it. Maybe a bit "too clever"
> would be to put a \0 char between short message and it's verbose
> continuation. The client could then reach the verbose part like this
> (assuming that libpq did put a verbose part there): msg + strlen(msg) + 1.
Blech :-(
Thinking about it, though, it seems to me that we could get away with
always performing both styles of conversion and sticking both strings
into the PGresult. That would eat a little more space but not much.
Then we just need to add API to let the application get at both forms.
regards, tom lane
On Mon, Mar 14, 2016 at 7:55 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
"Shulgin, Oleksandr" <oleksandr.shulgin@zalando.de> writes:
> What I dislike about this POC is all the disruption in libpq, to be
> honest.
Yeah, I don't much like that either. But I don't think we can avoid
some refactoring there; as designed, conversion of an error message into
user-visible form is too tightly tied to receipt of the message.
True. Attached is a v2 which addresses all of the points raised earlier I believe.
We still extract the message building part of code from pqGetErrorNotice3(), but the user-facing change is much more sane now: just added a new function PQresultVerboseErrorMessage().
> It would be much neater if we could form the verbose message every
> time and let the client decide where to cut it. Maybe a bit "too clever"
> would be to put a \0 char between short message and it's verbose
> continuation. The client could then reach the verbose part like this
> (assuming that libpq did put a verbose part there): msg + strlen(msg) + 1.
Blech :-(
:-) That would not work either way, I've just noticed that SQLLEVEL is put at the start of the message in verbose mode, but not by default.
Thinking about it, though, it seems to me that we could get away with
always performing both styles of conversion and sticking both strings
into the PGresult. That would eat a little more space but not much.
Then we just need to add API to let the application get at both forms.
This is what the v2 basically implements, now complete with help, tab-complete and documentation changes. I don't think we can add a usual simple regression test here reliably, due to LOCATION field which might be subject to unexpected line number changes. But do we really need one?
--
Regards,
Alex
Вложения
On Tue, Mar 15, 2016 at 4:44 PM, Shulgin, Oleksandr <oleksandr.shulgin@zalando.de> wrote:
On Mon, Mar 14, 2016 at 7:55 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:"Shulgin, Oleksandr" <oleksandr.shulgin@zalando.de> writes:
> What I dislike about this POC is all the disruption in libpq, to be
> honest.
Yeah, I don't much like that either. But I don't think we can avoid
some refactoring there; as designed, conversion of an error message into
user-visible form is too tightly tied to receipt of the message.True. Attached is a v2 which addresses all of the points raised earlier I believe.We still extract the message building part of code from pqGetErrorNotice3(), but the user-facing change is much more sane now: just added a new function PQresultVerboseErrorMessage().
I wonder if this sort of change has any chance to be back-patched? If not, it would be really nice to have any sort of review soonish.
Thank you.
--
Alex
"Shulgin, Oleksandr" <oleksandr.shulgin@zalando.de> writes:
> On Mon, Mar 14, 2016 at 7:55 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Yeah, I don't much like that either. But I don't think we can avoid
>> some refactoring there; as designed, conversion of an error message into
>> user-visible form is too tightly tied to receipt of the message.
> True. Attached is a v2 which addresses all of the points raised earlier I
> believe.
I took a closer look at what's going on here and realized that actually
it's not that hard to decouple the message-building routine from the
PGconn state, because mostly it works with fields it's extracting out
of the PGresult anyway. The only piece of information that's lacking
is conn->last_query. I propose therefore that instead of doing it like
this, we copy last_query into error PGresults. This is strictly less
added storage requirement than storing the whole verbose message would be,
and it saves time compared to the v2 patch in the typical case where
the application never does ask for an alternately-formatted error message.
Plus we can actually support requests for any variant format, not only
VERBOSE.
Attached is a libpq-portion-only version of a patch doing it this way.
I've not yet looked at the psql part of your patch.
Comments?
regards, tom lane
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 2328d8f..80f7014 100644
*** a/doc/src/sgml/libpq.sgml
--- b/doc/src/sgml/libpq.sgml
*************** char *PQresultErrorMessage(const PGresul
*** 2691,2696 ****
--- 2691,2738 ----
</listitem>
</varlistentry>
+ <varlistentry id="libpq-pqresultverboseerrormessage">
+ <term>
+ <function>PQresultVerboseErrorMessage</function>
+ <indexterm>
+ <primary>PQresultVerboseErrorMessage</primary>
+ </indexterm>
+ </term>
+
+ <listitem>
+ <para>
+ Returns a reformatted version of the error message associated with
+ a <structname>PGresult</> object.
+ <synopsis>
+ char *PQresultVerboseErrorMessage(const PGresult *res,
+ PGVerbosity verbosity,
+ PGContextVisibility show_context);
+ </synopsis>
+ In some situations a client might wish to obtain a more detailed
+ version of a previously-reported error.
+ <function>PQresultVerboseErrorMessage</function> addresses this need
+ by computing the message that would have been produced
+ by <function>PQresultErrorMessage</function> if the specified
+ verbosity settings had been in effect for the connection when the
+ given <structname>PGresult</> was generated. If
+ the <structname>PGresult</> is not an error result,
+ <quote>PGresult is not an error result</> is reported instead.
+ The returned string includes a trailing newline.
+ </para>
+
+ <para>
+ Unlike most other functions for extracting data from
+ a <structname>PGresult</>, the result of this function is a freshly
+ allocated string. The caller must free it
+ using <function>PQfreemem()</> when the string is no longer needed.
+ </para>
+
+ <para>
+ A NULL return is possible if there is insufficient memory.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="libpq-pqresulterrorfield">
<term><function>PQresultErrorField</function><indexterm><primary>PQresultErrorField</></></term>
<listitem>
*************** PGVerbosity PQsetErrorVerbosity(PGconn *
*** 5582,5587 ****
--- 5624,5631 ----
mode includes all available fields. Changing the verbosity does not
affect the messages available from already-existing
<structname>PGresult</> objects, only subsequently-created ones.
+ (But see <function>PQresultVerboseErrorMessage</function> if you
+ want to print a previous error with a different verbosity.)
</para>
</listitem>
</varlistentry>
*************** PGContextVisibility PQsetErrorContextVis
*** 5622,5627 ****
--- 5666,5673 ----
affect the messages available from
already-existing <structname>PGresult</> objects, only
subsequently-created ones.
+ (But see <function>PQresultVerboseErrorMessage</function> if you
+ want to print a previous error with a different display mode.)
</para>
</listitem>
</varlistentry>
*************** PQsetNoticeProcessor(PGconn *conn,
*** 6089,6096 ****
receiver function is called. It is passed the message in the form of
a <symbol>PGRES_NONFATAL_ERROR</symbol>
<structname>PGresult</structname>. (This allows the receiver to extract
! individual fields using <function>PQresultErrorField</>, or the complete
! preformatted message using <function>PQresultErrorMessage</>.) The same
void pointer passed to <function>PQsetNoticeReceiver</function> is also
passed. (This pointer can be used to access application-specific state
if needed.)
--- 6135,6143 ----
receiver function is called. It is passed the message in the form of
a <symbol>PGRES_NONFATAL_ERROR</symbol>
<structname>PGresult</structname>. (This allows the receiver to extract
! individual fields using <function>PQresultErrorField</>, or complete
! preformatted messages using <function>PQresultErrorMessage</> or
! <function>PQresultVerboseErrorMessage</>.) The same
void pointer passed to <function>PQsetNoticeReceiver</function> is also
passed. (This pointer can be used to access application-specific state
if needed.)
diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt
index c69a4d5..21dd772 100644
*** a/src/interfaces/libpq/exports.txt
--- b/src/interfaces/libpq/exports.txt
*************** PQsslStruct 167
*** 170,172 ****
--- 170,173 ----
PQsslAttributeNames 168
PQsslAttribute 169
PQsetErrorContextVisibility 170
+ PQresultVerboseErrorMessage 171
diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index 41937c0..2621767 100644
*** a/src/interfaces/libpq/fe-exec.c
--- b/src/interfaces/libpq/fe-exec.c
*************** PQmakeEmptyPGresult(PGconn *conn, ExecSt
*** 159,164 ****
--- 159,165 ----
result->nEvents = 0;
result->errMsg = NULL;
result->errFields = NULL;
+ result->errQuery = NULL;
result->null_field[0] = '\0';
result->curBlock = NULL;
result->curOffset = 0;
*************** PQresultErrorMessage(const PGresult *res
*** 2599,2604 ****
--- 2600,2643 ----
}
char *
+ PQresultVerboseErrorMessage(const PGresult *res,
+ PGVerbosity verbosity,
+ PGContextVisibility show_context)
+ {
+ PQExpBufferData workBuf;
+
+ /*
+ * Because the caller is expected to free the result string, we must
+ * strdup any constant result. We use plain strdup and document that
+ * callers should expect NULL if out-of-memory.
+ */
+ if (!res ||
+ (res->resultStatus != PGRES_FATAL_ERROR &&
+ res->resultStatus != PGRES_NONFATAL_ERROR))
+ return strdup(libpq_gettext("PGresult is not an error result\n"));
+
+ initPQExpBuffer(&workBuf);
+
+ /*
+ * Currently, we pass this off to fe-protocol3.c in all cases; it will
+ * behave reasonably sanely with an error reported by fe-protocol2.c as
+ * well. If necessary, we could record the protocol version in PGresults
+ * so as to be able to invoke a version-specific message formatter, but
+ * for now there's no need.
+ */
+ pqBuildErrorMessage3(&workBuf, res, verbosity, show_context);
+
+ /* If insufficient memory to format the message, fail cleanly */
+ if (PQExpBufferDataBroken(workBuf))
+ {
+ termPQExpBuffer(&workBuf);
+ return strdup(libpq_gettext("out of memory\n"));
+ }
+
+ return workBuf.data;
+ }
+
+ char *
PQresultErrorField(const PGresult *res, int fieldcode)
{
PGMessageField *pfield;
diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index 3034773..481ee9b 100644
*** a/src/interfaces/libpq/fe-protocol3.c
--- b/src/interfaces/libpq/fe-protocol3.c
*************** pqGetErrorNotice3(PGconn *conn, bool isE
*** 878,886 ****
PGresult *res = NULL;
PQExpBufferData workBuf;
char id;
- const char *val;
- const char *querytext = NULL;
- int querypos = 0;
/*
* Since the fields might be pretty long, we create a temporary
--- 878,883 ----
*************** pqGetErrorNotice3(PGconn *conn, bool isE
*** 905,910 ****
--- 902,909 ----
/*
* Read the fields and save into res.
+ *
+ * Also, save the SQLSTATE in conn->last_sqlstate.
*/
for (;;)
{
*************** pqGetErrorNotice3(PGconn *conn, bool isE
*** 915,956 ****
if (pqGets(&workBuf, conn))
goto fail;
pqSaveMessageField(res, id, workBuf.data);
}
/*
* Now build the "overall" error message for PQresultErrorMessage.
- *
- * Also, save the SQLSTATE in conn->last_sqlstate.
*/
resetPQExpBuffer(&workBuf);
val = PQresultErrorField(res, PG_DIAG_SEVERITY);
if (val)
! appendPQExpBuffer(&workBuf, "%s: ", val);
! val = PQresultErrorField(res, PG_DIAG_SQLSTATE);
! if (val)
{
! if (strlen(val) < sizeof(conn->last_sqlstate))
! strcpy(conn->last_sqlstate, val);
! if (conn->verbosity == PQERRORS_VERBOSE)
! appendPQExpBuffer(&workBuf, "%s: ", val);
}
val = PQresultErrorField(res, PG_DIAG_MESSAGE_PRIMARY);
if (val)
! appendPQExpBufferStr(&workBuf, val);
val = PQresultErrorField(res, PG_DIAG_STATEMENT_POSITION);
if (val)
{
! if (conn->verbosity != PQERRORS_TERSE && conn->last_query != NULL)
{
/* emit position as a syntax cursor display */
! querytext = conn->last_query;
querypos = atoi(val);
}
else
{
/* emit position as text addition to primary message */
/* translator: %s represents a digit string */
! appendPQExpBuffer(&workBuf, libpq_gettext(" at character %s"),
val);
}
}
--- 914,1032 ----
if (pqGets(&workBuf, conn))
goto fail;
pqSaveMessageField(res, id, workBuf.data);
+ if (id == PG_DIAG_SQLSTATE)
+ strlcpy(conn->last_sqlstate, workBuf.data,
+ sizeof(conn->last_sqlstate));
}
/*
+ * Save the active query text, if any, into res as well.
+ */
+ if (res && conn->last_query)
+ res->errQuery = pqResultStrdup(res, conn->last_query);
+
+ /*
* Now build the "overall" error message for PQresultErrorMessage.
*/
resetPQExpBuffer(&workBuf);
+ pqBuildErrorMessage3(&workBuf, res, conn->verbosity, conn->show_context);
+
+ /*
+ * Either save error as current async result, or just emit the notice.
+ */
+ if (isError)
+ {
+ if (res)
+ res->errMsg = pqResultStrdup(res, workBuf.data);
+ pqClearAsyncResult(conn);
+ conn->result = res;
+ if (PQExpBufferDataBroken(workBuf))
+ printfPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("out of memory"));
+ else
+ appendPQExpBufferStr(&conn->errorMessage, workBuf.data);
+ }
+ else
+ {
+ /* if we couldn't allocate the result set, just discard the NOTICE */
+ if (res)
+ {
+ /* We can cheat a little here and not copy the message. */
+ res->errMsg = workBuf.data;
+ if (res->noticeHooks.noticeRec != NULL)
+ (*res->noticeHooks.noticeRec) (res->noticeHooks.noticeRecArg, res);
+ PQclear(res);
+ }
+ }
+
+ termPQExpBuffer(&workBuf);
+ return 0;
+
+ fail:
+ PQclear(res);
+ termPQExpBuffer(&workBuf);
+ return EOF;
+ }
+
+ /*
+ * Construct an error message from the fields in the given PGresult,
+ * appending it to the contents of "msg".
+ */
+ void
+ pqBuildErrorMessage3(PQExpBuffer msg, const PGresult *res,
+ PGVerbosity verbosity, PGContextVisibility show_context)
+ {
+ const char *val;
+ const char *querytext = NULL;
+ int querypos = 0;
+
+ /* If we couldn't allocate a PGresult, just say "out of memory" */
+ if (res == NULL)
+ {
+ appendPQExpBuffer(msg, libpq_gettext("out of memory\n"));
+ return;
+ }
+
+ /*
+ * If we don't have any broken-down fields, just return the base message.
+ * This mainly applies if we're given a libpq-generated error result.
+ */
+ if (res->errFields == NULL)
+ {
+ if (res->errMsg && res->errMsg[0])
+ appendPQExpBufferStr(msg, res->errMsg);
+ else
+ appendPQExpBuffer(msg, libpq_gettext("no error message available\n"));
+ return;
+ }
+
+ /* Else build error message from relevant fields */
val = PQresultErrorField(res, PG_DIAG_SEVERITY);
if (val)
! appendPQExpBuffer(msg, "%s: ", val);
! if (verbosity == PQERRORS_VERBOSE)
{
! val = PQresultErrorField(res, PG_DIAG_SQLSTATE);
! if (val)
! appendPQExpBuffer(msg, "%s: ", val);
}
val = PQresultErrorField(res, PG_DIAG_MESSAGE_PRIMARY);
if (val)
! appendPQExpBufferStr(msg, val);
val = PQresultErrorField(res, PG_DIAG_STATEMENT_POSITION);
if (val)
{
! if (verbosity != PQERRORS_TERSE && res->errQuery != NULL)
{
/* emit position as a syntax cursor display */
! querytext = res->errQuery;
querypos = atoi(val);
}
else
{
/* emit position as text addition to primary message */
/* translator: %s represents a digit string */
! appendPQExpBuffer(msg, libpq_gettext(" at character %s"),
val);
}
}
*************** pqGetErrorNotice3(PGconn *conn, bool isE
*** 960,966 ****
if (val)
{
querytext = PQresultErrorField(res, PG_DIAG_INTERNAL_QUERY);
! if (conn->verbosity != PQERRORS_TERSE && querytext != NULL)
{
/* emit position as a syntax cursor display */
querypos = atoi(val);
--- 1036,1042 ----
if (val)
{
querytext = PQresultErrorField(res, PG_DIAG_INTERNAL_QUERY);
! if (verbosity != PQERRORS_TERSE && querytext != NULL)
{
/* emit position as a syntax cursor display */
querypos = atoi(val);
*************** pqGetErrorNotice3(PGconn *conn, bool isE
*** 969,1027 ****
{
/* emit position as text addition to primary message */
/* translator: %s represents a digit string */
! appendPQExpBuffer(&workBuf, libpq_gettext(" at character %s"),
val);
}
}
}
! appendPQExpBufferChar(&workBuf, '\n');
! if (conn->verbosity != PQERRORS_TERSE)
{
if (querytext && querypos > 0)
! reportErrorPosition(&workBuf, querytext, querypos,
! conn->client_encoding);
val = PQresultErrorField(res, PG_DIAG_MESSAGE_DETAIL);
if (val)
! appendPQExpBuffer(&workBuf, libpq_gettext("DETAIL: %s\n"), val);
val = PQresultErrorField(res, PG_DIAG_MESSAGE_HINT);
if (val)
! appendPQExpBuffer(&workBuf, libpq_gettext("HINT: %s\n"), val);
val = PQresultErrorField(res, PG_DIAG_INTERNAL_QUERY);
if (val)
! appendPQExpBuffer(&workBuf, libpq_gettext("QUERY: %s\n"), val);
! if (conn->show_context == PQSHOW_CONTEXT_ALWAYS ||
! (conn->show_context == PQSHOW_CONTEXT_ERRORS && isError))
{
val = PQresultErrorField(res, PG_DIAG_CONTEXT);
if (val)
! appendPQExpBuffer(&workBuf, libpq_gettext("CONTEXT: %s\n"),
val);
}
}
! if (conn->verbosity == PQERRORS_VERBOSE)
{
val = PQresultErrorField(res, PG_DIAG_SCHEMA_NAME);
if (val)
! appendPQExpBuffer(&workBuf,
libpq_gettext("SCHEMA NAME: %s\n"), val);
val = PQresultErrorField(res, PG_DIAG_TABLE_NAME);
if (val)
! appendPQExpBuffer(&workBuf,
libpq_gettext("TABLE NAME: %s\n"), val);
val = PQresultErrorField(res, PG_DIAG_COLUMN_NAME);
if (val)
! appendPQExpBuffer(&workBuf,
libpq_gettext("COLUMN NAME: %s\n"), val);
val = PQresultErrorField(res, PG_DIAG_DATATYPE_NAME);
if (val)
! appendPQExpBuffer(&workBuf,
libpq_gettext("DATATYPE NAME: %s\n"), val);
val = PQresultErrorField(res, PG_DIAG_CONSTRAINT_NAME);
if (val)
! appendPQExpBuffer(&workBuf,
libpq_gettext("CONSTRAINT NAME: %s\n"), val);
}
! if (conn->verbosity == PQERRORS_VERBOSE)
{
const char *valf;
const char *vall;
--- 1045,1104 ----
{
/* emit position as text addition to primary message */
/* translator: %s represents a digit string */
! appendPQExpBuffer(msg, libpq_gettext(" at character %s"),
val);
}
}
}
! appendPQExpBufferChar(msg, '\n');
! if (verbosity != PQERRORS_TERSE)
{
if (querytext && querypos > 0)
! reportErrorPosition(msg, querytext, querypos,
! res->client_encoding);
val = PQresultErrorField(res, PG_DIAG_MESSAGE_DETAIL);
if (val)
! appendPQExpBuffer(msg, libpq_gettext("DETAIL: %s\n"), val);
val = PQresultErrorField(res, PG_DIAG_MESSAGE_HINT);
if (val)
! appendPQExpBuffer(msg, libpq_gettext("HINT: %s\n"), val);
val = PQresultErrorField(res, PG_DIAG_INTERNAL_QUERY);
if (val)
! appendPQExpBuffer(msg, libpq_gettext("QUERY: %s\n"), val);
! if (show_context == PQSHOW_CONTEXT_ALWAYS ||
! (show_context == PQSHOW_CONTEXT_ERRORS &&
! res->resultStatus == PGRES_FATAL_ERROR))
{
val = PQresultErrorField(res, PG_DIAG_CONTEXT);
if (val)
! appendPQExpBuffer(msg, libpq_gettext("CONTEXT: %s\n"),
val);
}
}
! if (verbosity == PQERRORS_VERBOSE)
{
val = PQresultErrorField(res, PG_DIAG_SCHEMA_NAME);
if (val)
! appendPQExpBuffer(msg,
libpq_gettext("SCHEMA NAME: %s\n"), val);
val = PQresultErrorField(res, PG_DIAG_TABLE_NAME);
if (val)
! appendPQExpBuffer(msg,
libpq_gettext("TABLE NAME: %s\n"), val);
val = PQresultErrorField(res, PG_DIAG_COLUMN_NAME);
if (val)
! appendPQExpBuffer(msg,
libpq_gettext("COLUMN NAME: %s\n"), val);
val = PQresultErrorField(res, PG_DIAG_DATATYPE_NAME);
if (val)
! appendPQExpBuffer(msg,
libpq_gettext("DATATYPE NAME: %s\n"), val);
val = PQresultErrorField(res, PG_DIAG_CONSTRAINT_NAME);
if (val)
! appendPQExpBuffer(msg,
libpq_gettext("CONSTRAINT NAME: %s\n"), val);
}
! if (verbosity == PQERRORS_VERBOSE)
{
const char *valf;
const char *vall;
*************** pqGetErrorNotice3(PGconn *conn, bool isE
*** 1031,1081 ****
val = PQresultErrorField(res, PG_DIAG_SOURCE_FUNCTION);
if (val || valf || vall)
{
! appendPQExpBufferStr(&workBuf, libpq_gettext("LOCATION: "));
if (val)
! appendPQExpBuffer(&workBuf, libpq_gettext("%s, "), val);
if (valf && vall) /* unlikely we'd have just one */
! appendPQExpBuffer(&workBuf, libpq_gettext("%s:%s"),
valf, vall);
! appendPQExpBufferChar(&workBuf, '\n');
! }
! }
!
! /*
! * Either save error as current async result, or just emit the notice.
! */
! if (isError)
! {
! if (res)
! res->errMsg = pqResultStrdup(res, workBuf.data);
! pqClearAsyncResult(conn);
! conn->result = res;
! if (PQExpBufferDataBroken(workBuf))
! printfPQExpBuffer(&conn->errorMessage,
! libpq_gettext("out of memory"));
! else
! appendPQExpBufferStr(&conn->errorMessage, workBuf.data);
! }
! else
! {
! /* if we couldn't allocate the result set, just discard the NOTICE */
! if (res)
! {
! /* We can cheat a little here and not copy the message. */
! res->errMsg = workBuf.data;
! if (res->noticeHooks.noticeRec != NULL)
! (*res->noticeHooks.noticeRec) (res->noticeHooks.noticeRecArg, res);
! PQclear(res);
}
}
-
- termPQExpBuffer(&workBuf);
- return 0;
-
- fail:
- PQclear(res);
- termPQExpBuffer(&workBuf);
- return EOF;
}
/*
--- 1108,1122 ----
val = PQresultErrorField(res, PG_DIAG_SOURCE_FUNCTION);
if (val || valf || vall)
{
! appendPQExpBufferStr(msg, libpq_gettext("LOCATION: "));
if (val)
! appendPQExpBuffer(msg, libpq_gettext("%s, "), val);
if (valf && vall) /* unlikely we'd have just one */
! appendPQExpBuffer(msg, libpq_gettext("%s:%s"),
valf, vall);
! appendPQExpBufferChar(msg, '\n');
}
}
}
/*
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index 6bf34b3..9ca0756 100644
*** a/src/interfaces/libpq/libpq-fe.h
--- b/src/interfaces/libpq/libpq-fe.h
*************** extern PGresult *PQfn(PGconn *conn,
*** 463,468 ****
--- 463,471 ----
extern ExecStatusType PQresultStatus(const PGresult *res);
extern char *PQresStatus(ExecStatusType status);
extern char *PQresultErrorMessage(const PGresult *res);
+ extern char *PQresultVerboseErrorMessage(const PGresult *res,
+ PGVerbosity verbosity,
+ PGContextVisibility show_context);
extern char *PQresultErrorField(const PGresult *res, int fieldcode);
extern int PQntuples(const PGresult *res);
extern int PQnfields(const PGresult *res);
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 6c9bbf7..1183323 100644
*** a/src/interfaces/libpq/libpq-int.h
--- b/src/interfaces/libpq/libpq-int.h
*************** struct pg_result
*** 197,202 ****
--- 197,203 ----
*/
char *errMsg; /* error message, or NULL if no error */
PGMessageField *errFields; /* message broken into fields */
+ char *errQuery; /* text of triggering query, if available */
/* All NULL attributes in the query result point to this null string */
char null_field[1];
*************** extern char *pqBuildStartupPacket3(PGcon
*** 575,580 ****
--- 576,583 ----
const PQEnvironmentOption *options);
extern void pqParseInput3(PGconn *conn);
extern int pqGetErrorNotice3(PGconn *conn, bool isError);
+ extern void pqBuildErrorMessage3(PQExpBuffer msg, const PGresult *res,
+ PGVerbosity verbosity, PGContextVisibility show_context);
extern int pqGetCopyData3(PGconn *conn, char **buffer, int async);
extern int pqGetline3(PGconn *conn, char *s, int maxlen);
extern int pqGetlineAsync3(PGconn *conn, char *buffer, int bufsize);
I wrote:
> Attached is a libpq-portion-only version of a patch doing it this way.
> I've not yet looked at the psql part of your patch.
Here's an update for the psql side.
regards, tom lane
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 385cb59..330127a 100644
*** a/doc/src/sgml/ref/psql-ref.sgml
--- b/doc/src/sgml/ref/psql-ref.sgml
*************** Tue Oct 26 21:40:57 CEST 1999
*** 1718,1723 ****
--- 1718,1737 ----
<varlistentry>
+ <term><literal>\errverbose</literal></term>
+
+ <listitem>
+ <para>
+ Repeats the most recent server error or notice message at maximum
+ verbosity, as though <varname>VERBOSITY</varname> were set
+ to <literal>verbose</> and <varname>SHOW_CONTEXT</varname> were
+ set to <literal>always</>.
+ </para>
+ </listitem>
+ </varlistentry>
+
+
+ <varlistentry>
<term><literal>\f [ <replaceable class="parameter">string</replaceable> ]</literal></term>
<listitem>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 50dc43b..3401b51 100644
*** a/src/bin/psql/command.c
--- b/src/bin/psql/command.c
*************** exec_command(const char *cmd,
*** 822,827 ****
--- 822,849 ----
}
}
+ /* \errverbose -- display verbose message from last failed query */
+ else if (strcmp(cmd, "errverbose") == 0)
+ {
+ if (pset.last_error_result)
+ {
+ char *msg;
+
+ msg = PQresultVerboseErrorMessage(pset.last_error_result,
+ PQERRORS_VERBOSE,
+ PQSHOW_CONTEXT_ALWAYS);
+ if (msg)
+ {
+ psql_error("%s", msg);
+ PQfreemem(msg);
+ }
+ else
+ puts(_("out of memory"));
+ }
+ else
+ puts(_("There was no previous error."));
+ }
+
/* \f -- change field separator */
else if (strcmp(cmd, "f") == 0)
{
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 892058e..a2a07fb 100644
*** a/src/bin/psql/common.c
--- b/src/bin/psql/common.c
*************** AcceptResult(const PGresult *result)
*** 497,502 ****
--- 497,529 ----
}
+ /*
+ * ClearOrSaveResult
+ *
+ * If the result represents an error, remember it for possible display by
+ * \errverbose. Otherwise, just PQclear() it.
+ */
+ static void
+ ClearOrSaveResult(PGresult *result)
+ {
+ if (result)
+ {
+ switch (PQresultStatus(result))
+ {
+ case PGRES_NONFATAL_ERROR:
+ case PGRES_FATAL_ERROR:
+ if (pset.last_error_result)
+ PQclear(pset.last_error_result);
+ pset.last_error_result = result;
+ break;
+
+ default:
+ PQclear(result);
+ break;
+ }
+ }
+ }
+
/*
* PSQLexec
*************** PSQLexec(const char *query)
*** 548,554 ****
if (!AcceptResult(res))
{
! PQclear(res);
res = NULL;
}
--- 575,581 ----
if (!AcceptResult(res))
{
! ClearOrSaveResult(res);
res = NULL;
}
*************** PSQLexecWatch(const char *query, const p
*** 590,596 ****
if (!AcceptResult(res))
{
! PQclear(res);
return 0;
}
--- 617,623 ----
if (!AcceptResult(res))
{
! ClearOrSaveResult(res);
return 0;
}
*************** SendQuery(const char *query)
*** 1077,1087 ****
if (PQresultStatus(results) != PGRES_COMMAND_OK)
{
psql_error("%s", PQerrorMessage(pset.db));
! PQclear(results);
ResetCancelConn();
goto sendquery_cleanup;
}
! PQclear(results);
transaction_status = PQtransactionStatus(pset.db);
}
--- 1104,1114 ----
if (PQresultStatus(results) != PGRES_COMMAND_OK)
{
psql_error("%s", PQerrorMessage(pset.db));
! ClearOrSaveResult(results);
ResetCancelConn();
goto sendquery_cleanup;
}
! ClearOrSaveResult(results);
transaction_status = PQtransactionStatus(pset.db);
}
*************** SendQuery(const char *query)
*** 1102,1112 ****
if (PQresultStatus(results) != PGRES_COMMAND_OK)
{
psql_error("%s", PQerrorMessage(pset.db));
! PQclear(results);
ResetCancelConn();
goto sendquery_cleanup;
}
! PQclear(results);
on_error_rollback_savepoint = true;
}
}
--- 1129,1139 ----
if (PQresultStatus(results) != PGRES_COMMAND_OK)
{
psql_error("%s", PQerrorMessage(pset.db));
! ClearOrSaveResult(results);
ResetCancelConn();
goto sendquery_cleanup;
}
! ClearOrSaveResult(results);
on_error_rollback_savepoint = true;
}
}
*************** SendQuery(const char *query)
*** 1202,1208 ****
if (PQresultStatus(svptres) != PGRES_COMMAND_OK)
{
psql_error("%s", PQerrorMessage(pset.db));
! PQclear(svptres);
OK = false;
PQclear(results);
--- 1229,1235 ----
if (PQresultStatus(svptres) != PGRES_COMMAND_OK)
{
psql_error("%s", PQerrorMessage(pset.db));
! ClearOrSaveResult(svptres);
OK = false;
PQclear(results);
*************** SendQuery(const char *query)
*** 1213,1219 ****
}
}
! PQclear(results);
/* Possible microtiming output */
if (pset.timing)
--- 1240,1246 ----
}
}
! ClearOrSaveResult(results);
/* Possible microtiming output */
if (pset.timing)
*************** ExecQueryUsingCursor(const char *query,
*** 1299,1305 ****
results = PQexec(pset.db, "BEGIN");
OK = AcceptResult(results) &&
(PQresultStatus(results) == PGRES_COMMAND_OK);
! PQclear(results);
if (!OK)
return false;
started_txn = true;
--- 1326,1332 ----
results = PQexec(pset.db, "BEGIN");
OK = AcceptResult(results) &&
(PQresultStatus(results) == PGRES_COMMAND_OK);
! ClearOrSaveResult(results);
if (!OK)
return false;
started_txn = true;
*************** ExecQueryUsingCursor(const char *query,
*** 1313,1319 ****
results = PQexec(pset.db, buf.data);
OK = AcceptResult(results) &&
(PQresultStatus(results) == PGRES_COMMAND_OK);
! PQclear(results);
termPQExpBuffer(&buf);
if (!OK)
goto cleanup;
--- 1340,1346 ----
results = PQexec(pset.db, buf.data);
OK = AcceptResult(results) &&
(PQresultStatus(results) == PGRES_COMMAND_OK);
! ClearOrSaveResult(results);
termPQExpBuffer(&buf);
if (!OK)
goto cleanup;
*************** ExecQueryUsingCursor(const char *query,
*** 1384,1390 ****
OK = AcceptResult(results);
Assert(!OK);
! PQclear(results);
break;
}
--- 1411,1417 ----
OK = AcceptResult(results);
Assert(!OK);
! ClearOrSaveResult(results);
break;
}
*************** ExecQueryUsingCursor(const char *query,
*** 1392,1398 ****
{
/* StoreQueryTuple will complain if not exactly one row */
OK = StoreQueryTuple(results);
! PQclear(results);
break;
}
--- 1419,1425 ----
{
/* StoreQueryTuple will complain if not exactly one row */
OK = StoreQueryTuple(results);
! ClearOrSaveResult(results);
break;
}
*************** ExecQueryUsingCursor(const char *query,
*** 1415,1421 ****
printQuery(results, &my_popt, fout, is_pager, pset.logfile);
! PQclear(results);
/* after the first result set, disallow header decoration */
my_popt.topt.start_table = false;
--- 1442,1448 ----
printQuery(results, &my_popt, fout, is_pager, pset.logfile);
! ClearOrSaveResult(results);
/* after the first result set, disallow header decoration */
my_popt.topt.start_table = false;
*************** cleanup:
*** 1473,1486 ****
OK = AcceptResult(results) &&
(PQresultStatus(results) == PGRES_COMMAND_OK);
}
! PQclear(results);
if (started_txn)
{
results = PQexec(pset.db, OK ? "COMMIT" : "ROLLBACK");
OK &= AcceptResult(results) &&
(PQresultStatus(results) == PGRES_COMMAND_OK);
! PQclear(results);
}
if (pset.timing)
--- 1500,1513 ----
OK = AcceptResult(results) &&
(PQresultStatus(results) == PGRES_COMMAND_OK);
}
! ClearOrSaveResult(results);
if (started_txn)
{
results = PQexec(pset.db, OK ? "COMMIT" : "ROLLBACK");
OK &= AcceptResult(results) &&
(PQresultStatus(results) == PGRES_COMMAND_OK);
! ClearOrSaveResult(results);
}
if (pset.timing)
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 59f6f25..c6f0993 100644
*** a/src/bin/psql/help.c
--- b/src/bin/psql/help.c
*************** slashUsage(unsigned short int pager)
*** 168,177 ****
* Use "psql --help=commands | wc" to count correctly. It's okay to count
* the USE_READLINE line even in builds without that.
*/
! output = PageOutput(109, pager ? &(pset.popt.topt) : NULL);
fprintf(output, _("General\n"));
fprintf(output, _(" \\copyright show PostgreSQL usage and distribution terms\n"));
fprintf(output, _(" \\g [FILE] or ; execute query (and send results to file or |pipe)\n"));
fprintf(output, _(" \\gset [PREFIX] execute query and store results in psql variables\n"));
fprintf(output, _(" \\q quit psql\n"));
--- 168,178 ----
* Use "psql --help=commands | wc" to count correctly. It's okay to count
* the USE_READLINE line even in builds without that.
*/
! output = PageOutput(110, pager ? &(pset.popt.topt) : NULL);
fprintf(output, _("General\n"));
fprintf(output, _(" \\copyright show PostgreSQL usage and distribution terms\n"));
+ fprintf(output, _(" \\errverbose show most recent error message at maximum verbosity\n"));
fprintf(output, _(" \\g [FILE] or ; execute query (and send results to file or |pipe)\n"));
fprintf(output, _(" \\gset [PREFIX] execute query and store results in psql variables\n"));
fprintf(output, _(" \\q quit psql\n"));
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
index 159a7a5..ae30b2e 100644
*** a/src/bin/psql/settings.h
--- b/src/bin/psql/settings.h
*************** typedef struct _psqlSettings
*** 86,91 ****
--- 86,93 ----
FILE *copyStream; /* Stream to read/write for \copy command */
+ PGresult *last_error_result; /* most recent error result, if any */
+
printQueryOpt popt;
char *gfname; /* one-shot file output argument for \g */
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index 07e94d0..b96cdc4 100644
*** a/src/bin/psql/startup.c
--- b/src/bin/psql/startup.c
*************** main(int argc, char *argv[])
*** 135,140 ****
--- 135,141 ----
pset.queryFout = stdout;
pset.queryFoutPipe = false;
pset.copyStream = NULL;
+ pset.last_error_result = NULL;
pset.cur_cmd_source = stdin;
pset.cur_cmd_interactive = false;
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index eb592bb..688d92a 100644
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
*************** psql_completion(const char *text, int st
*** 1280,1286 ****
"\\dF", "\\dFd", "\\dFp", "\\dFt", "\\dg", "\\di", "\\dl", "\\dL",
"\\dm", "\\dn", "\\do", "\\dO", "\\dp", "\\drds", "\\ds", "\\dS",
"\\dt", "\\dT", "\\dv", "\\du", "\\dx", "\\dy",
! "\\e", "\\echo", "\\ef", "\\encoding", "\\ev",
"\\f", "\\g", "\\gset", "\\h", "\\help", "\\H", "\\i", "\\ir", "\\l",
"\\lo_import", "\\lo_export", "\\lo_list", "\\lo_unlink",
"\\o", "\\p", "\\password", "\\prompt", "\\pset", "\\q", "\\qecho", "\\r",
--- 1280,1286 ----
"\\dF", "\\dFd", "\\dFp", "\\dFt", "\\dg", "\\di", "\\dl", "\\dL",
"\\dm", "\\dn", "\\do", "\\dO", "\\dp", "\\drds", "\\ds", "\\dS",
"\\dt", "\\dT", "\\dv", "\\du", "\\dx", "\\dy",
! "\\e", "\\echo", "\\ef", "\\encoding", "\\errverbose", "\\ev",
"\\f", "\\g", "\\gset", "\\h", "\\help", "\\H", "\\i", "\\ir", "\\l",
"\\lo_import", "\\lo_export", "\\lo_list", "\\lo_unlink",
"\\o", "\\p", "\\password", "\\prompt", "\\pset", "\\q", "\\qecho", "\\r",
On Thu, Feb 11, 2016 at 9:50 AM, Robert Haas <robertmhaas@gmail.com> wrote: >> Actually, what'd be really handy IMO is something to regurgitate the >> most recent error in verbose mode, without making a permanent session >> state change. Something like >> >> regression=# insert into bar values(1); >> ERROR: insert or update on table "bar" violates foreign key constraint "bar_f1_fkey" >> DETAIL: Key (f1)=(1) is not present in table "foo". >> regression=# \saywhat >> ERROR: 23503: insert or update on table "bar" violates foreign key constraint "bar_f1_fkey" >> DETAIL: Key (f1)=(1) is not present in table "foo". >> SCHEMA NAME: public >> TABLE NAME: bar >> CONSTRAINT NAME: bar_f1_fkey >> LOCATION: ri_ReportViolation, ri_triggers.c:3326 >> regression=# > > Wow, that's a fabulous idea. I see Oleksandr has tried to implement > it, although I haven't looked at the patch. But I think this would be > REALLY helpful. +1 -- Peter Geoghegan
On Sat, Apr 2, 2016 at 11:41 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
"Shulgin, Oleksandr" <oleksandr.shulgin@zalando.de> writes:
> On Mon, Mar 14, 2016 at 7:55 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Yeah, I don't much like that either. But I don't think we can avoid
>> some refactoring there; as designed, conversion of an error message into
>> user-visible form is too tightly tied to receipt of the message.
> True. Attached is a v2 which addresses all of the points raised earlier I
> believe.
I took a closer look at what's going on here and realized that actually
it's not that hard to decouple the message-building routine from the
PGconn state, because mostly it works with fields it's extracting out
of the PGresult anyway. The only piece of information that's lacking
is conn->last_query. I propose therefore that instead of doing it like
this, we copy last_query into error PGresults. This is strictly less
added storage requirement than storing the whole verbose message would be,
and it saves time compared to the v2 patch in the typical case where
the application never does ask for an alternately-formatted error message.
Plus we can actually support requests for any variant format, not only
VERBOSE.
Attached is a libpq-portion-only version of a patch doing it this way.
I've not yet looked at the psql part of your patch.
Comments?
Ah, neat, that's even better. :-)
What about regression tests? My assumption was that we won't be able to add them with the usual expected file approach, but that we also don't need it that hard. Everyone's in favor?
--
Alex
Alex Shulgin <alex.shulgin@gmail.com> writes:
> What about regression tests? My assumption was that we won't be able to
> add them with the usual expected file approach, but that we also don't need
> it that hard. Everyone's in favor?
It'd be nice to have a regression test, but I concur that the LOCATION
information is too unstable for that to be practical.
We could imagine testing some variant behavior that omits printing
LOCATION, but that doesn't really seem worth the trouble; I think
we'd be inventing the variant behavior only for testing purposes.
regards, tom lane
Peter Geoghegan <pg@heroku.com> writes:
> On Thu, Feb 11, 2016 at 9:50 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>> Wow, that's a fabulous idea. I see Oleksandr has tried to implement
>> it, although I haven't looked at the patch. But I think this would be
>> REALLY helpful.
> +1
Pushed.
regards, tom lane
On Sun, Apr 3, 2016, 18:32 Tom Lane <tgl@sss.pgh.pa.us> wrote:
Peter Geoghegan <pg@heroku.com> writes:
> On Thu, Feb 11, 2016 at 9:50 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>> Wow, that's a fabulous idea. I see Oleksandr has tried to implement
>> it, although I haven't looked at the patch. But I think this would be
>> REALLY helpful.
> +1
Pushed.
Yay!