[proposal] Add an option for returning SQLSTATE in psql error message
Hi,
Currently on error psql is printing Postgres' PQerrorMessage text, but
there's no guarantee these messages are constant between Postgres
versions and it's a pain when using psql for writing regression tests,
Prior
Disallow setting client_min_messages higher than ERROR.
a bad workaround was to discarded error, now you have to do something like
CREATE OR REPLACE FUNCTION catch_error(
query text
)
RETURNS void AS $$
DECLARE
BEGIN
EXECUTE query;
EXCEPTION WHEN OTHERS THEN
RAISE 'Query failed: %', SQLSTATE;
END;
$$LANGUAGE plpgsql;
SELECT catch_error('foo');
What about a new \whatever for setting psql error to either PQerrorMessage or
PQresultErrorField(res, PG_DIAG_SQLSTATE) if available?
ne 2. 12. 2018 v 15:34 odesílatel didier <did447@gmail.com> napsal:
Hi,
Currently on error psql is printing Postgres' PQerrorMessage text, but
there's no guarantee these messages are constant between Postgres
versions and it's a pain when using psql for writing regression tests,Prior
Disallow setting client_min_messages higher than ERROR.a bad workaround was to discarded error, now you have to do something like
CREATE OR REPLACE FUNCTION catch_error(
query text
)
RETURNS void AS $$
DECLARE
BEGIN
EXECUTE query;
EXCEPTION WHEN OTHERS THEN
RAISE 'Query failed: %', SQLSTATE;
END;
$$LANGUAGE plpgsql;SELECT catch_error('foo');
What about a new \whatever for setting psql error to either PQerrorMessage
or
PQresultErrorField(res, PG_DIAG_SQLSTATE) if available?
It looks weird. Maybe we can define a option where only SQL state will be
displayed.
Regards
Pavel
didier <did447@gmail.com> writes:
Currently on error psql is printing Postgres' PQerrorMessage text, but
there's no guarantee these messages are constant between Postgres
versions and it's a pain when using psql for writing regression tests,
I don't buy that argument. We use psql's normal display in all the
regular regression tests, and it's not a big maintenance problem.
If the error message changes, that's usually interesting in itself.
Also, if it does change, it's often because you're hitting a different
error-detection test, which more than likely is throwing a different
SQLSTATE anyway.
Lastly, because the SQL spec has been rather miserly in assigning
SQLSTATEs in some areas, there are lots of cases where the same
SQLSTATE is used for several distinct errors; for instance
ERRCODE_UNDEFINED_OBJECT covers a lot of ground. Do you really
want your test cases to be unable to distinguish those?
There might be a use-case for showing only SQLSTATE, but writing
regression tests doesn't seem like a good example.
regards, tom lane
ne 2. 12. 2018 v 16:56 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:
didier <did447@gmail.com> writes:
Currently on error psql is printing Postgres' PQerrorMessage text, but
there's no guarantee these messages are constant between Postgres
versions and it's a pain when using psql for writing regression tests,I don't buy that argument. We use psql's normal display in all the
regular regression tests, and it's not a big maintenance problem.
If the error message changes, that's usually interesting in itself.
our tests are not against different PostgreSQL releases. When you have a
code, that should to support more PostgreSQL releases, then it is sometimes
difficult.
Show quoted text
Also, if it does change, it's often because you're hitting a different
error-detection test, which more than likely is throwing a different
SQLSTATE anyway.Lastly, because the SQL spec has been rather miserly in assigning
SQLSTATEs in some areas, there are lots of cases where the same
SQLSTATE is used for several distinct errors; for instance
ERRCODE_UNDEFINED_OBJECT covers a lot of ground. Do you really
want your test cases to be unable to distinguish those?There might be a use-case for showing only SQLSTATE, but writing
regression tests doesn't seem like a good example.regards, tom lane
Currently with 10 head
ERROR: could not determine polymorphic type because input has type unknown
With 9 head
ERROR: could not determine polymorphic type because input has type "unknown"
Another option could be: set display error to none and let user's
script do some regular expression on pdsl variable.
Show quoted text
On Sun, Dec 2, 2018 at 5:05 PM Pavel Stehule <pavel.stehule@gmail.com> wrote:
ne 2. 12. 2018 v 16:56 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:
didier <did447@gmail.com> writes:
Currently on error psql is printing Postgres' PQerrorMessage text, but
there's no guarantee these messages are constant between Postgres
versions and it's a pain when using psql for writing regression tests,I don't buy that argument. We use psql's normal display in all the
regular regression tests, and it's not a big maintenance problem.
If the error message changes, that's usually interesting in itself.our tests are not against different PostgreSQL releases. When you have a code, that should to support more PostgreSQL releases, then it is sometimes difficult.
Also, if it does change, it's often because you're hitting a different
error-detection test, which more than likely is throwing a different
SQLSTATE anyway.Lastly, because the SQL spec has been rather miserly in assigning
SQLSTATEs in some areas, there are lots of cases where the same
SQLSTATE is used for several distinct errors; for instance
ERRCODE_UNDEFINED_OBJECT covers a lot of ground. Do you really
want your test cases to be unable to distinguish those?There might be a use-case for showing only SQLSTATE, but writing
regression tests doesn't seem like a good example.regards, tom lane
"Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:
Tom> I don't buy that argument. We use psql's normal display in all the
Tom> regular regression tests, and it's not a big maintenance problem.
The regular regression tests have the advantage that they don't need to
work across pg versions.
It is more of a problem for extensions; I just ran into this myself in
fact, with a test failing because "invalid input syntax for integer" got
changed to "invalid input syntax for type integer".
--
Andrew (irc:RhodiumToad)
Attached a POC adding a new variable ECHO_ERROR
\set ECHO_ERROR text|none|psqlstate
On Mon, Dec 3, 2018 at 2:47 AM Andrew Gierth
<andrew@tao11.riddles.org.uk> wrote:
Show quoted text
"Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:
Tom> I don't buy that argument. We use psql's normal display in all the
Tom> regular regression tests, and it's not a big maintenance problem.The regular regression tests have the advantage that they don't need to
work across pg versions.It is more of a problem for extensions; I just ran into this myself in
fact, with a test failing because "invalid input syntax for integer" got
changed to "invalid input syntax for type integer".--
Andrew (irc:RhodiumToad)
Attachments:
master_psql_error_output.difftext/x-patch; charset=US-ASCII; name=master_psql_error_output.diffDownload
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 62c2928e6b..049d43086f 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -536,10 +536,22 @@ AcceptResult(const PGresult *result)
if (!OK)
{
- const char *error = PQerrorMessage(pset.db);
+ const char *error = "";
+ const char *err_fmt = "%s";
+ switch (pset.echo_error) {
+ case PSQL_ECHO_ERROR_NONE:
+ break;
+ case PSQL_ECHO_ERROR_PSQLSTATE:
+ err_fmt = "%s\n";
+ error = PQresultErrorField(result, PG_DIAG_SQLSTATE);
+ break;
+ default:
+ error = PQerrorMessage(pset.db);
+ break;
+ }
if (strlen(error))
- psql_error("%s", error);
+ psql_error(err_fmt, error);
CheckConnection();
}
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 2e9fe760eb..b653e37526 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -339,7 +339,7 @@ helpVariables(unsigned short int pager)
* Windows builds currently print one more line than non-Windows builds.
* Using the larger number is fine.
*/
- output = PageOutput(156, pager ? &(pset.popt.topt) : NULL);
+ output = PageOutput(159, pager ? &(pset.popt.topt) : NULL);
fprintf(output, _("List of specially treated variables\n\n"));
@@ -360,6 +360,9 @@ helpVariables(unsigned short int pager)
fprintf(output, _(" ECHO_HIDDEN\n"
" if set, display internal queries executed by backslash commands;\n"
" if set to \"noexec\", just show them without execution\n"));
+ fprintf(output, _(" ECHO_ERROR\n"
+ " controls what error msg is written to standard error\n"
+ " [text, sqlstate, none]\n"));
fprintf(output, _(" ENCODING\n"
" current client character set encoding\n"));
fprintf(output, _(" ERROR\n"
@@ -411,7 +414,7 @@ helpVariables(unsigned short int pager)
fprintf(output, _(" USER\n"
" the currently connected database user\n"));
fprintf(output, _(" VERBOSITY\n"
- " controls verbosity of error reports [default, verbose, terse]\n"));
+ " controls verbosity of server error reports [default, verbose, terse]\n"));
fprintf(output, _(" VERSION\n"
" VERSION_NAME\n"
" VERSION_NUM\n"
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
index 176c85afd0..dd1f5f2c55 100644
--- a/src/bin/psql/settings.h
+++ b/src/bin/psql/settings.h
@@ -48,6 +48,13 @@ typedef enum
PSQL_ECHO_HIDDEN_NOEXEC
} PSQL_ECHO_HIDDEN;
+typedef enum
+{
+ PSQL_ECHO_ERROR_NONE,
+ PSQL_ECHO_ERROR_TEXT,
+ PSQL_ECHO_ERROR_PSQLSTATE
+} PSQL_ECHO_ERROR;
+
typedef enum
{
PSQL_ERROR_ROLLBACK_OFF,
@@ -132,6 +139,7 @@ typedef struct _psqlSettings
int ignoreeof;
PSQL_ECHO echo;
PSQL_ECHO_HIDDEN echo_hidden;
+ PSQL_ECHO_ERROR echo_error;
PSQL_ERROR_ROLLBACK on_error_rollback;
PSQL_COMP_CASE comp_case;
HistControl histcontrol;
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index e7536a8a06..156b4665ac 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -949,6 +949,33 @@ echo_hook(const char *newval)
return true;
}
+static char *
+echo_error_substitute_hook(char *newval)
+{
+ if (newval == NULL)
+ newval = pg_strdup("text");
+ return newval;
+}
+
+static bool
+echo_error_hook(const char *newval)
+{
+ Assert(newval != NULL); /* else substitute hook messed up */
+ if (pg_strcasecmp(newval, "text") == 0)
+ pset.echo_error = PSQL_ECHO_ERROR_TEXT;
+ else if (pg_strcasecmp(newval, "none") == 0)
+ pset.echo_error = PSQL_ECHO_ERROR_NONE;
+ else if (pg_strcasecmp(newval, "psqlstate") == 0)
+ pset.echo_error = PSQL_ECHO_ERROR_PSQLSTATE;
+ else
+ {
+ PsqlVarEnumError("ECHO_ERROR", newval, "none, psqlstate, text");
+ return false;
+ }
+ return true;
+}
+
+
static bool
echo_hidden_hook(const char *newval)
{
@@ -1167,6 +1194,9 @@ EstablishVariableSpace(void)
SetVariableHooks(pset.vars, "ECHO_HIDDEN",
bool_substitute_hook,
echo_hidden_hook);
+ SetVariableHooks(pset.vars, "ECHO_ERROR",
+ echo_error_substitute_hook,
+ echo_error_hook);
SetVariableHooks(pset.vars, "ON_ERROR_ROLLBACK",
bool_substitute_hook,
on_error_rollback_hook);
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index fa44b2820b..bf23c1f4bb 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -3561,6 +3561,8 @@ psql_completion(const char *text, int start, int end)
"preserve-lower", "preserve-upper");
else if (TailMatchesCS("ECHO"))
COMPLETE_WITH_CS("errors", "queries", "all", "none");
+ else if (TailMatchesCS("ECHO_ERROR"))
+ COMPLETE_WITH_CS("text", "sqlstate", "none");
else if (TailMatchesCS("ECHO_HIDDEN"))
COMPLETE_WITH_CS("noexec", "off", "on");
else if (TailMatchesCS("HISTCONTROL"))
"didier" == didier <did447@gmail.com> writes:
didier> Attached a POC adding a new variable ECHO_ERROR
didier> \set ECHO_ERROR text|none|psqlstate
I wouldn't have called it that. Possibly another option to the existing
VERBOSITY variable? \set VERBOSITY sqlstate_only or something of that
ilk (it's already not unusual to use \set VERBOSITY terse in regression
tests)
--
Andrew (irc:RhodiumToad)
po 3. 12. 2018 v 16:49 odesílatel Andrew Gierth <andrew@tao11.riddles.org.uk>
napsal:
"didier" == didier <did447@gmail.com> writes:
didier> Attached a POC adding a new variable ECHO_ERROR
didier> \set ECHO_ERROR text|none|psqlstateI wouldn't have called it that. Possibly another option to the existing
VERBOSITY variable? \set VERBOSITY sqlstate_only or something of that
ilk (it's already not unusual to use \set VERBOSITY terse in regression
tests)
It is good idea to look to this option.
Pavel
Show quoted text
--
Andrew (irc:RhodiumToad)
Yep, name is bad, but I'm not sure about VERBOSITY, isn't it
controlling output from the server not the client?
You may want to set both VERBOSITY to 'verbose' and ECHO_ERROR to
'none' then in script do
SELECT .... -- no error output
\if :ERROR
-- do something with LAST_ERROR_MESSAGE
On Mon, Dec 3, 2018 at 4:49 PM Andrew Gierth
<andrew@tao11.riddles.org.uk> wrote:
Show quoted text
"didier" == didier <did447@gmail.com> writes:
didier> Attached a POC adding a new variable ECHO_ERROR
didier> \set ECHO_ERROR text|none|psqlstateI wouldn't have called it that. Possibly another option to the existing
VERBOSITY variable? \set VERBOSITY sqlstate_only or something of that
ilk (it's already not unusual to use \set VERBOSITY terse in regression
tests)--
Andrew (irc:RhodiumToad)
didier <did447@gmail.com> writes:
Yep, name is bad, but I'm not sure about VERBOSITY, isn't it
controlling output from the server not the client?
No, it's in libpq, so you'd have to touch that not the server.
I agree with Andrew's thought, and would further say that just
"\set VERBOSITY sqlstate" would be a reasonable API. Making this
separate from VERBOSITY is weird.
regards, tom lane
On Mon, Dec 3, 2018 at 5:51 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
No, it's in libpq, so you'd have to touch that not the server.
libpq, not as bad as the server but nonetheless maybe a bit too much for this.
Is adding value in library contract?
Anyway. attached a POC adding a new value to VERBOSITY (hopefully
nobody is using it).
Attachments:
master_verbosity_sqlstate.difftext/x-patch; charset=US-ASCII; name=master_verbosity_sqlstate.diffDownload
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 2e9fe760eb..e0c3a4fb9f 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -411,7 +411,7 @@ helpVariables(unsigned short int pager)
fprintf(output, _(" USER\n"
" the currently connected database user\n"));
fprintf(output, _(" VERBOSITY\n"
- " controls verbosity of error reports [default, verbose, terse]\n"));
+ " controls verbosity of error reports [default, verbose, terse, sqlstate]\n"));
fprintf(output, _(" VERSION\n"
" VERSION_NAME\n"
" VERSION_NUM\n"
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index e7536a8a06..293ffcc5ef 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -1088,9 +1088,11 @@ verbosity_hook(const char *newval)
pset.verbosity = PQERRORS_TERSE;
else if (pg_strcasecmp(newval, "verbose") == 0)
pset.verbosity = PQERRORS_VERBOSE;
+ else if (pg_strcasecmp(newval, "sqlstate") == 0)
+ pset.verbosity = PQERRORS_SQLSTATE;
else
{
- PsqlVarEnumError("VERBOSITY", newval, "default, terse, verbose");
+ PsqlVarEnumError("VERBOSITY", newval, "default, terse, sqlstate, verbose");
return false;
}
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index fa44b2820b..3f0ad5192a 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -3571,7 +3571,7 @@ psql_completion(const char *text, int start, int end)
else if (TailMatchesCS("SHOW_CONTEXT"))
COMPLETE_WITH_CS("never", "errors", "always");
else if (TailMatchesCS("VERBOSITY"))
- COMPLETE_WITH_CS("default", "verbose", "terse");
+ COMPLETE_WITH_CS("default", "verbose", "terse", "sqlstate");
}
else if (TailMatchesCS("\\sf*"))
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_routines, NULL);
diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index 8345faface..c58eabcd10 100644
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -1017,6 +1017,15 @@ pqBuildErrorMessage3(PQExpBuffer msg, const PGresult *res,
val = PQresultErrorField(res, PG_DIAG_SEVERITY);
if (val)
appendPQExpBuffer(msg, "%s: ", val);
+ if (verbosity == PQERRORS_SQLSTATE)
+ {
+ val = PQresultErrorField(res, PG_DIAG_SQLSTATE);
+ if (val)
+ appendPQExpBuffer(msg, "%s", val);
+ appendPQExpBufferChar(msg, '\n');
+ return;
+ }
+
if (verbosity == PQERRORS_VERBOSE)
{
val = PQresultErrorField(res, PG_DIAG_SQLSTATE);
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index 3f13ddf092..f280a19b5a 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -111,7 +111,8 @@ typedef enum
{
PQERRORS_TERSE, /* single-line error messages */
PQERRORS_DEFAULT, /* recommended style */
- PQERRORS_VERBOSE /* all the facts, ma'am */
+ PQERRORS_VERBOSE, /* all the facts, ma'am */
+ PQERRORS_SQLSTATE /* single-line SQLSTATE error */
} PGVerbosity;
typedef enum
po 3. 12. 2018 v 18:57 odesílatel didier <did447@gmail.com> napsal:
On Mon, Dec 3, 2018 at 5:51 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
No, it's in libpq, so you'd have to touch that not the server.
libpq, not as bad as the server but nonetheless maybe a bit too much for
this.
Is adding value in library contract?Anyway. attached a POC adding a new value to VERBOSITY (hopefully
nobody is using it).
It can works :). Please, assign it to next commitfest.
Regards
Pavel
On Mon, Dec 3, 2018 at 7:02 PM Pavel Stehule <pavel.stehule@gmail.com> wrote:
It can works :). Please, assign it to next commitfest.
Ok
Attachments:
0001-Add-sqlstate-output-mode-to-VERBOSITY_v1.patchtext/x-patch; charset=US-ASCII; name=0001-Add-sqlstate-output-mode-to-VERBOSITY_v1.patchDownload
From e6d0261156838b07a5a253355552fa0121d6d5c5 Mon Sep 17 00:00:00 2001
From: didier <didier@users.sourceforge.net>
Date: Mon, 3 Dec 2018 19:20:54 +0100
Subject: [PATCH] Add sqlstate output mode to VERBOSITY
Returned messages include severity and SQLSTATE error code.
---
doc/src/sgml/libpq.sgml | 28 +++++++++++++++------------
doc/src/sgml/ref/psql-ref.sgml | 7 ++++---
src/bin/psql/help.c | 2 +-
src/bin/psql/startup.c | 4 +++-
src/bin/psql/tab-complete.c | 2 +-
src/interfaces/libpq/fe-protocol3.c | 9 +++++++++
src/interfaces/libpq/libpq-fe.h | 3 ++-
src/test/regress/expected/psql.out | 30 +++++++++++++++++++++++++++++
src/test/regress/sql/psql.sql | 19 ++++++++++++++++++
9 files changed, 85 insertions(+), 19 deletions(-)
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 2e9fe760eb..e0c3a4fb9f 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -411,7 +411,7 @@ helpVariables(unsigned short int pager)
fprintf(output, _(" USER\n"
" the currently connected database user\n"));
fprintf(output, _(" VERBOSITY\n"
- " controls verbosity of error reports [default, verbose, terse]\n"));
+ " controls verbosity of error reports [default, verbose, terse, sqlstate]\n"));
fprintf(output, _(" VERSION\n"
" VERSION_NAME\n"
" VERSION_NUM\n"
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index e7536a8a06..293ffcc5ef 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -1088,9 +1088,11 @@ verbosity_hook(const char *newval)
pset.verbosity = PQERRORS_TERSE;
else if (pg_strcasecmp(newval, "verbose") == 0)
pset.verbosity = PQERRORS_VERBOSE;
+ else if (pg_strcasecmp(newval, "sqlstate") == 0)
+ pset.verbosity = PQERRORS_SQLSTATE;
else
{
- PsqlVarEnumError("VERBOSITY", newval, "default, terse, verbose");
+ PsqlVarEnumError("VERBOSITY", newval, "default, terse, sqlstate, verbose");
return false;
}
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index fa44b2820b..3f0ad5192a 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -3571,7 +3571,7 @@ psql_completion(const char *text, int start, int end)
else if (TailMatchesCS("SHOW_CONTEXT"))
COMPLETE_WITH_CS("never", "errors", "always");
else if (TailMatchesCS("VERBOSITY"))
- COMPLETE_WITH_CS("default", "verbose", "terse");
+ COMPLETE_WITH_CS("default", "verbose", "terse", "sqlstate");
}
else if (TailMatchesCS("\\sf*"))
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_routines, NULL);
diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index 8345faface..c58eabcd10 100644
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -1017,6 +1017,15 @@ pqBuildErrorMessage3(PQExpBuffer msg, const PGresult *res,
val = PQresultErrorField(res, PG_DIAG_SEVERITY);
if (val)
appendPQExpBuffer(msg, "%s: ", val);
+ if (verbosity == PQERRORS_SQLSTATE)
+ {
+ val = PQresultErrorField(res, PG_DIAG_SQLSTATE);
+ if (val)
+ appendPQExpBuffer(msg, "%s", val);
+ appendPQExpBufferChar(msg, '\n');
+ return;
+ }
+
if (verbosity == PQERRORS_VERBOSE)
{
val = PQresultErrorField(res, PG_DIAG_SQLSTATE);
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index 3f13ddf092..f280a19b5a 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -111,7 +111,8 @@ typedef enum
{
PQERRORS_TERSE, /* single-line error messages */
PQERRORS_DEFAULT, /* recommended style */
- PQERRORS_VERBOSE /* all the facts, ma'am */
+ PQERRORS_VERBOSE, /* all the facts, ma'am */
+ PQERRORS_SQLSTATE /* single-line SQLSTATE error code */
} PGVerbosity;
typedef enum
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index 1bb2a6e16d..64628f29a3 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -1016,3 +1016,22 @@ select 1/(15-unique2) from tenk1 order by unique2 limit 19;
\echo 'last error code:' :LAST_ERROR_SQLSTATE
\unset FETCH_COUNT
+
+-- verbosity error setting
+\set VERBOSITY terse
+SELECT 1 UNION;
+\echo 'error:' :ERROR
+\echo 'error code:' :SQLSTATE
+\echo 'last error message:' :LAST_ERROR_MESSAGE
+
+\set VERBOSITY sqlstate
+SELECT 1 UNION;
+\echo 'error:' :ERROR
+\echo 'error code:' :SQLSTATE
+\echo 'last error message:' :LAST_ERROR_MESSAGE
+
+\set VERBOSITY default
+SELECT 1 UNION;
+\echo 'error:' :ERROR
+\echo 'error code:' :SQLSTATE
+\echo 'last error message:' :LAST_ERROR_MESSAGE
--
2.17.1
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index 775b127121..113d910202 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -4539,3 +4539,33 @@ last error message: division by zero
\echo 'last error code:' :LAST_ERROR_SQLSTATE
last error code: 22012
\unset FETCH_COUNT
+-- verbosity error setting
+\set VERBOSITY terse
+SELECT 1 UNION;
+ERROR: syntax error at or near ";" at character 15
+\echo 'error:' :ERROR
+error: true
+\echo 'error code:' :SQLSTATE
+error code: 42601
+\echo 'last error message:' :LAST_ERROR_MESSAGE
+last error message: syntax error at or near ";"
+\set VERBOSITY sqlstate
+SELECT 1 UNION;
+ERROR: 42601
+\echo 'error:' :ERROR
+error: true
+\echo 'error code:' :SQLSTATE
+error code: 42601
+\echo 'last error message:' :LAST_ERROR_MESSAGE
+last error message: syntax error at or near ";"
+\set VERBOSITY default
+SELECT 1 UNION;
+ERROR: syntax error at or near ";"
+LINE 1: SELECT 1 UNION;
+ ^
+\echo 'error:' :ERROR
+error: true
+\echo 'error code:' :SQLSTATE
+error code: 42601
+\echo 'last error message:' :LAST_ERROR_MESSAGE
+last error message: syntax error at or near ";"
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index d2e5b08541..3301c45b1d 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -5957,23 +5957,27 @@ typedef enum
{
PQERRORS_TERSE,
PQERRORS_DEFAULT,
- PQERRORS_VERBOSE
+ PQERRORS_VERBOSE,
+ PQERRORS_SQLSTATE
} PGVerbosity;
PGVerbosity PQsetErrorVerbosity(PGconn *conn, PGVerbosity verbosity);
</synopsis>
- <function>PQsetErrorVerbosity</function> sets the verbosity mode, returning
- the connection's previous setting. In <firstterm>TERSE</firstterm> mode,
- returned messages include severity, primary text, and position only;
- this will normally fit on a single line. The default mode produces
- messages that include the above plus any detail, hint, or context
- fields (these might span multiple lines). The <firstterm>VERBOSE</firstterm>
- mode includes all available fields. Changing the verbosity does not
- affect the messages available from already-existing
- <structname>PGresult</structname> objects, only subsequently-created ones.
- (But see <function>PQresultVerboseErrorMessage</function> if you
- want to print a previous error with a different verbosity.)
+ <function>PQsetErrorVerbosity</function> sets the verbosity mode,
+ returning the connection's previous setting. In
+ <firstterm>SQLSTATE</firstterm> mode, returned messages include
+ severity and <symbol>SQLSTATE</symbol> error code. In
+ <firstterm>TERSE</firstterm> mode, returned messages include severity,
+ primary text, and position only; this will normally fit on a single
+ line. The default mode produces messages that include the above plus
+ any detail, hint, or context fields (these might span multiple lines).
+ The <firstterm>VERBOSE</firstterm> mode includes all available fields.
+ Changing the verbosity does not affect the messages available from
+ already-existing <structname>PGresult</structname> 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>
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 6c76cf2f00..05946e8507 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -3867,7 +3867,8 @@ bar
messages from the server. The default is <literal>errors</literal> (meaning
that context will be shown in error messages, but not in notice or
warning messages). This setting has no effect
- when <varname>VERBOSITY</varname> is set to <literal>terse</literal>.
+ when <varname>VERBOSITY</varname> is set to <literal>sqlstate</literal>
+ or <literal>terse</literal>.
(See also <command>\errverbose</command>, for use when you want a verbose
version of the error you just got.)
</para>
@@ -3921,8 +3922,8 @@ bar
<listitem>
<para>
This variable can be set to the values <literal>default</literal>,
- <literal>verbose</literal>, or <literal>terse</literal> to control the verbosity
- of error reports.
+ <literal>verbose</literal>, <literal>sqlstate</literal> or <literal>terse</literal>
+ to control the verbosity of error reports.
(See also <command>\errverbose</command>, for use when you want a verbose
version of the error you just got.)
</para>
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation: tested, passed
This is a handy feature, and the implementation looks good to me.
There might be some nit-picking about the vertical whitespace around
the code in added to fe-protocol3.c, but I'll leave that to the committer.
The new status of this patch is: Ready for Committer
On 04/12/2018 13:18, didier wrote:
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql index 1bb2a6e16d..64628f29a3 100644 --- a/src/test/regress/sql/psql.sql +++ b/src/test/regress/sql/psql.sql @@ -1016,3 +1016,22 @@ select 1/(15-unique2) from tenk1 order by unique2 limit 19; \echo 'last error code:' :LAST_ERROR_SQLSTATE\unset FETCH_COUNT + +-- verbosity error setting +\set VERBOSITY terse +SELECT 1 UNION; +\echo 'error:' :ERROR +\echo 'error code:' :SQLSTATE +\echo 'last error message:' :LAST_ERROR_MESSAGE + +\set VERBOSITY sqlstate +SELECT 1 UNION; +\echo 'error:' :ERROR +\echo 'error code:' :SQLSTATE +\echo 'last error message:' :LAST_ERROR_MESSAGE + +\set VERBOSITY default +SELECT 1 UNION; +\echo 'error:' :ERROR +\echo 'error code:' :SQLSTATE +\echo 'last error message:' :LAST_ERROR_MESSAGE --
Why are you not including a test for \set VERBOSITY verbose?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
Why are you not including a test for \set VERBOSITY verbose?
Stability of the output would be a problem ...
regards, tom lane
On Sat, Jan 5, 2019 at 6:30 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
Why are you not including a test for \set VERBOSITY verbose?
Stability of the output would be a problem ...
Yes it could moreover the functionality wasn't tested before.
Should I add one ?
On Mon, Jan 07, 2019 at 10:44:24PM +0100, didier wrote:
On Sat, Jan 5, 2019 at 6:30 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
Why are you not including a test for \set VERBOSITY verbose?
Stability of the output would be a problem ...
Yes it could moreover the functionality wasn't tested before.
Should I add one ?
Unpredictible outputs mean more maintenance and more alternate
outputs. I have moved the patch to next CF, still ready for
committer.
--
Michael
On 2/4/19 5:54 AM, Michael Paquier wrote:
On Mon, Jan 07, 2019 at 10:44:24PM +0100, didier wrote:
On Sat, Jan 5, 2019 at 6:30 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
Why are you not including a test for \set VERBOSITY verbose?
Stability of the output would be a problem ...
Yes it could moreover the functionality wasn't tested before.
Should I add one ?
Unpredictible outputs mean more maintenance and more alternate
outputs. I have moved the patch to next CF, still ready for
committer.
What do you think, Peter? Is the extra test valuable or will it cause
unpredictable outputs as Tom and Michael predict?
Regards,
--
-David
david@pgmasters.net
David Steele <david@pgmasters.net> writes:
Why are you not including a test for \set VERBOSITY verbose?
What do you think, Peter? Is the extra test valuable or will it cause
unpredictable outputs as Tom and Michael predict?
I'm not really sure why this is open for discussion.
regression=# \set VERBOSITY verbose
regression=# select 1/0;
ERROR: 22012: division by zero
LOCATION: int4div, int.c:824
It's not going to be tolerable to have to adjust such a test anytime
somebody adds or removes lines in whichever backend file throws the
tested-for error (never mind more-substantial refactoring such as
moving the ereport call to a different function or file). I also
believe that the reported line number will vary across compilers
even without that: IME you might get either the starting or ending
line number of the ereport() construct.
There's also the question of whether the function name is reliably
available. Maybe it is now that we require C99 compatibility,
but the code hasn't been taught to assume that.
regards, tom lane
On Thursday, March 21, 2019 7:47 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
David Steele david@pgmasters.net writes:
Why are you not including a test for \set VERBOSITY verbose?
What do you think, Peter? Is the extra test valuable or will it cause
unpredictable outputs as Tom and Michael predict?I'm not really sure why this is open for discussion.
regression=# \set VERBOSITY verbose
regression=# select 1/0;
ERROR: 22012: division by zero
LOCATION: int4div, int.c:824It's not going to be tolerable to have to adjust such a test anytime
somebody adds or removes lines in whichever backend file throws the
tested-for error (never mind more-substantial refactoring such as
moving the ereport call to a different function or file). I also
believe that the reported line number will vary across compilers
even without that: IME you might get either the starting or ending
line number of the ereport() construct.
In GPDB we have to handle variable output in tests similar to this (but
for very different reasons), and unless there is a big gain elsewhere
from having variable test output I would advise against it. It adds a
fair bit of complexity and another moving part which can mask subtle
test failures.
cheers ./daniel
On 2019-03-21 19:01, David Steele wrote:
What do you think, Peter? Is the extra test valuable or will it cause
unpredictable outputs as Tom and Michael predict?
Yes, I'm OK with that.
But now that I read the patch again, I'm not sure why this needs to
touch libpq. The formatting of error messages in psql should be handled
in psql.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
But now that I read the patch again, I'm not sure why this needs to
touch libpq. The formatting of error messages in psql should be handled
in psql.
Maybe in an ideal world that'd be the case, but psql has always just
depended on PQerrorMessage(). I don't think this patch should be
tasked with changing that. I'm not even sure that doing so would be
a good idea: I think the idea there is that whatever we believe is a
nice error-reporting option for psql ought to be easily available to
other libpq clients too.
(Note: I haven't read the patch and don't mean to be saying that it's
necessarily OK. But I'm fine with the idea that something like this
involves touching libpq more than psql.)
regards, tom lane
didier <did447@gmail.com> writes:
[ 0001-Add-sqlstate-output-mode-to-VERBOSITY_v1.patch ]
Pushed with some mostly-cosmetic adjustments. The main non-cosmetic
thing I did was to adjust the logic so that if no SQLSTATE is available,
it acts like TERSE mode. Otherwise, we'd print nothing at all except
"ERROR:", which seems both quite useless and contrary to our message
style guidelines. Moreover, documenting it this way makes the behavior
not inconsistent with what happens for libpq-generated errors and
errors from protocol-version-2 servers.
regards, tom lane