psql - add special variable to reflect the last query status

Started by Fabien COELHOalmost 9 years ago40 messages
#1Fabien COELHO
coelho@cri.ensmp.fr
1 attachment(s)

After some discussions about what could be useful since psql scripts now
accepts tests, this patch sets a few variables which can be used by psql
after a "front door" (i.e. actually typed by the user) query:

- RESULT_STATUS: the status of the query
- ERROR: whether the query failed
- ERROR_MESSAGE: ...
- ROW_COUNT: #rows affected

SELECT * FROM ;
\if :ERROR
\echo oops
\q
\endif

I'm not sure that the names are right. Maybe STATUS would be better than
RESULT_STATUS.

--
Fabien.

Attachments:

psql-result-status-1.patchtext/x-diff; name=psql-result-status-1.patchDownload
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 3b86612..7006f23 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -3449,6 +3449,24 @@ bar
       </varlistentry>
 
       <varlistentry>
+       <term><varname>ERROR</varname></term>
+       <listitem>
+        <para>
+         Whether the last query failed.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
+       <term><varname>ERROR_MESSAGE</varname></term>
+       <listitem>
+        <para>
+         If the last query failed, the associated error message.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
         <term><varname>FETCH_COUNT</varname></term>
         <listitem>
         <para>
@@ -3653,6 +3671,25 @@ bar
       </varlistentry>
 
       <varlistentry>
+       <term><varname>RESULT_STATUS</varname></term>
+       <listitem>
+        <para>
+         Text status of the last query.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
+       <term><varname>ROW_COUNT</varname></term>
+       <listitem>
+        <para>
+         When appropriate, how many rows were returned or affected by the
+         last query.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
         <term><varname>QUIET</varname></term>
         <listitem>
         <para>
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index a2f1259..74d22fb 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -1213,6 +1213,57 @@ PrintQueryResults(PGresult *results)
 	return success;
 }
 
+/*
+ * Set special variables for "front door" queries
+ * - RESULT_STATUS: last query status
+ * - ERROR: TRUE/FALSE, whether an error occurred
+ * - ERROR_MESSAGE: message if an error occured
+ * - ROW_COUNT: how many rows were returned or affected, if appropriate
+ */
+static void
+SetResultVariables(PGresult *results)
+{
+	bool			success;
+	ExecStatusType	restat = PQresultStatus(results);
+
+	SetVariable(pset.vars, "RESULT_STATUS", PQresStatus(restat));
+
+	switch (restat)
+	{
+		case PGRES_EMPTY_QUERY:
+		case PGRES_TUPLES_OK:
+		case PGRES_COMMAND_OK:
+		case PGRES_COPY_OUT:
+		case PGRES_COPY_IN:
+		case PGRES_COPY_BOTH:
+		case PGRES_SINGLE_TUPLE:
+			success = true;
+			SetVariable(pset.vars, "ERROR", "FALSE");
+			SetVariable(pset.vars, "ERROR_MESSAGE", NULL);
+			break;
+		case PGRES_BAD_RESPONSE:
+		case PGRES_NONFATAL_ERROR:
+		case PGRES_FATAL_ERROR:
+			success = false;
+			SetVariable(pset.vars, "ERROR", "TRUE");
+			SetVariable(pset.vars, "ERROR_MESSAGE", PQerrorMessage(pset.db));
+			break;
+		default:
+			/* dead code */
+			success = false;
+			psql_error("unexpected PQresultStatus: %d\n", restat);
+			break;
+	}
+
+	if (success)
+	{
+		/* set variable if non empty */
+		char   *ntuples = PQcmdTuples(results);
+		SetVariable(pset.vars, "ROW_COUNT", *ntuples ? ntuples : NULL);
+	}
+	else
+		SetVariable(pset.vars, "ROW_COUNT", NULL);
+}
 
 /*
  * SendQuery: send the query string to the backend
@@ -1346,6 +1397,9 @@ SendQuery(const char *query)
 			elapsed_msec = INSTR_TIME_GET_MILLISEC(after);
 		}
 
+		/* set special variables to reflect the result status */
+		SetResultVariables(results);
+
 		/* but printing results isn't: */
 		if (OK && results)
 			OK = PrintQueryResults(results);
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index d602aee..3ee96b5 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -2904,6 +2904,36 @@ bar 'bar' "bar"
 	\echo 'should print #8-1'
 should print #8-1
 \endif
+-- special result variables
+SELECT 1 UNION SELECT 2;
+ ?column? 
+----------
+        1
+        2
+(2 rows)
+
+\echo 'result status: ' :RESULT_STATUS
+result status:  PGRES_TUPLES_OK
+\echo 'number of rows: ' :ROW_COUNT
+number of rows:  2
+SELECT 1 UNION;
+ERROR:  syntax error at or near ";"
+LINE 1: SELECT 1 UNION;
+                      ^
+\echo 'result status: ' :RESULT_STATUS
+result status:  PGRES_FATAL_ERROR
+\if :ERROR
+  \echo 'Oops, an error occured...'
+Oops, an error occured...
+  \echo 'error message: ' :ERROR_MESSAGE
+error message:  ERROR:  syntax error at or near ";"
+LINE 1: SELECT 1 UNION;
+                      ^
+
+\endif
+;
+\echo 'result status: ' :RESULT_STATUS
+result status:  PGRES_EMPTY_QUERY
 -- SHOW_CONTEXT
 \set SHOW_CONTEXT never
 do $$
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index b56a05f..716fe06 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -526,6 +526,21 @@ select \if false \\ (bogus \else \\ 42 \endif \\ forty_two;
 	\echo 'should print #8-1'
 \endif
 
+-- special result variables
+SELECT 1 UNION SELECT 2;
+\echo 'result status: ' :RESULT_STATUS
+\echo 'number of rows: ' :ROW_COUNT
+
+SELECT 1 UNION;
+\echo 'result status: ' :RESULT_STATUS
+\if :ERROR
+  \echo 'Oops, an error occured...'
+  \echo 'error message: ' :ERROR_MESSAGE
+\endif
+
+;
+\echo 'result status: ' :RESULT_STATUS
+
 -- SHOW_CONTEXT
 
 \set SHOW_CONTEXT never
#2Pavel Stehule
pavel.stehule@gmail.com
In reply to: Fabien COELHO (#1)
Re: psql - add special variable to reflect the last query status

2017-04-04 22:05 GMT+02:00 Fabien COELHO <coelho@cri.ensmp.fr>:

After some discussions about what could be useful since psql scripts now
accepts tests, this patch sets a few variables which can be used by psql
after a "front door" (i.e. actually typed by the user) query:

- RESULT_STATUS: the status of the query
- ERROR: whether the query failed
- ERROR_MESSAGE: ...
- ROW_COUNT: #rows affected

SELECT * FROM ;
\if :ERROR
\echo oops
\q
\endif

I'm not sure that the names are right. Maybe STATUS would be better than
RESULT_STATUS.

good ideas

Regards

Pavel

Show quoted text

--
Fabien.

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

#3Pavel Stehule
pavel.stehule@gmail.com
In reply to: Pavel Stehule (#2)
Re: psql - add special variable to reflect the last query status

Hi

2017-04-04 23:01 GMT+02:00 Pavel Stehule <pavel.stehule@gmail.com>:

2017-04-04 22:05 GMT+02:00 Fabien COELHO <coelho@cri.ensmp.fr>:

After some discussions about what could be useful since psql scripts now
accepts tests, this patch sets a few variables which can be used by psql
after a "front door" (i.e. actually typed by the user) query:

- RESULT_STATUS: the status of the query
- ERROR: whether the query failed
- ERROR_MESSAGE: ...
- ROW_COUNT: #rows affected

SELECT * FROM ;
\if :ERROR
\echo oops
\q
\endif

I'm not sure that the names are right. Maybe STATUS would be better than
RESULT_STATUS.

I am sending review of this patch:

1. I agree so STATUS is better name, than RESULT status. Currently it
returns values with prefix PGRES (like PGRES_FATAL_ERROR, PGRES_TUPLES_OK).
Maybe we should to cut this prefix. FATAL_ERROR, TUPLES_OK looks better for
custom level. The PGRES prefix has not sense in psql.

2. I propose availability to read ERROR_CODE - sometimes it can be more
practical than parsing error possible translated message

3. The fields ERROR_MESSAGE and ROW_COUNT are set only when it has sense.
This behave is maybe too strict for psql and the processing needs more
nesting \if command. What do you think about -1 or 0 for ROW_COUNT (for
DDL) and "" for ERROR_MESSAGE when there are not any error? It will be
consistent with already implemented LASTOID variable (and other state psql
variables). Using default values are not strict clean, but it can reduce
complexity of psql scripts.

4. all regress tests passed
5. there are not any problem with doc building

Regards

Pavel

Show quoted text

good ideas

Regards

Pavel

--
Fabien.

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

#4Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Pavel Stehule (#3)
1 attachment(s)
Re: psql - add special variable to reflect the last query status

Hello Pavel,

After some discussions about what could be useful since psql scripts now
accepts tests, this patch sets a few variables which can be used by psql
after a "front door" (i.e. actually typed by the user) query:

- RESULT_STATUS: the status of the query
- ERROR: whether the query failed
- ERROR_MESSAGE: ...
- ROW_COUNT: #rows affected

SELECT * FROM ;
\if :ERROR
\echo oops
\q
\endif

I'm not sure that the names are right. Maybe STATUS would be better than
RESULT_STATUS.

I am sending review of this patch:

1. I agree so STATUS is better name, than RESULT status.

Ok, looks simpler.

Currently it returns values with prefix PGRES (like PGRES_FATAL_ERROR,
PGRES_TUPLES_OK). Maybe we should to cut this prefix. FATAL_ERROR,
TUPLES_OK looks better for custom level. The PGRES prefix has not sense
in psql.

Indeed. I skipped "PGRES_".

2. I propose availability to read ERROR_CODE - sometimes it can be more
practical than parsing error possible translated message

Ok.

3. The fields ERROR_MESSAGE and ROW_COUNT are set only when it has sense.
This behave is maybe too strict for psql and the processing needs more
nesting \if command. What do you think about -1 or 0 for ROW_COUNT (for
DDL) and "" for ERROR_MESSAGE when there are not any error? It will be
consistent with already implemented LASTOID variable (and other state psql
variables). Using default values are not strict clean, but it can reduce
complexity of psql scripts.

My intention was that it could be tested with the "is defined" syntax,
which is yet to be agreed upon and implemented, so maybe generating empty
string is a better option.

For ROW_COUNT, I think that it should be consistent with what PL/pgSQL
does, so it think that 0 should be the default.

4. all regress tests passed
5. there are not any problem with doc building

Please find attached a v2 which hopefully takes into account all your
points above.

Open question: should it gather more PQerrorResultField, or the two
selected one are enough? If more, which should be included?

--
Fabien.

Attachments:

psql-result-status-2.patchtext/x-diff; name=psql-result-status-2.patchDownload
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 3b86612..d33da32 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -3449,6 +3449,35 @@ bar
       </varlistentry>
 
       <varlistentry>
+       <term><varname>ERROR</varname></term>
+       <listitem>
+        <para>
+         Whether the last query failed, as a boolean.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
+       <term><varname>ERROR_CODE</varname></term>
+       <listitem>
+        <para>
+         The error code associated to the last query, or
+         <literal>00000</> if no error occured.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
+       <term><varname>ERROR_MESSAGE</varname></term>
+       <listitem>
+        <para>
+         If the last query failed, the associated error message,
+         otherwise an empty string.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
         <term><varname>FETCH_COUNT</varname></term>
         <listitem>
         <para>
@@ -3653,6 +3682,24 @@ bar
       </varlistentry>
 
       <varlistentry>
+       <term><varname>STATUS</varname></term>
+       <listitem>
+        <para>
+         Text status of the last query.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
+       <term><varname>ROW_COUNT</varname></term>
+       <listitem>
+        <para>
+         How many rows were returned or affected by the last query.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
         <term><varname>QUIET</varname></term>
         <listitem>
         <para>
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index a2f1259..4a3c6a9 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -1213,6 +1213,62 @@ PrintQueryResults(PGresult *results)
 	return success;
 }
 
+/*
+ * Set special variables for "front door" queries
+ * - STATUS: last query status
+ * - ERROR: TRUE/FALSE, whether an error occurred
+ * - ERROR_CODE: code if an error occured, or "00000"
+ * - ERROR_MESSAGE: message if an error occured, or ""
+ * - ROW_COUNT: how many rows were returned or affected, or "0"
+ */
+static void
+SetResultVariables(PGresult *results)
+{
+	bool			success;
+	ExecStatusType	restat = PQresultStatus(results);
+	char 		   *code = PQresultErrorField(results, PG_DIAG_SQLSTATE);
+	char 		   *mesg = PQresultErrorField(results, PG_DIAG_MESSAGE_PRIMARY);
+
+	SetVariable(pset.vars, "STATUS", PQresStatus(restat) + strlen("PGRES_"));
+	SetVariable(pset.vars, "ERROR_CODE", code ? code : "00000");
+	SetVariable(pset.vars, "ERROR_MESSAGE", mesg ? mesg : "");
+
+	/* check all possible PGRES_ */
+	switch (restat)
+	{
+		case PGRES_EMPTY_QUERY:
+		case PGRES_TUPLES_OK:
+		case PGRES_COMMAND_OK:
+		case PGRES_COPY_OUT:
+		case PGRES_COPY_IN:
+		case PGRES_COPY_BOTH:
+		case PGRES_SINGLE_TUPLE:
+			success = true;
+			break;
+		case PGRES_BAD_RESPONSE:
+		case PGRES_NONFATAL_ERROR:
+		case PGRES_FATAL_ERROR:
+			success = false;
+			break;
+		default:
+			/* dead code */
+			success = false;
+			psql_error("unexpected PQresultStatus: %d\n", restat);
+			break;
+	}
+
+	if (success)
+	{
+		char   *ntuples = PQcmdTuples(results);
+		SetVariable(pset.vars, "ROW_COUNT", *ntuples ? ntuples : "0");
+		SetVariable(pset.vars, "ERROR", "TRUE");
+	}
+	else
+	{
+		SetVariable(pset.vars, "ROW_COUNT", "0");
+		SetVariable(pset.vars, "ERROR", "FALSE");
+	}
+}
 
 /*
  * SendQuery: send the query string to the backend
@@ -1346,6 +1402,9 @@ SendQuery(const char *query)
 			elapsed_msec = INSTR_TIME_GET_MILLISEC(after);
 		}
 
+		/* set special variables to reflect the result status */
+		SetResultVariables(results);
+
 		/* but printing results isn't: */
 		if (OK && results)
 			OK = PrintQueryResults(results);
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index d602aee..eaf538a 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -2904,6 +2904,60 @@ bar 'bar' "bar"
 	\echo 'should print #8-1'
 should print #8-1
 \endif
+-- special result variables
+-- 2 rows select
+SELECT 1 AS stuff UNION SELECT 2;
+ stuff 
+-------
+     1
+     2
+(2 rows)
+
+\echo 'status:' :STATUS
+status: TUPLES_OK
+\echo 'error code:' :ERROR_CODE
+error code: 00000
+\echo 'error message:' :ERROR_MESSAGE
+error message: 
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 2
+-- syntax error
+SELECT 1 UNION;
+ERROR:  syntax error at or near ";"
+LINE 1: SELECT 1 UNION;
+                      ^
+\echo 'status:' :STATUS
+status: FATAL_ERROR
+\if :ERROR
+  \echo 'Oops, an error occured...'
+\endif
+\echo 'error code:' :ERROR_CODE
+error code: 42601
+\echo 'error message:' :ERROR_MESSAGE
+error message: syntax error at or near ";"
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
+-- empty query
+;
+\echo 'status:' :STATUS
+status: EMPTY_QUERY
+\echo 'error code:' :ERROR_CODE
+error code: 00000
+\echo 'error message:' :ERROR_MESSAGE
+error message: 
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
+-- other query error
+DROP TABLE this_table_does_not_exist;
+ERROR:  table "this_table_does_not_exist" does not exist
+\echo 'status:' :STATUS
+status: FATAL_ERROR
+\echo 'error code:' :ERROR_CODE
+error code: 42P01
+\echo 'error message:' :ERROR_MESSAGE
+error message: table "this_table_does_not_exist" does not exist
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
 -- SHOW_CONTEXT
 \set SHOW_CONTEXT never
 do $$
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index b56a05f..79efa74 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -526,6 +526,39 @@ select \if false \\ (bogus \else \\ 42 \endif \\ forty_two;
 	\echo 'should print #8-1'
 \endif
 
+-- special result variables
+
+-- 2 rows select
+SELECT 1 AS stuff UNION SELECT 2;
+\echo 'status:' :STATUS
+\echo 'error code:' :ERROR_CODE
+\echo 'error message:' :ERROR_MESSAGE
+\echo 'number of rows:' :ROW_COUNT
+
+-- syntax error
+SELECT 1 UNION;
+\echo 'status:' :STATUS
+\if :ERROR
+  \echo 'Oops, an error occured...'
+\endif
+\echo 'error code:' :ERROR_CODE
+\echo 'error message:' :ERROR_MESSAGE
+\echo 'number of rows:' :ROW_COUNT
+
+-- empty query
+;
+\echo 'status:' :STATUS
+\echo 'error code:' :ERROR_CODE
+\echo 'error message:' :ERROR_MESSAGE
+\echo 'number of rows:' :ROW_COUNT
+
+-- other query error
+DROP TABLE this_table_does_not_exist;
+\echo 'status:' :STATUS
+\echo 'error code:' :ERROR_CODE
+\echo 'error message:' :ERROR_MESSAGE
+\echo 'number of rows:' :ROW_COUNT
+
 -- SHOW_CONTEXT
 
 \set SHOW_CONTEXT never
#5Pavel Stehule
pavel.stehule@gmail.com
In reply to: Fabien COELHO (#4)
Re: psql - add special variable to reflect the last query status

2017-05-22 9:40 GMT+02:00 Fabien COELHO <coelho@cri.ensmp.fr>:

Hello Pavel,

After some discussions about what could be useful since psql scripts now

accepts tests, this patch sets a few variables which can be used by psql
after a "front door" (i.e. actually typed by the user) query:

- RESULT_STATUS: the status of the query
- ERROR: whether the query failed
- ERROR_MESSAGE: ...
- ROW_COUNT: #rows affected

SELECT * FROM ;
\if :ERROR
\echo oops
\q
\endif

I'm not sure that the names are right. Maybe STATUS would be better than
RESULT_STATUS.

I am sending review of this patch:

1. I agree so STATUS is better name, than RESULT status.

Ok, looks simpler.

Currently it returns values with prefix PGRES (like PGRES_FATAL_ERROR,

PGRES_TUPLES_OK). Maybe we should to cut this prefix. FATAL_ERROR,
TUPLES_OK looks better for custom level. The PGRES prefix has not sense in
psql.

Indeed. I skipped "PGRES_".

2. I propose availability to read ERROR_CODE - sometimes it can be more

practical than parsing error possible translated message

Ok.

3. The fields ERROR_MESSAGE and ROW_COUNT are set only when it has sense.

This behave is maybe too strict for psql and the processing needs more
nesting \if command. What do you think about -1 or 0 for ROW_COUNT (for
DDL) and "" for ERROR_MESSAGE when there are not any error? It will be
consistent with already implemented LASTOID variable (and other state psql
variables). Using default values are not strict clean, but it can reduce
complexity of psql scripts.

My intention was that it could be tested with the "is defined" syntax,
which is yet to be agreed upon and implemented, so maybe generating empty
string is a better option.

For ROW_COUNT, I think that it should be consistent with what PL/pgSQL
does, so it think that 0 should be the default.

4. all regress tests passed

5. there are not any problem with doc building

Please find attached a v2 which hopefully takes into account all your
points above.

Open question: should it gather more PQerrorResultField, or the two
selected one are enough? If more, which should be included?

I don't think so it is necessary. No in this moment. ERROR_CODE and
ERROR_MESSAGE are fundamental - and if we add other, then we should to add
all. Has not sense to add only some.

Regards

Pavel

Show quoted text

--
Fabien.

#6Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Pavel Stehule (#5)
Re: psql - add special variable to reflect the last query status

Please find attached a v2 which hopefully takes into account all your
points above.

Open question: should it gather more PQerrorResultField, or the two
selected one are enough? If more, which should be included?

I don't think so it is necessary. No in this moment. ERROR_CODE and
ERROR_MESSAGE are fundamental - and if we add other, then we should to add
all. Has not sense to add only some.

Ok. I'm fine with stopping at CODE & MESSAGE.

--
Fabien.

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

#7Pavel Stehule
pavel.stehule@gmail.com
In reply to: Fabien COELHO (#6)
Re: psql - add special variable to reflect the last query status

2017-05-22 21:33 GMT+02:00 Fabien COELHO <coelho@cri.ensmp.fr>:

Please find attached a v2 which hopefully takes into account all your

points above.

Open question: should it gather more PQerrorResultField, or the two
selected one are enough? If more, which should be included?

I don't think so it is necessary. No in this moment. ERROR_CODE and
ERROR_MESSAGE are fundamental - and if we add other, then we should to add
all. Has not sense to add only some.

Ok. I'm fine with stopping at CODE & MESSAGE.

I have not any other comments. The implementation is trivial. I rerun all
tests and tests passed.

I'll mark this patch as ready for commiters.

Regards

Pavel

Show quoted text

--
Fabien.

#8Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Pavel Stehule (#7)
Re: psql - add special variable to reflect the last query status

Hello Pavel,

I have not any other comments. The implementation is trivial. [...]

Indeed.

I'll mark this patch as ready for commiters.

Thanks for the review.

--
Fabien.

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

#9Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Pavel Stehule (#7)
1 attachment(s)
Re: psql - add special variable to reflect the last query status

I have not any other comments. The implementation is trivial. I rerun all
tests and tests passed.

I'll mark this patch as ready for commiters.

Oops, I just noticed a stupid confusion on my part which got through, I
was setting "ERROR" as "success", inverting the expected boolean value.

Here is a fixed version.

--
Fabien.

Attachments:

psql-result-status-3.patchtext/x-diff; name=psql-result-status-3.patchDownload
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index e6eba21..18c3e7e 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -3450,6 +3450,35 @@ bar
       </varlistentry>
 
       <varlistentry>
+       <term><varname>ERROR</varname></term>
+       <listitem>
+        <para>
+         Whether the last query failed, as a boolean.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
+       <term><varname>ERROR_CODE</varname></term>
+       <listitem>
+        <para>
+         The error code associated to the last query, or
+         <literal>00000</> if no error occured.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
+       <term><varname>ERROR_MESSAGE</varname></term>
+       <listitem>
+        <para>
+         If the last query failed, the associated error message,
+         otherwise an empty string.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
         <term><varname>FETCH_COUNT</varname></term>
         <listitem>
         <para>
@@ -3654,6 +3683,24 @@ bar
       </varlistentry>
 
       <varlistentry>
+       <term><varname>STATUS</varname></term>
+       <listitem>
+        <para>
+         Text status of the last query.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
+       <term><varname>ROW_COUNT</varname></term>
+       <listitem>
+        <para>
+         How many rows were returned or affected by the last query.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
         <term><varname>QUIET</varname></term>
         <listitem>
         <para>
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index a2f1259..e717159 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -1213,6 +1213,62 @@ PrintQueryResults(PGresult *results)
 	return success;
 }
 
+/*
+ * Set special variables for "front door" queries
+ * - STATUS: last query status
+ * - ERROR: TRUE/FALSE, whether an error occurred
+ * - ERROR_CODE: code if an error occured, or "00000"
+ * - ERROR_MESSAGE: message if an error occured, or ""
+ * - ROW_COUNT: how many rows were returned or affected, or "0"
+ */
+static void
+SetResultVariables(PGresult *results)
+{
+	bool			success;
+	ExecStatusType	restat = PQresultStatus(results);
+	char 		   *code = PQresultErrorField(results, PG_DIAG_SQLSTATE);
+	char 		   *mesg = PQresultErrorField(results, PG_DIAG_MESSAGE_PRIMARY);
+
+	SetVariable(pset.vars, "STATUS", PQresStatus(restat) + strlen("PGRES_"));
+	SetVariable(pset.vars, "ERROR_CODE", code ? code : "00000");
+	SetVariable(pset.vars, "ERROR_MESSAGE", mesg ? mesg : "");
+
+	/* check all possible PGRES_ */
+	switch (restat)
+	{
+		case PGRES_EMPTY_QUERY:
+		case PGRES_TUPLES_OK:
+		case PGRES_COMMAND_OK:
+		case PGRES_COPY_OUT:
+		case PGRES_COPY_IN:
+		case PGRES_COPY_BOTH:
+		case PGRES_SINGLE_TUPLE:
+			success = true;
+			break;
+		case PGRES_BAD_RESPONSE:
+		case PGRES_NONFATAL_ERROR:
+		case PGRES_FATAL_ERROR:
+			success = false;
+			break;
+		default:
+			/* dead code */
+			success = false;
+			psql_error("unexpected PQresultStatus: %d\n", restat);
+			break;
+	}
+
+	if (success)
+	{
+		char   *ntuples = PQcmdTuples(results);
+		SetVariable(pset.vars, "ROW_COUNT", *ntuples ? ntuples : "0");
+		SetVariable(pset.vars, "ERROR", "FALSE");
+	}
+	else
+	{
+		SetVariable(pset.vars, "ROW_COUNT", "0");
+		SetVariable(pset.vars, "ERROR", "TRUE");
+	}
+}
 
 /*
  * SendQuery: send the query string to the backend
@@ -1346,6 +1402,9 @@ SendQuery(const char *query)
 			elapsed_msec = INSTR_TIME_GET_MILLISEC(after);
 		}
 
+		/* set special variables to reflect the result status */
+		SetResultVariables(results);
+
 		/* but printing results isn't: */
 		if (OK && results)
 			OK = PrintQueryResults(results);
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index d602aee..c3972a6 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -2904,6 +2904,69 @@ bar 'bar' "bar"
 	\echo 'should print #8-1'
 should print #8-1
 \endif
+-- special result variables
+-- 2 rows select
+SELECT 1 AS stuff UNION SELECT 2;
+ stuff 
+-------
+     1
+     2
+(2 rows)
+
+\if :ERROR
+  \echo 'MUST NOT SHOW'
+\else
+  \echo 'ERROR is FALSE as expected'
+ERROR is FALSE as expected
+\endif
+\echo 'status:' :STATUS
+status: TUPLES_OK
+\echo 'error code:' :ERROR_CODE
+error code: 00000
+\echo 'error message:' :ERROR_MESSAGE
+error message: 
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 2
+-- syntax error
+SELECT 1 UNION;
+ERROR:  syntax error at or near ";"
+LINE 1: SELECT 1 UNION;
+                      ^
+\echo 'status:' :STATUS
+status: FATAL_ERROR
+\if :ERROR
+  \echo 'ERROR is TRUE as expected'
+ERROR is TRUE as expected
+\else
+  \echo 'MUST NOT SHOW'
+\endif
+\echo 'error code:' :ERROR_CODE
+error code: 42601
+\echo 'error message:' :ERROR_MESSAGE
+error message: syntax error at or near ";"
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
+-- empty query
+;
+\echo 'status:' :STATUS
+status: EMPTY_QUERY
+\echo 'error code:' :ERROR_CODE
+error code: 00000
+\echo 'error message:' :ERROR_MESSAGE
+error message: 
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
+-- other query error
+DROP TABLE this_table_does_not_exist;
+ERROR:  table "this_table_does_not_exist" does not exist
+\echo 'status:' :STATUS
+status: FATAL_ERROR
+\echo 'error code:' :ERROR_CODE
+error code: 42P01
+\echo 'error message:' :ERROR_MESSAGE
+error message: table "this_table_does_not_exist" does not exist
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
 -- SHOW_CONTEXT
 \set SHOW_CONTEXT never
 do $$
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index b56a05f..f83ac70 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -526,6 +526,46 @@ select \if false \\ (bogus \else \\ 42 \endif \\ forty_two;
 	\echo 'should print #8-1'
 \endif
 
+-- special result variables
+
+-- 2 rows select
+SELECT 1 AS stuff UNION SELECT 2;
+\if :ERROR
+  \echo 'MUST NOT SHOW'
+\else
+  \echo 'ERROR is FALSE as expected'
+\endif
+\echo 'status:' :STATUS
+\echo 'error code:' :ERROR_CODE
+\echo 'error message:' :ERROR_MESSAGE
+\echo 'number of rows:' :ROW_COUNT
+
+-- syntax error
+SELECT 1 UNION;
+\echo 'status:' :STATUS
+\if :ERROR
+  \echo 'ERROR is TRUE as expected'
+\else
+  \echo 'MUST NOT SHOW'
+\endif
+\echo 'error code:' :ERROR_CODE
+\echo 'error message:' :ERROR_MESSAGE
+\echo 'number of rows:' :ROW_COUNT
+
+-- empty query
+;
+\echo 'status:' :STATUS
+\echo 'error code:' :ERROR_CODE
+\echo 'error message:' :ERROR_MESSAGE
+\echo 'number of rows:' :ROW_COUNT
+
+-- other query error
+DROP TABLE this_table_does_not_exist;
+\echo 'status:' :STATUS
+\echo 'error code:' :ERROR_CODE
+\echo 'error message:' :ERROR_MESSAGE
+\echo 'number of rows:' :ROW_COUNT
+
 -- SHOW_CONTEXT
 
 \set SHOW_CONTEXT never
#10Pavel Stehule
pavel.stehule@gmail.com
In reply to: Fabien COELHO (#9)
Re: psql - add special variable to reflect the last query status

2017-06-17 7:58 GMT+02:00 Fabien COELHO <coelho@cri.ensmp.fr>:

I have not any other comments. The implementation is trivial. I rerun all

tests and tests passed.

I'll mark this patch as ready for commiters.

Oops, I just noticed a stupid confusion on my part which got through, I
was setting "ERROR" as "success", inverting the expected boolean value.

Here is a fixed version.

I missed it too.

We can introduce macro SetVariableBool(vars, varname, bool) instead

SetVariable(pset.vars, "ERROR", "FALSE");

Regards

Pavel

Show quoted text

--
Fabien.

#11Pavel Stehule
pavel.stehule@gmail.com
In reply to: Pavel Stehule (#10)
Re: psql - add special variable to reflect the last query status

Hi

2017-06-19 5:55 GMT+02:00 Pavel Stehule <pavel.stehule@gmail.com>:

2017-06-17 7:58 GMT+02:00 Fabien COELHO <coelho@cri.ensmp.fr>:

I have not any other comments. The implementation is trivial. I rerun all

tests and tests passed.

I'll mark this patch as ready for commiters.

Oops, I just noticed a stupid confusion on my part which got through, I
was setting "ERROR" as "success", inverting the expected boolean value.

Here is a fixed version.

I missed it too.

We can introduce macro SetVariableBool(vars, varname, bool) instead

SetVariable(pset.vars, "ERROR", "FALSE");

I checked source code, and it requires little bit more harder refactoring
because now we have SetVariableBool - what is unhappy name, because it
initialize variable to ON value. It is question what is better name?

I found more interesting issue - the code of SetResultVariables is
partially redundant with AcceptResult - maybe the switch there can be
shared.

Regards

Pavel

Show quoted text

Regards

Pavel

--
Fabien.

#12Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Pavel Stehule (#11)
1 attachment(s)
Re: psql - add special variable to reflect the last query status

Hello Pavel,

We can introduce macro SetVariableBool(vars, varname, bool) instead

SetVariable(pset.vars, "ERROR", "FALSE");

I checked source code, and it requires little bit more harder refactoring
because now we have SetVariableBool - what is unhappy name, because it
initialize variable to ON value. It is question what is better name?

The boolean values (on/off 1/0 true/false...) accepted for pg settings is
probably convenient but also somehow fuzzy.

From a programming point of view, I like booleans to have either true or
false values, and nothing else.

I agree that the existing "SetVariableBool" function is a misnommer, it
should be "SetVariableOn" given what it does, and it is not what we need.

Here is a v4 which attempts to extend & reuse the function. People might
be surprised that TRUE is used where ON was used before, so I'm not sure.

I found more interesting issue - the code of SetResultVariables is
partially redundant with AcceptResult - maybe the switch there can be
shared.

I agree that there is some common structure, but ISTM that the
AcceptResult function is called in a variety of situation where variables
are not to be set (eg "internal" queries, not user provided queries), so I
thought it best to keep the two apart.

--
Fabien.

Attachments:

psql-result-status-4.patchtext/x-diff; name=psql-result-status-4.patchDownload
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 9faa365..bc9a2e4 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -3452,6 +3452,35 @@ bar
       </varlistentry>
 
       <varlistentry>
+       <term><varname>ERROR</varname></term>
+       <listitem>
+        <para>
+         Whether the last query failed, as a boolean.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
+       <term><varname>ERROR_CODE</varname></term>
+       <listitem>
+        <para>
+         The error code associated to the last query, or
+         <literal>00000</> if no error occured.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
+       <term><varname>ERROR_MESSAGE</varname></term>
+       <listitem>
+        <para>
+         If the last query failed, the associated error message,
+         otherwise an empty string.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
         <term><varname>FETCH_COUNT</varname></term>
         <listitem>
         <para>
@@ -3656,6 +3685,24 @@ bar
       </varlistentry>
 
       <varlistentry>
+       <term><varname>STATUS</varname></term>
+       <listitem>
+        <para>
+         Text status of the last query.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
+       <term><varname>ROW_COUNT</varname></term>
+       <listitem>
+        <para>
+         How many rows were returned or affected by the last query.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
         <term><varname>QUIET</varname></term>
         <listitem>
         <para>
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 044cdb8..3abb3e7 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -1213,6 +1213,62 @@ PrintQueryResults(PGresult *results)
 	return success;
 }
 
+/*
+ * Set special variables for "front door" queries
+ * - STATUS: last query status
+ * - ERROR: TRUE/FALSE, whether an error occurred
+ * - ERROR_CODE: code if an error occured, or "00000"
+ * - ERROR_MESSAGE: message if an error occured, or ""
+ * - ROW_COUNT: how many rows were returned or affected, or "0"
+ */
+static void
+SetResultVariables(PGresult *results)
+{
+	bool			success;
+	ExecStatusType	restat = PQresultStatus(results);
+	char 		   *code = PQresultErrorField(results, PG_DIAG_SQLSTATE);
+	char 		   *mesg = PQresultErrorField(results, PG_DIAG_MESSAGE_PRIMARY);
+
+	SetVariable(pset.vars, "STATUS", PQresStatus(restat) + strlen("PGRES_"));
+	SetVariable(pset.vars, "ERROR_CODE", code ? code : "00000");
+	SetVariable(pset.vars, "ERROR_MESSAGE", mesg ? mesg : "");
+
+	/* check all possible PGRES_ */
+	switch (restat)
+	{
+		case PGRES_EMPTY_QUERY:
+		case PGRES_TUPLES_OK:
+		case PGRES_COMMAND_OK:
+		case PGRES_COPY_OUT:
+		case PGRES_COPY_IN:
+		case PGRES_COPY_BOTH:
+		case PGRES_SINGLE_TUPLE:
+			success = true;
+			break;
+		case PGRES_BAD_RESPONSE:
+		case PGRES_NONFATAL_ERROR:
+		case PGRES_FATAL_ERROR:
+			success = false;
+			break;
+		default:
+			/* dead code */
+			success = false;
+			psql_error("unexpected PQresultStatus: %d\n", restat);
+			break;
+	}
+
+	if (success)
+	{
+		char   *ntuples = PQcmdTuples(results);
+		SetVariable(pset.vars, "ROW_COUNT", *ntuples ? ntuples : "0");
+	}
+	else
+	{
+		SetVariable(pset.vars, "ROW_COUNT", "0");
+	}
+
+	SetVariableBool(pset.vars, "ERROR", !success);
+}
 
 /*
  * SendQuery: send the query string to the backend
@@ -1346,6 +1402,9 @@ SendQuery(const char *query)
 			elapsed_msec = INSTR_TIME_GET_MILLISEC(after);
 		}
 
+		/* set special variables to reflect the result status */
+		SetResultVariables(results);
+
 		/* but printing results isn't: */
 		if (OK && results)
 			OK = PrintQueryResults(results);
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index 7f76797..4ee2855 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -163,7 +163,7 @@ main(int argc, char *argv[])
 	SetVariable(pset.vars, "VERSION", PG_VERSION_STR);
 
 	/* Default values for variables (that don't match the result of \unset) */
-	SetVariableBool(pset.vars, "AUTOCOMMIT");
+	SetVariableBool(pset.vars, "AUTOCOMMIT", true);
 	SetVariable(pset.vars, "PROMPT1", DEFAULT_PROMPT1);
 	SetVariable(pset.vars, "PROMPT2", DEFAULT_PROMPT2);
 	SetVariable(pset.vars, "PROMPT3", DEFAULT_PROMPT3);
@@ -489,7 +489,7 @@ parse_psql_options(int argc, char *argv[], struct adhoc_opts *options)
 				SetVariable(pset.vars, "ECHO", "queries");
 				break;
 			case 'E':
-				SetVariableBool(pset.vars, "ECHO_HIDDEN");
+				SetVariableBool(pset.vars, "ECHO_HIDDEN", true);
 				break;
 			case 'f':
 				simple_action_list_append(&options->actions,
@@ -548,17 +548,17 @@ parse_psql_options(int argc, char *argv[], struct adhoc_opts *options)
 					break;
 				}
 			case 'q':
-				SetVariableBool(pset.vars, "QUIET");
+				SetVariableBool(pset.vars, "QUIET", true);
 				break;
 			case 'R':
 				pset.popt.topt.recordSep.separator = pg_strdup(optarg);
 				pset.popt.topt.recordSep.separator_zero = false;
 				break;
 			case 's':
-				SetVariableBool(pset.vars, "SINGLESTEP");
+				SetVariableBool(pset.vars, "SINGLESTEP", true);
 				break;
 			case 'S':
-				SetVariableBool(pset.vars, "SINGLELINE");
+				SetVariableBool(pset.vars, "SINGLELINE", true);
 				break;
 			case 't':
 				pset.popt.topt.tuples_only = true;
diff --git a/src/bin/psql/variables.c b/src/bin/psql/variables.c
index 806d39b..65aa6c6 100644
--- a/src/bin/psql/variables.c
+++ b/src/bin/psql/variables.c
@@ -361,12 +361,12 @@ SetVariableHooks(VariableSpace space, const char *name,
 }
 
 /*
- * Convenience function to set a variable's value to "on".
+ * Convenience function to set a variable's value to a boolean value
  */
 bool
-SetVariableBool(VariableSpace space, const char *name)
+SetVariableBool(VariableSpace space, const char *name, bool bval)
 {
-	return SetVariable(space, name, "on");
+	return SetVariable(space, name, bval ? "TRUE" : "FALSE");
 }
 
 /*
diff --git a/src/bin/psql/variables.h b/src/bin/psql/variables.h
index 02d85b1..1f2b0ba 100644
--- a/src/bin/psql/variables.h
+++ b/src/bin/psql/variables.h
@@ -84,7 +84,7 @@ bool ParseVariableNum(const char *value, const char *name,
 void		PrintVariables(VariableSpace space);
 
 bool		SetVariable(VariableSpace space, const char *name, const char *value);
-bool		SetVariableBool(VariableSpace space, const char *name);
+bool		SetVariableBool(VariableSpace space, const char *name, bool bval);
 bool		DeleteVariable(VariableSpace space, const char *name);
 
 void SetVariableHooks(VariableSpace space, const char *name,
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index d602aee..c3972a6 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -2904,6 +2904,69 @@ bar 'bar' "bar"
 	\echo 'should print #8-1'
 should print #8-1
 \endif
+-- special result variables
+-- 2 rows select
+SELECT 1 AS stuff UNION SELECT 2;
+ stuff 
+-------
+     1
+     2
+(2 rows)
+
+\if :ERROR
+  \echo 'MUST NOT SHOW'
+\else
+  \echo 'ERROR is FALSE as expected'
+ERROR is FALSE as expected
+\endif
+\echo 'status:' :STATUS
+status: TUPLES_OK
+\echo 'error code:' :ERROR_CODE
+error code: 00000
+\echo 'error message:' :ERROR_MESSAGE
+error message: 
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 2
+-- syntax error
+SELECT 1 UNION;
+ERROR:  syntax error at or near ";"
+LINE 1: SELECT 1 UNION;
+                      ^
+\echo 'status:' :STATUS
+status: FATAL_ERROR
+\if :ERROR
+  \echo 'ERROR is TRUE as expected'
+ERROR is TRUE as expected
+\else
+  \echo 'MUST NOT SHOW'
+\endif
+\echo 'error code:' :ERROR_CODE
+error code: 42601
+\echo 'error message:' :ERROR_MESSAGE
+error message: syntax error at or near ";"
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
+-- empty query
+;
+\echo 'status:' :STATUS
+status: EMPTY_QUERY
+\echo 'error code:' :ERROR_CODE
+error code: 00000
+\echo 'error message:' :ERROR_MESSAGE
+error message: 
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
+-- other query error
+DROP TABLE this_table_does_not_exist;
+ERROR:  table "this_table_does_not_exist" does not exist
+\echo 'status:' :STATUS
+status: FATAL_ERROR
+\echo 'error code:' :ERROR_CODE
+error code: 42P01
+\echo 'error message:' :ERROR_MESSAGE
+error message: table "this_table_does_not_exist" does not exist
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
 -- SHOW_CONTEXT
 \set SHOW_CONTEXT never
 do $$
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index b56a05f..f83ac70 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -526,6 +526,46 @@ select \if false \\ (bogus \else \\ 42 \endif \\ forty_two;
 	\echo 'should print #8-1'
 \endif
 
+-- special result variables
+
+-- 2 rows select
+SELECT 1 AS stuff UNION SELECT 2;
+\if :ERROR
+  \echo 'MUST NOT SHOW'
+\else
+  \echo 'ERROR is FALSE as expected'
+\endif
+\echo 'status:' :STATUS
+\echo 'error code:' :ERROR_CODE
+\echo 'error message:' :ERROR_MESSAGE
+\echo 'number of rows:' :ROW_COUNT
+
+-- syntax error
+SELECT 1 UNION;
+\echo 'status:' :STATUS
+\if :ERROR
+  \echo 'ERROR is TRUE as expected'
+\else
+  \echo 'MUST NOT SHOW'
+\endif
+\echo 'error code:' :ERROR_CODE
+\echo 'error message:' :ERROR_MESSAGE
+\echo 'number of rows:' :ROW_COUNT
+
+-- empty query
+;
+\echo 'status:' :STATUS
+\echo 'error code:' :ERROR_CODE
+\echo 'error message:' :ERROR_MESSAGE
+\echo 'number of rows:' :ROW_COUNT
+
+-- other query error
+DROP TABLE this_table_does_not_exist;
+\echo 'status:' :STATUS
+\echo 'error code:' :ERROR_CODE
+\echo 'error message:' :ERROR_MESSAGE
+\echo 'number of rows:' :ROW_COUNT
+
 -- SHOW_CONTEXT
 
 \set SHOW_CONTEXT never
#13Pavel Stehule
pavel.stehule@gmail.com
In reply to: Fabien COELHO (#12)
Re: psql - add special variable to reflect the last query status

2017-06-27 17:30 GMT+02:00 Fabien COELHO <coelho@cri.ensmp.fr>:

Hello Pavel,

We can introduce macro SetVariableBool(vars, varname, bool) instead

SetVariable(pset.vars, "ERROR", "FALSE");

I checked source code, and it requires little bit more harder refactoring
because now we have SetVariableBool - what is unhappy name, because it
initialize variable to ON value. It is question what is better name?

The boolean values (on/off 1/0 true/false...) accepted for pg settings is
probably convenient but also somehow fuzzy.

From a programming point of view, I like booleans to have either true or
false values, and nothing else.

I agree that the existing "SetVariableBool" function is a misnommer, it
should be "SetVariableOn" given what it does, and it is not what we need.

switching default setting from ON to TRUE requires wider discussion - in
this moment I like to have special function "SetVariableON".

Here is a v4 which attempts to extend & reuse the function. People might
be surprised that TRUE is used where ON was used before, so I'm not sure.

I found more interesting issue - the code of SetResultVariables is

partially redundant with AcceptResult - maybe the switch there can be
shared.

I agree that there is some common structure, but ISTM that the
AcceptResult function is called in a variety of situation where variables
are not to be set (eg "internal" queries, not user provided queries), so I
thought it best to keep the two apart.

I understand, but It is not nice, really - maybe only switch can be moved
to some inlining function like IsSuccess() - more .. with this function,
the SetResultVariables function will be more cleaner

Regards

Pavel

Show quoted text

--
Fabien.

#14Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Pavel Stehule (#13)
1 attachment(s)
Re: psql - add special variable to reflect the last query status

Hello Pavel,

I agree that the existing "SetVariableBool" function is a misnommer, it
should be "SetVariableOn" given what it does, and it is not what we
need.

switching default setting from ON to TRUE requires wider discussion -

Yep.

in this moment I like to have special function "SetVariableON".

I'm fine with this, but this make it a change totally unrelated to this
patch as it would not use the function... Moreover, this function would
not use an hypothetical "set var bool" function because of the debatable
on/off vs true/false change.

Also, a "set var bool" function would be called only twice, which is not
very beneficial for a oneliner, so I left it out.

I agree that there is some common structure, but ISTM that the
AcceptResult function is called in a variety of situation where variables
are not to be set (eg "internal" queries, not user provided queries), so I
thought it best to keep the two apart.

I understand, but It is not nice, really - maybe only switch can be moved
to some inlining function like IsSuccess() - more .. with this function,
the SetResultVariables function will be more cleaner

Indeed. Attached v5 does that.

--
Fabien.

Attachments:

psql-result-status-5.patchtext/x-diff; name=psql-result-status-5.patchDownload
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 9faa365..bc9a2e4 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -3452,6 +3452,35 @@ bar
       </varlistentry>
 
       <varlistentry>
+       <term><varname>ERROR</varname></term>
+       <listitem>
+        <para>
+         Whether the last query failed, as a boolean.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
+       <term><varname>ERROR_CODE</varname></term>
+       <listitem>
+        <para>
+         The error code associated to the last query, or
+         <literal>00000</> if no error occured.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
+       <term><varname>ERROR_MESSAGE</varname></term>
+       <listitem>
+        <para>
+         If the last query failed, the associated error message,
+         otherwise an empty string.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
         <term><varname>FETCH_COUNT</varname></term>
         <listitem>
         <para>
@@ -3656,6 +3685,24 @@ bar
       </varlistentry>
 
       <varlistentry>
+       <term><varname>STATUS</varname></term>
+       <listitem>
+        <para>
+         Text status of the last query.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
+       <term><varname>ROW_COUNT</varname></term>
+       <listitem>
+        <para>
+         How many rows were returned or affected by the last query.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
         <term><varname>QUIET</varname></term>
         <listitem>
         <para>
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 044cdb8..02fa89a 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -492,6 +492,48 @@ ResetCancelConn(void)
 #endif
 }
 
+/*
+ * ResultIsSuccess
+ *
+ * Tell whether query result is a success.
+ */
+static bool
+ResultIsSuccess(const PGresult *result)
+{
+	bool success;
+	if (!result)
+		success = false;
+	else
+	{
+		ExecStatusType	restat = PQresultStatus(result);
+
+		/* check all possible PGRES_ */
+		switch (restat)
+		{
+			case PGRES_EMPTY_QUERY:
+			case PGRES_TUPLES_OK:
+			case PGRES_COMMAND_OK:
+			case PGRES_COPY_OUT:
+			case PGRES_COPY_IN:
+			case PGRES_COPY_BOTH:
+			case PGRES_SINGLE_TUPLE:
+				success = true;
+				break;
+			case PGRES_BAD_RESPONSE:
+			case PGRES_NONFATAL_ERROR:
+			case PGRES_FATAL_ERROR:
+				success = false;
+				break;
+			default:
+				/* dead code */
+				success = false;
+				psql_error("unexpected PQresultStatus: %d\n", restat);
+				break;
+		}
+	}
+
+	return success;
+}
 
 /*
  * AcceptResult
@@ -504,34 +546,7 @@ ResetCancelConn(void)
 static bool
 AcceptResult(const PGresult *result)
 {
-	bool		OK;
-
-	if (!result)
-		OK = false;
-	else
-		switch (PQresultStatus(result))
-		{
-			case PGRES_COMMAND_OK:
-			case PGRES_TUPLES_OK:
-			case PGRES_EMPTY_QUERY:
-			case PGRES_COPY_IN:
-			case PGRES_COPY_OUT:
-				/* Fine, do nothing */
-				OK = true;
-				break;
-
-			case PGRES_BAD_RESPONSE:
-			case PGRES_NONFATAL_ERROR:
-			case PGRES_FATAL_ERROR:
-				OK = false;
-				break;
-
-			default:
-				OK = false;
-				psql_error("unexpected PQresultStatus: %d\n",
-						   PQresultStatus(result));
-				break;
-		}
+	bool		OK = ResultIsSuccess(result);
 
 	if (!OK)
 	{
@@ -1213,6 +1228,38 @@ PrintQueryResults(PGresult *results)
 	return success;
 }
 
+/*
+ * Set special variables for "front door" queries
+ * - STATUS: last query status
+ * - ERROR: TRUE/FALSE, whether an error occurred
+ * - ERROR_CODE: code if an error occured, or "00000"
+ * - ERROR_MESSAGE: message if an error occured, or ""
+ * - ROW_COUNT: how many rows were returned or affected, or "0"
+ */
+static void
+SetResultVariables(PGresult *results)
+{
+	bool			success = ResultIsSuccess(results);
+	ExecStatusType	restat = PQresultStatus(results);
+	char 		   *code = PQresultErrorField(results, PG_DIAG_SQLSTATE);
+	char 		   *mesg = PQresultErrorField(results, PG_DIAG_MESSAGE_PRIMARY);
+
+	SetVariable(pset.vars, "STATUS", PQresStatus(restat) + strlen("PGRES_"));
+	SetVariable(pset.vars, "ERROR_CODE", code ? code : "00000");
+	SetVariable(pset.vars, "ERROR_MESSAGE", mesg ? mesg : "");
+
+	if (success)
+	{
+		char   *ntuples = PQcmdTuples(results);
+		SetVariable(pset.vars, "ROW_COUNT", *ntuples ? ntuples : "0");
+		SetVariable(pset.vars, "ERROR", "FALSE");
+	}
+	else
+	{
+		SetVariable(pset.vars, "ROW_COUNT", "0");
+		SetVariable(pset.vars, "ERROR", "TRUE");
+	}
+}
 
 /*
  * SendQuery: send the query string to the backend
@@ -1346,6 +1393,9 @@ SendQuery(const char *query)
 			elapsed_msec = INSTR_TIME_GET_MILLISEC(after);
 		}
 
+		/* set special variables to reflect the result status */
+		SetResultVariables(results);
+
 		/* but printing results isn't: */
 		if (OK && results)
 			OK = PrintQueryResults(results);
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index d602aee..c3972a6 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -2904,6 +2904,69 @@ bar 'bar' "bar"
 	\echo 'should print #8-1'
 should print #8-1
 \endif
+-- special result variables
+-- 2 rows select
+SELECT 1 AS stuff UNION SELECT 2;
+ stuff 
+-------
+     1
+     2
+(2 rows)
+
+\if :ERROR
+  \echo 'MUST NOT SHOW'
+\else
+  \echo 'ERROR is FALSE as expected'
+ERROR is FALSE as expected
+\endif
+\echo 'status:' :STATUS
+status: TUPLES_OK
+\echo 'error code:' :ERROR_CODE
+error code: 00000
+\echo 'error message:' :ERROR_MESSAGE
+error message: 
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 2
+-- syntax error
+SELECT 1 UNION;
+ERROR:  syntax error at or near ";"
+LINE 1: SELECT 1 UNION;
+                      ^
+\echo 'status:' :STATUS
+status: FATAL_ERROR
+\if :ERROR
+  \echo 'ERROR is TRUE as expected'
+ERROR is TRUE as expected
+\else
+  \echo 'MUST NOT SHOW'
+\endif
+\echo 'error code:' :ERROR_CODE
+error code: 42601
+\echo 'error message:' :ERROR_MESSAGE
+error message: syntax error at or near ";"
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
+-- empty query
+;
+\echo 'status:' :STATUS
+status: EMPTY_QUERY
+\echo 'error code:' :ERROR_CODE
+error code: 00000
+\echo 'error message:' :ERROR_MESSAGE
+error message: 
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
+-- other query error
+DROP TABLE this_table_does_not_exist;
+ERROR:  table "this_table_does_not_exist" does not exist
+\echo 'status:' :STATUS
+status: FATAL_ERROR
+\echo 'error code:' :ERROR_CODE
+error code: 42P01
+\echo 'error message:' :ERROR_MESSAGE
+error message: table "this_table_does_not_exist" does not exist
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
 -- SHOW_CONTEXT
 \set SHOW_CONTEXT never
 do $$
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index b56a05f..f83ac70 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -526,6 +526,46 @@ select \if false \\ (bogus \else \\ 42 \endif \\ forty_two;
 	\echo 'should print #8-1'
 \endif
 
+-- special result variables
+
+-- 2 rows select
+SELECT 1 AS stuff UNION SELECT 2;
+\if :ERROR
+  \echo 'MUST NOT SHOW'
+\else
+  \echo 'ERROR is FALSE as expected'
+\endif
+\echo 'status:' :STATUS
+\echo 'error code:' :ERROR_CODE
+\echo 'error message:' :ERROR_MESSAGE
+\echo 'number of rows:' :ROW_COUNT
+
+-- syntax error
+SELECT 1 UNION;
+\echo 'status:' :STATUS
+\if :ERROR
+  \echo 'ERROR is TRUE as expected'
+\else
+  \echo 'MUST NOT SHOW'
+\endif
+\echo 'error code:' :ERROR_CODE
+\echo 'error message:' :ERROR_MESSAGE
+\echo 'number of rows:' :ROW_COUNT
+
+-- empty query
+;
+\echo 'status:' :STATUS
+\echo 'error code:' :ERROR_CODE
+\echo 'error message:' :ERROR_MESSAGE
+\echo 'number of rows:' :ROW_COUNT
+
+-- other query error
+DROP TABLE this_table_does_not_exist;
+\echo 'status:' :STATUS
+\echo 'error code:' :ERROR_CODE
+\echo 'error message:' :ERROR_MESSAGE
+\echo 'number of rows:' :ROW_COUNT
+
 -- SHOW_CONTEXT
 
 \set SHOW_CONTEXT never
#15Pavel Stehule
pavel.stehule@gmail.com
In reply to: Fabien COELHO (#14)
Re: psql - add special variable to reflect the last query status

2017-06-28 9:25 GMT+02:00 Fabien COELHO <coelho@cri.ensmp.fr>:

Hello Pavel,

I agree that the existing "SetVariableBool" function is a misnommer, it

should be "SetVariableOn" given what it does, and it is not what we need.

switching default setting from ON to TRUE requires wider discussion -

Yep.

in this moment I like to have special function "SetVariableON".

I'm fine with this, but this make it a change totally unrelated to this
patch as it would not use the function... Moreover, this function would not
use an hypothetical "set var bool" function because of the debatable on/off
vs true/false change.

Also, a "set var bool" function would be called only twice, which is not
very beneficial for a oneliner, so I left it out.

I agree that there is some common structure, but ISTM that the

AcceptResult function is called in a variety of situation where variables
are not to be set (eg "internal" queries, not user provided queries), so
I
thought it best to keep the two apart.

I understand, but It is not nice, really - maybe only switch can be moved
to some inlining function like IsSuccess() - more .. with this function,
the SetResultVariables function will be more cleaner

Indeed. Attached v5 does that.

juju - something like this

+ if (success)
+ {
+ char   *ntuples = PQcmdTuples(results);
+ SetVariable(pset.vars, "ROW_COUNT", *ntuples ? ntuples : "0");
+ SetVariable(pset.vars, "ERROR", "FALSE");
+ }
+ else
+ {
+ SetVariable(pset.vars, "ROW_COUNT", "0");
+ SetVariable(pset.vars, "ERROR", "TRUE");
+ }
+}

It can be simplified

SetVariable(pset.vars, "ROW_COUNT", success ? PQcmdTuples(results) : 0);
SetVariable(pset.vars, "ERROR", success ? "FALSE" : "TRUE");

Regards

Pavel

Show quoted text

--
Fabien.

#16Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Pavel Stehule (#15)
Re: psql - add special variable to reflect the last query status

Hello Pavel,

+ if (success)
+ {
+ char   *ntuples = PQcmdTuples(results);
+ SetVariable(pset.vars, "ROW_COUNT", *ntuples ? ntuples : "0");
+ SetVariable(pset.vars, "ERROR", "FALSE");
+ }
+ else
+ {
+ SetVariable(pset.vars, "ROW_COUNT", "0");
+ SetVariable(pset.vars, "ERROR", "TRUE");
+ }
+}

It can be simplified

SetVariable(pset.vars, "ROW_COUNT", success ? PQcmdTuples(results) : 0);

According to the documentation, PQcmdTuples returns "" in some cases and
ISTM we want "0" instead for consistency, so that it is always a number. I
rejected calling PQcmdTuples twice:

..., success && *PQcmdTuples(results) ? PQcmdTuples(results) : "0")

Thus it makes the "if (success)" necessary for ROW_COUNT, and then it
looked simpler to handle ERROR the same way.

Now if the semantics is changed to put as row count whatever comes out of
the function, even if not a count, then the code could indeed be
simplified as you suggest.

--
Fabien.

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

#17Pavel Stehule
pavel.stehule@gmail.com
In reply to: Fabien COELHO (#16)
Re: psql - add special variable to reflect the last query status

2017-06-28 10:04 GMT+02:00 Fabien COELHO <coelho@cri.ensmp.fr>:

Hello Pavel,

+ if (success)

+ {
+ char   *ntuples = PQcmdTuples(results);
+ SetVariable(pset.vars, "ROW_COUNT", *ntuples ? ntuples : "0");
+ SetVariable(pset.vars, "ERROR", "FALSE");
+ }
+ else
+ {
+ SetVariable(pset.vars, "ROW_COUNT", "0");
+ SetVariable(pset.vars, "ERROR", "TRUE");
+ }
+}

It can be simplified

SetVariable(pset.vars, "ROW_COUNT", success ? PQcmdTuples(results) : 0);

According to the documentation, PQcmdTuples returns "" in some cases and
ISTM we want "0" instead for consistency, so that it is always a number. I
rejected calling PQcmdTuples twice:

..., success && *PQcmdTuples(results) ? PQcmdTuples(results) : "0")

Thus it makes the "if (success)" necessary for ROW_COUNT, and then it
looked simpler to handle ERROR the same way.

Now if the semantics is changed to put as row count whatever comes out of
the function, even if not a count, then the code could indeed be simplified
as you suggest.

Understand

Pavel

Show quoted text

--
Fabien.

#18Tom Lane
tgl@sss.pgh.pa.us
In reply to: Pavel Stehule (#17)
Re: psql - add special variable to reflect the last query status

A few thoughts about this patch:

* I think the ERROR_CODE variable should instead be named SQLSTATE.
That is what the SQL standard calls this string, and it's also what
just about all our documentation calls it; see PG_DIAG_SQLSTATE
in libpq, or the SQLSTATE 'xxxxx' construct in pl/pgsql, or the
sqlstate attribute of an exception object in plpython, etc etc.

* I'm not exactly convinced that there's a use-case for STATUS
that's not covered as well or better by ERROR. Client code that
looks at PQresStatus for anything beyond error/not-error is
usually doing that because it's library code that doesn't know
what kind of query it's working on. It seems like a stretch that
a psql script would not know that. Also, PQresultStatus memorializes
some legacy distinctions, like "fatal" vs "nonfatal" error, that
I think we'd be better off not exposing in psql scripting.

* It might be better if SQLSTATE and ERROR_MESSAGE were left
unchanged by a non-error query. That would reduce the need to
copy them into other variables just because you needed to do
something else before printing them. It'd save a few cycles too.

* Speaking of which, has anyone tried to quantify the performance
impact of this patch? It might well be negligible, but I do not
think we should assume that without evidence.

* I wonder why you didn't merge this processing into ProcessResult,
instead of inventing an extra function (whose call site seems rather
poorly chosen anyhow --- what's the justification for not counting this
overhead as part of the query runtime?). You could probably eliminate
the refactoring you did, since it wouldn't be necessary to recompute
AcceptResult's result that way.

regards, tom lane

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

#19Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Tom Lane (#18)
Re: psql - add special variable to reflect the last query status

Hello Tom,

* I think the ERROR_CODE variable should instead be named SQLSTATE.
That is what the SQL standard calls this string, and it's also what
just about all our documentation calls it; see PG_DIAG_SQLSTATE
in libpq, or the SQLSTATE 'xxxxx' construct in pl/pgsql, or the
sqlstate attribute of an exception object in plpython, etc etc.

I choose ERROR_CODE because it matched the ERROR boolean. But is may be a
misnomer if the status is that all is well. I'm okay with SQLSTATE.

* I'm not exactly convinced that there's a use-case for STATUS that's
not covered as well or better by ERROR. Client code that looks at
PQresStatus for anything beyond error/not-error is usually doing that
because it's library code that doesn't know what kind of query it's
working on. It seems like a stretch that a psql script would not know
that. Also, PQresultStatus memorializes some legacy distinctions, like
"fatal" vs "nonfatal" error, that I think we'd be better off not
exposing in psql scripting.

Ok.

* It might be better if SQLSTATE and ERROR_MESSAGE were left
unchanged by a non-error query.

Hmmm. I'm not sure. If so, ERROR_MESSAGE should be LAST_ERROR_MESSAGE
maybe? Then what about LAST_ERROR_SQLSTATE to go with it, and let SQLSTATE
& ERROR_MESSAGE reflect the last command, and ERROR is just the boolean to
test if it occured?

That would reduce the need to copy them into other variables just
because you needed to do something else before printing them. It'd save
a few cycles too.

Well, my suggestion would mean that they would be copied when an error
occurs, but only when it occurs, which would not be often.

If you would like them, I'm not sure how these variable should be
initialized. Undefined? Empty?

* Speaking of which, has anyone tried to quantify the performance
impact of this patch? It might well be negligible, but I do not
think we should assume that without evidence.

I think it should be negligible compared to network connections, aborting
an ongoing transaction, reading the script...

But I do not know libpq internals so I may be quite naive.

* I wonder why you didn't merge this processing into ProcessResult,
instead of inventing an extra function (whose call site seems rather
poorly chosen anyhow --- what's the justification for not counting this
overhead as part of the query runtime?). You could probably eliminate
the refactoring you did, since it wouldn't be necessary to recompute
AcceptResult's result that way.

Hmmm. I assume that you are unhappy about ResultIsSuccess.

The refactoring is because the function is used twice. I choose to do that
because the functionality is clear and could be made as a function which
improved readability. Ok, PQresultStatus is thus called twice, I assumed
that it is just reading a field in a struct... it could be returned to the
caller with an additional pointer to avoid that.

The SendResult & ProcessResult functions are already quite heavy to my
taste, I did not want to add significantly to that.

The ProcessResult switch does not test all states cleanly, it is really
about checking about copy, and not so clear, so I do not think that it
would mix well to add the variable stuff in the middle of that.

SendQuery is also pretty complex, including gotos everywhere.

So I did want to add to these two functions beyond the minimum. Now, I can
also inline everything coldly in ProcessResult, no problem.

--
Fabien.

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

#20Tom Lane
tgl@sss.pgh.pa.us
In reply to: Fabien COELHO (#19)
Re: psql - add special variable to reflect the last query status

Fabien COELHO <coelho@cri.ensmp.fr> writes:

* It might be better if SQLSTATE and ERROR_MESSAGE were left
unchanged by a non-error query.

Hmmm. I'm not sure. If so, ERROR_MESSAGE should be LAST_ERROR_MESSAGE
maybe? Then what about LAST_ERROR_SQLSTATE to go with it, and let SQLSTATE
& ERROR_MESSAGE reflect the last command, and ERROR is just the boolean to
test if it occured?

That would reduce the need to copy them into other variables just
because you needed to do something else before printing them. It'd save
a few cycles too.

Well, my suggestion would mean that they would be copied when an error
occurs, but only when it occurs, which would not be often.

Uh ... what?

regards, tom lane

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

#21Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Tom Lane (#20)
Re: psql - add special variable to reflect the last query status

* It might be better if SQLSTATE and ERROR_MESSAGE were left
unchanged by a non-error query.

Hmmm. I'm not sure. If so, ERROR_MESSAGE should be LAST_ERROR_MESSAGE
maybe? Then what about LAST_ERROR_SQLSTATE to go with it, and let SQLSTATE
& ERROR_MESSAGE reflect the last command, and ERROR is just the boolean to
test if it occured?

That would reduce the need to copy them into other variables just
because you needed to do something else before printing them. It'd save
a few cycles too.

Well, my suggestion would mean that they would be copied when an error
occurs, but only when it occurs, which would not be often.

Uh ... what?

Sorry if my sentence was not very clear. Time to go do bed:-)

I just mean that a LAST_ERROR_* would be set when an error occurs. When
there is no error, it is expected to remain the same, and it does not cost
anything to let it as is. If an error occured then you had a problem, a
transaction aborted, paying to set a few variables when it occurs does not
look like a big performance issue. Script usually expect to run without
error, errors are rare events.

--
Fabien.

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

#22Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Tom Lane (#18)
1 attachment(s)
Re: psql - add special variable to reflect the last query status

Hello Tom,

Here is a version 6.

A few thoughts about this patch:

* I think the ERROR_CODE variable should instead be named SQLSTATE.
That is what the SQL standard calls this string, and it's also what
just about all our documentation calls it; see PG_DIAG_SQLSTATE
in libpq, or the SQLSTATE 'xxxxx' construct in pl/pgsql, or the
sqlstate attribute of an exception object in plpython, etc etc.

ERROR_CODE -> SQLSTATE.

* I'm not exactly convinced that there's a use-case for STATUS

Removed, but I think it was nice to have, it is easier to interpret than
error codes and their classes that I have not memorized yet:-)

* It might be better if SQLSTATE and ERROR_MESSAGE were left
unchanged by a non-error query. That would reduce the need to
copy them into other variables just because you needed to do
something else before printing them. It'd save a few cycles too.

Added LAST_ERROR_SQLSTATE & MESSAGE, only reset when an error occured.

* Speaking of which, has anyone tried to quantify the performance
impact of this patch? It might well be negligible, but I do not
think we should assume that without evidence.

My guess is negligible. Not sure how to measure this negligible, as many
very fast query should be executed to have something significant. Maybe
100,000 "SELECT 1;" in a script?

* I wonder why you didn't merge this processing into ProcessResult,
instead of inventing an extra function (whose call site seems rather
poorly chosen anyhow --- what's the justification for not counting this
overhead as part of the query runtime?). You could probably eliminate
the refactoring you did, since it wouldn't be necessary to recompute
AcceptResult's result that way.

Variable setting moved at then end of ProcessResult, no new functions,
result is clean, so I should have done it like that in the beginning.

Forgotten help stuff added.

--
Fabien.

Attachments:

psql-result-status-6.patchtext/x-diff; name=psql-result-status-6.patchDownload
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 5bdbc1e..0bbe1c9 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -3475,6 +3475,16 @@ bar
       </varlistentry>
 
       <varlistentry>
+       <term><varname>ERROR</varname></term>
+       <listitem>
+        <para>
+         Whether the last query failed, as a boolean.
+         See also <varname>SQLSTATE</>.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
         <term><varname>FETCH_COUNT</varname></term>
         <listitem>
         <para>
@@ -3611,6 +3621,18 @@ bar
       </varlistentry>
 
       <varlistentry>
+       <term><varname>LAST_ERROR_SQLSTATE</varname></term>
+       <term><varname>LAST_ERROR_MESSAGE</varname></term>
+       <listitem>
+        <para>
+         The error code and associated error message of the last
+         error, or empty strings if no error occured since the
+         beginning of the script.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
       <term>
        <varname>ON_ERROR_ROLLBACK</varname>
        <indexterm>
@@ -3679,6 +3701,25 @@ bar
       </varlistentry>
 
       <varlistentry>
+       <term><varname>ROW_COUNT</varname></term>
+       <listitem>
+        <para>
+         How many rows were returned or affected by the last query.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
+       <term><varname>SQLSTATE</varname></term>
+       <listitem>
+        <para>
+         The error code associated to the last query, or
+         <literal>00000</> if no error occured.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
         <term><varname>QUIET</varname></term>
         <listitem>
         <para>
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index b997058..93950fd 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -493,7 +493,6 @@ ResetCancelConn(void)
 #endif
 }
 
-
 /*
  * AcceptResult
  *
@@ -1107,6 +1106,35 @@ ProcessResult(PGresult **results)
 		first_cycle = false;
 	}
 
+	/*
+	 * Set special variables
+	 * - ERROR: TRUE/FALSE, whether an error occurred
+	 * - SQLSTATE: code of error, or "00000"
+	 * - LAST_ERROR_SQLSTATE: same for last error
+	 * - LAST_ERROR_MESSAGE: message of last error
+	 * - ROW_COUNT: how many rows were returned or affected, or "0"
+	*/
+	if (success)
+	{
+		char   *ntuples = PQcmdTuples(*results);
+		SetVariable(pset.vars, "ERROR", "FALSE");
+		SetVariable(pset.vars, "SQLSTATE", "00000");
+		SetVariable(pset.vars, "ROW_COUNT", *ntuples ? ntuples : "0");
+	}
+	else
+	{
+		char 		   *code = PQresultErrorField(*results, PG_DIAG_SQLSTATE);
+		char 		   *mesg = PQresultErrorField(*results, PG_DIAG_MESSAGE_PRIMARY);
+
+		SetVariable(pset.vars, "ERROR", "TRUE");
+		/* if an error was detected, it must have a code! */
+		Assert(code != NULL);
+		SetVariable(pset.vars, "SQLSTATE", code);
+		SetVariable(pset.vars, "LAST_ERROR_SQLSTATE", code);
+		SetVariable(pset.vars, "LAST_ERROR_MESSAGE", mesg ? mesg : "");
+		SetVariable(pset.vars, "ROW_COUNT", "0");
+	}
+
 	/* may need this to recover from conn loss during COPY */
 	if (!first_cycle && !CheckConnection())
 		return false;
@@ -1214,7 +1242,6 @@ PrintQueryResults(PGresult *results)
 	return success;
 }
 
-
 /*
  * SendQuery: send the query string to the backend
  * (and print out results)
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 4d1c0ec..ae951f5 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -337,7 +337,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(147, pager ? &(pset.popt.topt) : NULL);
+	output = PageOutput(155, pager ? &(pset.popt.topt) : NULL);
 
 	fprintf(output, _("List of specially treated variables\n\n"));
 
@@ -360,6 +360,8 @@ helpVariables(unsigned short int pager)
 					  "    if set to \"noexec\", just show them without execution\n"));
 	fprintf(output, _("  ENCODING\n"
 					  "    current client character set encoding\n"));
+	fprintf(output, _("  ERROR\n"
+					  "    whether the last query failed\n"));
 	fprintf(output, _("  FETCH_COUNT\n"
 					  "    the number of result rows to fetch and display at a time (0 = unlimited)\n"));
 	fprintf(output, _("  HISTCONTROL\n"
@@ -374,6 +376,9 @@ helpVariables(unsigned short int pager)
 					  "    number of EOFs needed to terminate an interactive session\n"));
 	fprintf(output, _("  LASTOID\n"
 					  "    value of the last affected OID\n"));
+	fprintf(output, _("  LAST_ERROR_SQLSTATE\n"
+					  "  LAST_ERROR_MESSAGE\n"
+					  "    error code and message of last error, or \"00000\" and empty if none\n"));
 	fprintf(output, _("  ON_ERROR_ROLLBACK\n"
 					  "    if set, an error doesn't stop a transaction (uses implicit savepoints)\n"));
 	fprintf(output, _("  ON_ERROR_STOP\n"
@@ -388,6 +393,8 @@ helpVariables(unsigned short int pager)
 					  "    specifies the prompt used during COPY ... FROM STDIN\n"));
 	fprintf(output, _("  QUIET\n"
 					  "    run quietly (same as -q option)\n"));
+	fprintf(output, _("  ROW_COUNT\n"
+					  "    number of rows of last query, or 0\n"));
 	fprintf(output, _("  SERVER_VERSION_NAME\n"
 					  "  SERVER_VERSION_NUM\n"
 					  "    server's version (in short string or numeric format)\n"));
@@ -397,6 +404,8 @@ helpVariables(unsigned short int pager)
 					  "    if set, end of line terminates SQL commands (same as -S option)\n"));
 	fprintf(output, _("  SINGLESTEP\n"
 					  "    single-step mode (same as -s option)\n"));
+	fprintf(output, _("  SQLSTATE\n"
+					  "    error code of last query, or \"00000\" if no error\n"));
 	fprintf(output, _("  USER\n"
 					  "    the currently connected database user\n"));
 	fprintf(output, _("  VERBOSITY\n"
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index 1e48f4a..d020f3f 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -165,6 +165,10 @@ main(int argc, char *argv[])
 	SetVariable(pset.vars, "VERSION_NAME", PG_VERSION);
 	SetVariable(pset.vars, "VERSION_NUM", CppAsString2(PG_VERSION_NUM));
 
+	/* Create variables for last error */
+	SetVariable(pset.vars, "LAST_ERROR_SQLSTATE", "00000");
+	SetVariable(pset.vars, "LAST_ERROR_MESSAGE", "");
+
 	/* Default values for variables (that don't match the result of \unset) */
 	SetVariableBool(pset.vars, "AUTOCOMMIT");
 	SetVariable(pset.vars, "PROMPT1", DEFAULT_PROMPT1);
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index 7957268..397a1de 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -28,6 +28,74 @@ on
 \unset ON_ERROR_ROLLBACK
 \echo :ON_ERROR_ROLLBACK
 off
+-- special result variables
+-- these test are performed early to check that for values after startup
+-- 2 rows select
+SELECT 1 AS stuff UNION SELECT 2;
+ stuff 
+-------
+     1
+     2
+(2 rows)
+
+\if :ERROR
+  \echo 'MUST NOT SHOW'
+\else
+  \echo 'ERROR is FALSE as expected'
+ERROR is FALSE as expected
+\endif
+\echo 'error code:' :SQLSTATE
+error code: 00000
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+last error code: 00000
+\echo 'last error message:' :LAST_ERROR_MESSAGE
+last error message: 
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 2
+-- syntax error
+SELECT 1 UNION;
+ERROR:  syntax error at or near ";"
+LINE 1: SELECT 1 UNION;
+                      ^
+\if :ERROR
+  \echo 'ERROR is TRUE as expected'
+ERROR is TRUE as expected
+\else
+  \echo 'MUST NOT SHOW'
+\endif
+\echo 'error code:' :SQLSTATE
+error code: 42601
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+last error code: 42601
+\echo 'error message:' :LAST_ERROR_MESSAGE
+error message: syntax error at or near ";"
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
+-- empty query
+;
+\echo 'error:' :ERROR
+error: FALSE
+\echo 'error code:' :SQLSTATE
+error code: 00000
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+last error code: 42601
+\echo 'error message:' :LAST_ERROR_MESSAGE
+error message: syntax error at or near ";"
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
+-- other query error
+DROP TABLE this_table_does_not_exist;
+ERROR:  table "this_table_does_not_exist" does not exist
+\echo 'error:' :ERROR
+error: TRUE
+\echo 'error code:' :SQLSTATE
+error code: 42P01
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+last error code: 42P01
+\echo 'error message:' :LAST_ERROR_MESSAGE
+error message: table "this_table_does_not_exist" does not exist
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
 -- \g and \gx
 SELECT 1 as one, 2 as two \g
  one | two 
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index 0556b7c..3713098 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -21,6 +21,49 @@
 \unset ON_ERROR_ROLLBACK
 \echo :ON_ERROR_ROLLBACK
 
+-- special result variables
+-- these test are performed early to check that for values after startup
+
+-- 2 rows select
+SELECT 1 AS stuff UNION SELECT 2;
+\if :ERROR
+  \echo 'MUST NOT SHOW'
+\else
+  \echo 'ERROR is FALSE as expected'
+\endif
+\echo 'error code:' :SQLSTATE
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+\echo 'last error message:' :LAST_ERROR_MESSAGE
+\echo 'number of rows:' :ROW_COUNT
+
+-- syntax error
+SELECT 1 UNION;
+\if :ERROR
+  \echo 'ERROR is TRUE as expected'
+\else
+  \echo 'MUST NOT SHOW'
+\endif
+\echo 'error code:' :SQLSTATE
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+\echo 'error message:' :LAST_ERROR_MESSAGE
+\echo 'number of rows:' :ROW_COUNT
+
+-- empty query
+;
+\echo 'error:' :ERROR
+\echo 'error code:' :SQLSTATE
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+\echo 'error message:' :LAST_ERROR_MESSAGE
+\echo 'number of rows:' :ROW_COUNT
+
+-- other query error
+DROP TABLE this_table_does_not_exist;
+\echo 'error:' :ERROR
+\echo 'error code:' :SQLSTATE
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+\echo 'error message:' :LAST_ERROR_MESSAGE
+\echo 'number of rows:' :ROW_COUNT
+
 -- \g and \gx
 
 SELECT 1 as one, 2 as two \g
#23Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Fabien COELHO (#22)
1 attachment(s)
Re: psql - add special variable to reflect the last query status

Here is a version 6.

Small v7 update, sorry for the noise.

Add testing the initial state of all variables.

Fix typos in a comment in tests.

Fix the documentation wrt the current implementation behavior.

--
Fabien.

Attachments:

psql-result-status-7.patchtext/x-diff; name=psql-result-status-7.patchDownload
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 5bdbc1e..97962d4 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -3475,6 +3475,16 @@ bar
       </varlistentry>
 
       <varlistentry>
+       <term><varname>ERROR</varname></term>
+       <listitem>
+        <para>
+         Whether the last query failed, as a boolean.
+         See also <varname>SQLSTATE</>.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
         <term><varname>FETCH_COUNT</varname></term>
         <listitem>
         <para>
@@ -3611,6 +3621,18 @@ bar
       </varlistentry>
 
       <varlistentry>
+       <term><varname>LAST_ERROR_SQLSTATE</varname></term>
+       <term><varname>LAST_ERROR_MESSAGE</varname></term>
+       <listitem>
+        <para>
+         The error code and associated error message of the last
+         error, or "00000" and empty strings if no error occured
+         since the beginning of the script.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
       <term>
        <varname>ON_ERROR_ROLLBACK</varname>
        <indexterm>
@@ -3679,6 +3701,25 @@ bar
       </varlistentry>
 
       <varlistentry>
+       <term><varname>ROW_COUNT</varname></term>
+       <listitem>
+        <para>
+         How many rows were returned or affected by the last query.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
+       <term><varname>SQLSTATE</varname></term>
+       <listitem>
+        <para>
+         The error code associated to the last query, or
+         <literal>00000</> if no error occured.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
         <term><varname>QUIET</varname></term>
         <listitem>
         <para>
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index b997058..93950fd 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -493,7 +493,6 @@ ResetCancelConn(void)
 #endif
 }
 
-
 /*
  * AcceptResult
  *
@@ -1107,6 +1106,35 @@ ProcessResult(PGresult **results)
 		first_cycle = false;
 	}
 
+	/*
+	 * Set special variables
+	 * - ERROR: TRUE/FALSE, whether an error occurred
+	 * - SQLSTATE: code of error, or "00000"
+	 * - LAST_ERROR_SQLSTATE: same for last error
+	 * - LAST_ERROR_MESSAGE: message of last error
+	 * - ROW_COUNT: how many rows were returned or affected, or "0"
+	*/
+	if (success)
+	{
+		char   *ntuples = PQcmdTuples(*results);
+		SetVariable(pset.vars, "ERROR", "FALSE");
+		SetVariable(pset.vars, "SQLSTATE", "00000");
+		SetVariable(pset.vars, "ROW_COUNT", *ntuples ? ntuples : "0");
+	}
+	else
+	{
+		char 		   *code = PQresultErrorField(*results, PG_DIAG_SQLSTATE);
+		char 		   *mesg = PQresultErrorField(*results, PG_DIAG_MESSAGE_PRIMARY);
+
+		SetVariable(pset.vars, "ERROR", "TRUE");
+		/* if an error was detected, it must have a code! */
+		Assert(code != NULL);
+		SetVariable(pset.vars, "SQLSTATE", code);
+		SetVariable(pset.vars, "LAST_ERROR_SQLSTATE", code);
+		SetVariable(pset.vars, "LAST_ERROR_MESSAGE", mesg ? mesg : "");
+		SetVariable(pset.vars, "ROW_COUNT", "0");
+	}
+
 	/* may need this to recover from conn loss during COPY */
 	if (!first_cycle && !CheckConnection())
 		return false;
@@ -1214,7 +1242,6 @@ PrintQueryResults(PGresult *results)
 	return success;
 }
 
-
 /*
  * SendQuery: send the query string to the backend
  * (and print out results)
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 4d1c0ec..ae951f5 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -337,7 +337,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(147, pager ? &(pset.popt.topt) : NULL);
+	output = PageOutput(155, pager ? &(pset.popt.topt) : NULL);
 
 	fprintf(output, _("List of specially treated variables\n\n"));
 
@@ -360,6 +360,8 @@ helpVariables(unsigned short int pager)
 					  "    if set to \"noexec\", just show them without execution\n"));
 	fprintf(output, _("  ENCODING\n"
 					  "    current client character set encoding\n"));
+	fprintf(output, _("  ERROR\n"
+					  "    whether the last query failed\n"));
 	fprintf(output, _("  FETCH_COUNT\n"
 					  "    the number of result rows to fetch and display at a time (0 = unlimited)\n"));
 	fprintf(output, _("  HISTCONTROL\n"
@@ -374,6 +376,9 @@ helpVariables(unsigned short int pager)
 					  "    number of EOFs needed to terminate an interactive session\n"));
 	fprintf(output, _("  LASTOID\n"
 					  "    value of the last affected OID\n"));
+	fprintf(output, _("  LAST_ERROR_SQLSTATE\n"
+					  "  LAST_ERROR_MESSAGE\n"
+					  "    error code and message of last error, or \"00000\" and empty if none\n"));
 	fprintf(output, _("  ON_ERROR_ROLLBACK\n"
 					  "    if set, an error doesn't stop a transaction (uses implicit savepoints)\n"));
 	fprintf(output, _("  ON_ERROR_STOP\n"
@@ -388,6 +393,8 @@ helpVariables(unsigned short int pager)
 					  "    specifies the prompt used during COPY ... FROM STDIN\n"));
 	fprintf(output, _("  QUIET\n"
 					  "    run quietly (same as -q option)\n"));
+	fprintf(output, _("  ROW_COUNT\n"
+					  "    number of rows of last query, or 0\n"));
 	fprintf(output, _("  SERVER_VERSION_NAME\n"
 					  "  SERVER_VERSION_NUM\n"
 					  "    server's version (in short string or numeric format)\n"));
@@ -397,6 +404,8 @@ helpVariables(unsigned short int pager)
 					  "    if set, end of line terminates SQL commands (same as -S option)\n"));
 	fprintf(output, _("  SINGLESTEP\n"
 					  "    single-step mode (same as -s option)\n"));
+	fprintf(output, _("  SQLSTATE\n"
+					  "    error code of last query, or \"00000\" if no error\n"));
 	fprintf(output, _("  USER\n"
 					  "    the currently connected database user\n"));
 	fprintf(output, _("  VERBOSITY\n"
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index 1e48f4a..d020f3f 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -165,6 +165,10 @@ main(int argc, char *argv[])
 	SetVariable(pset.vars, "VERSION_NAME", PG_VERSION);
 	SetVariable(pset.vars, "VERSION_NUM", CppAsString2(PG_VERSION_NUM));
 
+	/* Create variables for last error */
+	SetVariable(pset.vars, "LAST_ERROR_SQLSTATE", "00000");
+	SetVariable(pset.vars, "LAST_ERROR_MESSAGE", "");
+
 	/* Default values for variables (that don't match the result of \unset) */
 	SetVariableBool(pset.vars, "AUTOCOMMIT");
 	SetVariable(pset.vars, "PROMPT1", DEFAULT_PROMPT1);
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index 7957268..b9cdc44 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -28,6 +28,86 @@ on
 \unset ON_ERROR_ROLLBACK
 \echo :ON_ERROR_ROLLBACK
 off
+-- special result variables
+-- these tests are performed early to check that for values after startup
+-- 3 unset variables
+\echo 'error:' :ERROR
+error: :ERROR
+\echo 'error code:' :SQLSTATE
+error code: :SQLSTATE
+\echo 'number of rows:' :ROW_COUNT
+number of rows: :ROW_COUNT
+-- variables with default values
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+last error code: 00000
+\echo 'last error message:' :LAST_ERROR_MESSAGE
+last error message: 
+-- first script query, 2 rows select
+SELECT 1 AS stuff UNION SELECT 2;
+ stuff 
+-------
+     1
+     2
+(2 rows)
+
+\if :ERROR
+  \echo 'MUST NOT SHOW'
+\else
+  \echo 'ERROR is FALSE as expected'
+ERROR is FALSE as expected
+\endif
+\echo 'error code:' :SQLSTATE
+error code: 00000
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+last error code: 00000
+\echo 'last error message:' :LAST_ERROR_MESSAGE
+last error message: 
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 2
+-- syntax error
+SELECT 1 UNION;
+ERROR:  syntax error at or near ";"
+LINE 1: SELECT 1 UNION;
+                      ^
+\if :ERROR
+  \echo 'ERROR is TRUE as expected'
+ERROR is TRUE as expected
+\else
+  \echo 'MUST NOT SHOW'
+\endif
+\echo 'error code:' :SQLSTATE
+error code: 42601
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+last error code: 42601
+\echo 'error message:' :LAST_ERROR_MESSAGE
+error message: syntax error at or near ";"
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
+-- empty query
+;
+\echo 'error:' :ERROR
+error: FALSE
+\echo 'error code:' :SQLSTATE
+error code: 00000
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+last error code: 42601
+\echo 'error message:' :LAST_ERROR_MESSAGE
+error message: syntax error at or near ";"
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
+-- other query error
+DROP TABLE this_table_does_not_exist;
+ERROR:  table "this_table_does_not_exist" does not exist
+\echo 'error:' :ERROR
+error: TRUE
+\echo 'error code:' :SQLSTATE
+error code: 42P01
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+last error code: 42P01
+\echo 'error message:' :LAST_ERROR_MESSAGE
+error message: table "this_table_does_not_exist" does not exist
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
 -- \g and \gx
 SELECT 1 as one, 2 as two \g
  one | two 
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index 0556b7c..3a55d71 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -21,6 +21,58 @@
 \unset ON_ERROR_ROLLBACK
 \echo :ON_ERROR_ROLLBACK
 
+-- special result variables
+-- these tests are performed early to check that for values after startup
+
+-- 3 unset variables
+\echo 'error:' :ERROR
+\echo 'error code:' :SQLSTATE
+\echo 'number of rows:' :ROW_COUNT
+
+-- variables with default values
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+\echo 'last error message:' :LAST_ERROR_MESSAGE
+
+-- first script query, 2 rows select
+SELECT 1 AS stuff UNION SELECT 2;
+\if :ERROR
+  \echo 'MUST NOT SHOW'
+\else
+  \echo 'ERROR is FALSE as expected'
+\endif
+\echo 'error code:' :SQLSTATE
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+\echo 'last error message:' :LAST_ERROR_MESSAGE
+\echo 'number of rows:' :ROW_COUNT
+
+-- syntax error
+SELECT 1 UNION;
+\if :ERROR
+  \echo 'ERROR is TRUE as expected'
+\else
+  \echo 'MUST NOT SHOW'
+\endif
+\echo 'error code:' :SQLSTATE
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+\echo 'error message:' :LAST_ERROR_MESSAGE
+\echo 'number of rows:' :ROW_COUNT
+
+-- empty query
+;
+\echo 'error:' :ERROR
+\echo 'error code:' :SQLSTATE
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+\echo 'error message:' :LAST_ERROR_MESSAGE
+\echo 'number of rows:' :ROW_COUNT
+
+-- other query error
+DROP TABLE this_table_does_not_exist;
+\echo 'error:' :ERROR
+\echo 'error code:' :SQLSTATE
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+\echo 'error message:' :LAST_ERROR_MESSAGE
+\echo 'number of rows:' :ROW_COUNT
+
 -- \g and \gx
 
 SELECT 1 as one, 2 as two \g
#24Pavel Stehule
pavel.stehule@gmail.com
In reply to: Fabien COELHO (#23)
Re: psql - add special variable to reflect the last query status

Hi

2017-09-06 11:14 GMT+02:00 Fabien COELHO <coelho@cri.ensmp.fr>:

Here is a version 6.

Small v7 update, sorry for the noise.

Add testing the initial state of all variables.

Fix typos in a comment in tests.

Fix the documentation wrt the current implementation behavior.

I rechecked last patch

There is not any issue with patching. All regress tests passed

I checked performance - the most fast queries are execution of simple
prepared statement

prepare x as select 1;
-- 1000000 x
execute x;
execute x;
execute x;
execute x;

## patched
[pavel@nemesis postgresql]$ time psql -At -1 postgres -f ~/xxx.sql >
/dev/null

real 0m44,887s
user 0m11,703s
sys 0m6,942s

This is probably the most worst case, what is possible and see some
slowdown - in this case there is about 10% slowdown -

but it is pretty untypical - the one query was less than 50 microsec. When
there will be any IO activity or network usage, than this patch doesn't
create any significant overhead.

I'll mark this patch as ready for commiter

Regards

Pavel

Show quoted text

--
Fabien.

#25Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Pavel Stehule (#24)
Re: psql - add special variable to reflect the last query status

Hello Pavel,

I checked performance - the most fast queries are execution of simple
prepared statement

prepare x as select 1;
-- 1000000 x
execute x;
execute x;
execute x;
execute x;

## patched
[pavel@nemesis postgresql]$ time psql -At -1 postgres -f ~/xxx.sql >
/dev/null

real 0m44,887s
user 0m11,703s
sys 0m6,942s

This is probably the most worst case, what is possible and see some
slowdown - in this case there is about 10% slowdown -

but it is pretty untypical - the one query was less than 50 microsec. When
there will be any IO activity or network usage, than this patch doesn't
create any significant overhead.

Interesting. Thanks for the test.

I tried to replicate with a variant without any output: "SELECT;"

SELECT NOW() AS start \gset
BEGIN;
SELECT; -- 2^19 times
END;
SELECT NOW() - :'start';

The run time is about 19 µs per SELECT on my laptop. Over 33 runs each
alternating master with and without the patch, I got the following stats
on the measured time in seconds (average, stddev [min Q1 median Q3 max]):

- with : 9.898 ± 0.158 [9.564, 9.762, 9.936, 10.037, 10.108]
- without: 9.419 ± 0.294 [8.670, 9.226, 9.533, 9.625, 9.845]

This seems consistent and significant. It suggests a 0.40-0.50 s
difference, that is about 5%, i.e. about (under) 1 µs overhead per
statement in pretty defavorable circumstances.

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

#26Tom Lane
tgl@sss.pgh.pa.us
In reply to: Fabien COELHO (#23)
Re: psql - add special variable to reflect the last query status

Fabien COELHO <coelho@cri.ensmp.fr> writes:

Small v7 update, sorry for the noise.

Hm. Looking closer at this, I see that it doesn't work so well after all
to put the variable-setting code in ProcessResult: that fails to cover the
ExecQueryUsingCursor code path. And it also fails to cover DescribeQuery,
which arguably should set these variables as well -- certainly so if it
gets a failure. Maybe you could create a small subroutine along the lines
of SetResultVariables(PGresult *result, bool success) for all three places
to call. (ProcessResult certainly has already decided whether it's got a
success, and I think the other paths would know that as well, so no need
to re-extract it from the PGresult.)

I think you're overly optimistic to believe that every failure will
have a SQLSTATE; I don't think that's true for libpq-reported errors,
such as connection loss.

Using upper-case TRUE/FALSE for the values of ERROR seems a bit
ugly to me; we generally use lower case for other variable values,
so I'd go with true/false.

regards, tom lane

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

#27Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Tom Lane (#26)
Re: psql - add special variable to reflect the last query status

Hello Tom,

Hm. Looking closer at this, I see that it doesn't work so well after all
to put the variable-setting code in ProcessResult:
that fails to cover the ExecQueryUsingCursor code path.

Ok, I'll investigate this path.

And it also fails to cover DescribeQuery, which arguably should set
these variables as well

And this one.

-- certainly so if it gets a failure. Maybe you
could create a small subroutine along the lines of
SetResultVariables(PGresult *result, bool success) for all three places
to call. (ProcessResult certainly has already decided whether it's got
a success, and I think the other paths would know that as well, so no
need to re-extract it from the PGresult.)

Ok.

I think you're overly optimistic to believe that every failure will
have a SQLSTATE; I don't think that's true for libpq-reported errors,
such as connection loss.

Yep, I thought I was optimistic:-) Can I add a special SQLSTATE for that
situation where libpq did not report an error?

Using upper-case TRUE/FALSE for the values of ERROR seems a bit
ugly to me; we generally use lower case for other variable values,
so I'd go with true/false.

Ok. The choice is not aesthetic but systematic: I use upper-case for all
SQL keywords, and lower-case or capitalized for anything user land. I can
put lower-case if you want.

--
Fabien.

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

#28Tom Lane
tgl@sss.pgh.pa.us
In reply to: Fabien COELHO (#27)
Re: psql - add special variable to reflect the last query status

Fabien COELHO <coelho@cri.ensmp.fr> writes:

I think you're overly optimistic to believe that every failure will
have a SQLSTATE; I don't think that's true for libpq-reported errors,
such as connection loss.

Yep, I thought I was optimistic:-) Can I add a special SQLSTATE for that
situation where libpq did not report an error?

Meh. If we're going to do that I think it might be better to hack
libpq itself to do so, ie, force PQresultErrorField(..., PG_DIAG_SQLSTATE)
to always return something. But it seems like a hack either way.

regards, tom lane

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

#29Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Tom Lane (#28)
Re: psql - add special variable to reflect the last query status

I think you're overly optimistic to believe that every failure will
have a SQLSTATE; I don't think that's true for libpq-reported errors,
such as connection loss.

Yep, I thought I was optimistic:-) Can I add a special SQLSTATE for that
situation where libpq did not report an error?

Meh. If we're going to do that I think it might be better to hack
libpq itself to do so, ie, force PQresultErrorField(..., PG_DIAG_SQLSTATE)
to always return something. But it seems like a hack either way.

I would not have took the liberty to hack into libpq internals for such a
small front-end feature. However I agree that having libpq always return
some diagnostic, even if it means "something unclear happened, sorry not
to be very precise", would be better.

--
Fabien.

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

#30Pavel Stehule
pavel.stehule@gmail.com
In reply to: Fabien COELHO (#29)
Re: psql - add special variable to reflect the last query status

2017-09-11 20:46 GMT+02:00 Fabien COELHO <coelho@cri.ensmp.fr>:

I think you're overly optimistic to believe that every failure will

have a SQLSTATE; I don't think that's true for libpq-reported errors,
such as connection loss.

Yep, I thought I was optimistic:-) Can I add a special SQLSTATE for that

situation where libpq did not report an error?

Meh. If we're going to do that I think it might be better to hack
libpq itself to do so, ie, force PQresultErrorField(..., PG_DIAG_SQLSTATE)
to always return something. But it seems like a hack either way.

I would not have took the liberty to hack into libpq internals for such a
small front-end feature. However I agree that having libpq always return
some diagnostic, even if it means "something unclear happened, sorry not to
be very precise", would be better.

probably better don't do it before somebody implements this are correctly
.. some temporary solution can introduce possible compatibility issues.

If SQLSTATE has not know value, then it should be NULL or maybe empty
string.

Pavel

Show quoted text

--
Fabien.

#31Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Fabien COELHO (#29)
1 attachment(s)
Re: psql - add special variable to reflect the last query status

Hello Tom,

Yep, I thought I was optimistic:-) Can I add a special SQLSTATE for that
situation where libpq did not report an error?

Meh. If we're going to do that I think it might be better to hack
libpq itself to do so, ie, force PQresultErrorField(..., PG_DIAG_SQLSTATE)
to always return something. But it seems like a hack either way.

I would not have took the liberty to hack into libpq internals for such a
small front-end feature. However I agree that having libpq always return some
diagnostic, even if it means "something unclear happened, sorry not to be
very precise", would be better.

Here is an attempt at implementing your suggestions.

I added two error codes, which is debatable. One is used hardcoded by
libpq if no diagnostic is found, and the other by psql if libpq returned
something empty, which might happen if psql is linked with an older libpq,
maybe. I do not know how to trigger such errors anyway, so this is rather
academic.

I put back SetResultVariables function which is called twice, for SQL
queries and the new descriptions. It worked out of the box with DECLARE
which is just another SQL statement, so maybe I did not understood the
cursor issue you were signaling...

--
Fabien.

Attachments:

psql-result-status-8.patchtext/x-diff; name=psql-result-status-8.patchDownload
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index a74caf8..b994fcd 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -3518,6 +3518,16 @@ bar
       </varlistentry>
 
       <varlistentry>
+       <term><varname>ERROR</varname></term>
+       <listitem>
+        <para>
+         Whether the last query failed, as a boolean.
+         See also <varname>SQLSTATE</>.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
         <term><varname>FETCH_COUNT</varname></term>
         <listitem>
         <para>
@@ -3654,6 +3664,18 @@ bar
       </varlistentry>
 
       <varlistentry>
+       <term><varname>LAST_ERROR_SQLSTATE</varname></term>
+       <term><varname>LAST_ERROR_MESSAGE</varname></term>
+       <listitem>
+        <para>
+         The error code and associated error message of the last
+         error, or "00000" and empty strings if no error occured
+         since the beginning of the script.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
       <term>
        <varname>ON_ERROR_ROLLBACK</varname>
        <indexterm>
@@ -3722,6 +3744,25 @@ bar
       </varlistentry>
 
       <varlistentry>
+       <term><varname>ROW_COUNT</varname></term>
+       <listitem>
+        <para>
+         How many rows were returned or affected by the last query.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
+       <term><varname>SQLSTATE</varname></term>
+       <listitem>
+        <para>
+         The error code associated to the last query, or
+         <literal>00000</> if no error occured.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
         <term><varname>QUIET</varname></term>
         <listitem>
         <para>
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index b997058..bbffcac 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -493,7 +493,6 @@ ResetCancelConn(void)
 #endif
 }
 
-
 /*
  * AcceptResult
  *
@@ -971,6 +970,44 @@ loop_exit:
 	return success;
 }
 
+/*
+ * Set special variables
+ * - ERROR: true/false, whether an error occurred
+ * - SQLSTATE: code of error, or "00000"
+ * - LAST_ERROR_SQLSTATE: same for last error
+ * - LAST_ERROR_MESSAGE: message of last error
+ * - ROW_COUNT: how many rows were returned or affected, or "0"
+ */
+static void
+SetResultVariables(PGresult *results, bool success)
+{
+	if (success)
+	{
+		char   *ntuples = PQcmdTuples(results);
+		SetVariable(pset.vars, "ERROR", "false");
+		SetVariable(pset.vars, "SQLSTATE", "00000");
+		SetVariable(pset.vars, "ROW_COUNT", *ntuples ? ntuples : "0");
+	}
+	else
+	{
+		char 		   *code = PQresultErrorField(results, PG_DIAG_SQLSTATE);
+		char 		   *mesg = PQresultErrorField(results, PG_DIAG_MESSAGE_PRIMARY);
+
+		SetVariable(pset.vars, "ERROR", "true");
+
+		/*
+		 * Ensure that something sensible is shown,
+		 * without assumption about libpq implementation
+		 */
+		if (code == NULL || *code == '\0')
+			code = "PQ001" /* ERROR_LIBPQ_EMPTY_SQLSTATE */ ;
+
+		SetVariable(pset.vars, "SQLSTATE", code);
+		SetVariable(pset.vars, "LAST_ERROR_SQLSTATE", code);
+		SetVariable(pset.vars, "LAST_ERROR_MESSAGE", mesg ? mesg : "");
+		SetVariable(pset.vars, "ROW_COUNT", "0");
+	}
+}
 
 /*
  * ProcessResult: utility function for use by SendQuery() only
@@ -1107,6 +1144,8 @@ ProcessResult(PGresult **results)
 		first_cycle = false;
 	}
 
+	SetResultVariables(*results, success);
+
 	/* may need this to recover from conn loss during COPY */
 	if (!first_cycle && !CheckConnection())
 		return false;
@@ -1214,7 +1253,6 @@ PrintQueryResults(PGresult *results)
 	return success;
 }
 
-
 /*
  * SendQuery: send the query string to the backend
  * (and print out results)
@@ -1523,7 +1561,11 @@ DescribeQuery(const char *query, double *elapsed_msec)
 	 * good thing because libpq provides no easy way to do that.)
 	 */
 	results = PQprepare(pset.db, "", query, 0, NULL);
-	if (PQresultStatus(results) != PGRES_COMMAND_OK)
+	OK = PQresultStatus(results) == PGRES_COMMAND_OK;
+
+	SetResultVariables(results, OK);
+
+	if (!OK)
 	{
 		psql_error("%s", PQerrorMessage(pset.db));
 		ClearOrSaveResult(results);
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 4d1c0ec..ae951f5 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -337,7 +337,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(147, pager ? &(pset.popt.topt) : NULL);
+	output = PageOutput(155, pager ? &(pset.popt.topt) : NULL);
 
 	fprintf(output, _("List of specially treated variables\n\n"));
 
@@ -360,6 +360,8 @@ helpVariables(unsigned short int pager)
 					  "    if set to \"noexec\", just show them without execution\n"));
 	fprintf(output, _("  ENCODING\n"
 					  "    current client character set encoding\n"));
+	fprintf(output, _("  ERROR\n"
+					  "    whether the last query failed\n"));
 	fprintf(output, _("  FETCH_COUNT\n"
 					  "    the number of result rows to fetch and display at a time (0 = unlimited)\n"));
 	fprintf(output, _("  HISTCONTROL\n"
@@ -374,6 +376,9 @@ helpVariables(unsigned short int pager)
 					  "    number of EOFs needed to terminate an interactive session\n"));
 	fprintf(output, _("  LASTOID\n"
 					  "    value of the last affected OID\n"));
+	fprintf(output, _("  LAST_ERROR_SQLSTATE\n"
+					  "  LAST_ERROR_MESSAGE\n"
+					  "    error code and message of last error, or \"00000\" and empty if none\n"));
 	fprintf(output, _("  ON_ERROR_ROLLBACK\n"
 					  "    if set, an error doesn't stop a transaction (uses implicit savepoints)\n"));
 	fprintf(output, _("  ON_ERROR_STOP\n"
@@ -388,6 +393,8 @@ helpVariables(unsigned short int pager)
 					  "    specifies the prompt used during COPY ... FROM STDIN\n"));
 	fprintf(output, _("  QUIET\n"
 					  "    run quietly (same as -q option)\n"));
+	fprintf(output, _("  ROW_COUNT\n"
+					  "    number of rows of last query, or 0\n"));
 	fprintf(output, _("  SERVER_VERSION_NAME\n"
 					  "  SERVER_VERSION_NUM\n"
 					  "    server's version (in short string or numeric format)\n"));
@@ -397,6 +404,8 @@ helpVariables(unsigned short int pager)
 					  "    if set, end of line terminates SQL commands (same as -S option)\n"));
 	fprintf(output, _("  SINGLESTEP\n"
 					  "    single-step mode (same as -s option)\n"));
+	fprintf(output, _("  SQLSTATE\n"
+					  "    error code of last query, or \"00000\" if no error\n"));
 	fprintf(output, _("  USER\n"
 					  "    the currently connected database user\n"));
 	fprintf(output, _("  VERBOSITY\n"
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index 1e48f4a..d020f3f 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -165,6 +165,10 @@ main(int argc, char *argv[])
 	SetVariable(pset.vars, "VERSION_NAME", PG_VERSION);
 	SetVariable(pset.vars, "VERSION_NUM", CppAsString2(PG_VERSION_NUM));
 
+	/* Create variables for last error */
+	SetVariable(pset.vars, "LAST_ERROR_SQLSTATE", "00000");
+	SetVariable(pset.vars, "LAST_ERROR_MESSAGE", "");
+
 	/* Default values for variables (that don't match the result of \unset) */
 	SetVariableBool(pset.vars, "AUTOCOMMIT");
 	SetVariable(pset.vars, "PROMPT1", DEFAULT_PROMPT1);
diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index c24bce6..678aa02 100644
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -2717,6 +2717,9 @@ PQresultErrorField(const PGresult *res, int fieldcode)
 		if (pfield->code == fieldcode)
 			return pfield->contents;
 	}
+	/* special case if sqlstate was found */
+	if (fieldcode == PG_DIAG_SQLSTATE)
+		return "PQ000" /* aka ERROR_LIBPQ_MISSING_SQLSTATE */;
 	return NULL;
 }
 
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index bda8960..51a98bf 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -28,6 +28,197 @@ on
 \unset ON_ERROR_ROLLBACK
 \echo :ON_ERROR_ROLLBACK
 off
+-- special result variables
+-- these tests are performed early to check that for values after startup
+-- 3 initially unset variables
+\echo 'error:' :ERROR
+error: :ERROR
+\echo 'error code:' :SQLSTATE
+error code: :SQLSTATE
+\echo 'number of rows:' :ROW_COUNT
+number of rows: :ROW_COUNT
+-- variables with default values
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+last error code: 00000
+\echo 'last error message:' :LAST_ERROR_MESSAGE
+last error message: 
+-- first working query, 2 rows select
+SELECT 1 AS stuff UNION SELECT 2;
+ stuff 
+-------
+     1
+     2
+(2 rows)
+
+\if :ERROR
+  \echo 'MUST NOT SHOW'
+\else
+  \echo 'ERROR is FALSE as expected'
+ERROR is FALSE as expected
+\endif
+\echo 'error code:' :SQLSTATE
+error code: 00000
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 2
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+last error code: 00000
+\echo 'last error message:' :LAST_ERROR_MESSAGE
+last error message: 
+-- syntax error
+SELECT 1 UNION;
+ERROR:  syntax error at or near ";"
+LINE 1: SELECT 1 UNION;
+                      ^
+\if :ERROR
+  \echo 'ERROR is TRUE as expected'
+ERROR is TRUE as expected
+\else
+  \echo 'MUST NOT SHOW'
+\endif
+\echo 'error code:' :SQLSTATE
+error code: 42601
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+last error code: 42601
+\echo 'error message:' :LAST_ERROR_MESSAGE
+error message: syntax error at or near ";"
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
+-- empty query
+;
+\echo 'error:' :ERROR
+error: false
+\echo 'error code:' :SQLSTATE
+error code: 00000
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
+-- must have kept previous values
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+last error code: 42601
+\echo 'error message:' :LAST_ERROR_MESSAGE
+error message: syntax error at or near ";"
+-- other query error
+DROP TABLE this_table_does_not_exist;
+ERROR:  table "this_table_does_not_exist" does not exist
+\echo 'error:' :ERROR
+error: true
+\echo 'error code:' :SQLSTATE
+error code: 42P01
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
+-- new values
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+last error code: 42P01
+\echo 'error message:' :LAST_ERROR_MESSAGE
+error message: table "this_table_does_not_exist" does not exist
+-- cleanup
+\unset LAST_ERROR_SQLSTATE
+\unset LAST_ERROR_MESSAGE
+-- working CURSOR
+DECLARE one CURSOR WITH HOLD FOR SELECT 1 AS one;
+\echo 'error:' :ERROR
+error: false
+\echo 'error code:' :SQLSTATE
+error code: 00000
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
+FETCH 2 FROM one;
+ one 
+-----
+   1
+(1 row)
+
+\echo 'error:' :ERROR
+error: false
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 1
+FETCH NEXT FROM one;
+ one 
+-----
+(0 rows)
+
+\echo 'error:' :ERROR
+error: false
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
+CLOSE one;
+\echo 'error:' :ERROR
+error: false
+-- CURSOR with syntax error
+DECLARE two CURSOR WITH HOLD FOR SELECT 1 + 1 +;
+ERROR:  syntax error at or near ";"
+LINE 1: DECLARE two CURSOR WITH HOLD FOR SELECT 1 + 1 +;
+                                                       ^
+\echo 'error:' :ERROR
+error: true
+\echo 'error code:' :SQLSTATE
+error code: 42601
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+last error code: 42601
+\echo 'error message:' :LAST_ERROR_MESSAGE
+error message: syntax error at or near ";"
+-- cleanup
+\unset LAST_ERROR_SQLSTATE
+\unset LAST_ERROR_MESSAGE
+-- CURSOR with execution error
+BEGIN;
+CREATE FUNCTION raise_an_error()
+RETURNS INTEGER
+IMMUTABLE STRICT AS $$
+BEGIN
+  RAISE EXCEPTION 'function raise_an_error()';
+  RETURN 1;
+END;
+$$ LANGUAGE plpgsql;
+DECLARE cursor_error CURSOR FOR SELECT raise_an_error();
+ERROR:  function raise_an_error()
+CONTEXT:  PL/pgSQL function raise_an_error() line 3 at RAISE
+\echo 'error:' :ERROR
+error: true
+\echo 'error code:' :SQLSTATE
+error code: P0001
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+last error code: P0001
+\echo 'error message:' :LAST_ERROR_MESSAGE
+error message: function raise_an_error()
+ROLLBACK;
+-- working description
+SELECT 3 AS three \gdesc
+ Column |  Type   
+--------+---------
+ three  | integer
+(1 row)
+
+\echo 'error:' :ERROR
+error: false
+\echo 'error code:' :SQLSTATE
+error code: 00000
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
+-- describe with an error
+SELECT 4 AS \gdesc
+ERROR:  syntax error at end of input
+LINE 1: SELECT 4 AS 
+                    ^
+\echo 'error:' :ERROR
+error: true
+\echo 'error code:' :SQLSTATE
+error code: 42601
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+last error code: 42601
+\echo 'error message:' :LAST_ERROR_MESSAGE
+error message: syntax error at end of input
+-- cleanup all
+\unset ERROR
+\unset SQLSTATE
+\unset ROW_COUNT
+\unset LAST_ERROR_SQLSTATE
+\unset LAST_ERROR_MESSAGE
 -- \g and \gx
 SELECT 1 as one, 2 as two \g
  one | two 
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index 0556b7c..c3a8844 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -21,6 +21,130 @@
 \unset ON_ERROR_ROLLBACK
 \echo :ON_ERROR_ROLLBACK
 
+-- special result variables
+-- these tests are performed early to check that for values after startup
+
+-- 3 initially unset variables
+\echo 'error:' :ERROR
+\echo 'error code:' :SQLSTATE
+\echo 'number of rows:' :ROW_COUNT
+
+-- variables with default values
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+\echo 'last error message:' :LAST_ERROR_MESSAGE
+
+-- first working query, 2 rows select
+SELECT 1 AS stuff UNION SELECT 2;
+\if :ERROR
+  \echo 'MUST NOT SHOW'
+\else
+  \echo 'ERROR is FALSE as expected'
+\endif
+\echo 'error code:' :SQLSTATE
+\echo 'number of rows:' :ROW_COUNT
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+\echo 'last error message:' :LAST_ERROR_MESSAGE
+
+-- syntax error
+SELECT 1 UNION;
+\if :ERROR
+  \echo 'ERROR is TRUE as expected'
+\else
+  \echo 'MUST NOT SHOW'
+\endif
+\echo 'error code:' :SQLSTATE
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+\echo 'error message:' :LAST_ERROR_MESSAGE
+\echo 'number of rows:' :ROW_COUNT
+
+-- empty query
+;
+\echo 'error:' :ERROR
+\echo 'error code:' :SQLSTATE
+\echo 'number of rows:' :ROW_COUNT
+-- must have kept previous values
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+\echo 'error message:' :LAST_ERROR_MESSAGE
+
+-- other query error
+DROP TABLE this_table_does_not_exist;
+\echo 'error:' :ERROR
+\echo 'error code:' :SQLSTATE
+\echo 'number of rows:' :ROW_COUNT
+-- new values
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+\echo 'error message:' :LAST_ERROR_MESSAGE
+-- cleanup
+\unset LAST_ERROR_SQLSTATE
+\unset LAST_ERROR_MESSAGE
+
+-- working CURSOR
+DECLARE one CURSOR WITH HOLD FOR SELECT 1 AS one;
+\echo 'error:' :ERROR
+\echo 'error code:' :SQLSTATE
+\echo 'number of rows:' :ROW_COUNT
+FETCH 2 FROM one;
+\echo 'error:' :ERROR
+\echo 'number of rows:' :ROW_COUNT
+FETCH NEXT FROM one;
+\echo 'error:' :ERROR
+\echo 'number of rows:' :ROW_COUNT
+CLOSE one;
+\echo 'error:' :ERROR
+
+-- CURSOR with syntax error
+DECLARE two CURSOR WITH HOLD FOR SELECT 1 + 1 +;
+\echo 'error:' :ERROR
+\echo 'error code:' :SQLSTATE
+\echo 'number of rows:' :ROW_COUNT
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+\echo 'error message:' :LAST_ERROR_MESSAGE
+-- cleanup
+\unset LAST_ERROR_SQLSTATE
+\unset LAST_ERROR_MESSAGE
+
+-- CURSOR with execution error
+BEGIN;
+
+CREATE FUNCTION raise_an_error()
+RETURNS INTEGER
+IMMUTABLE STRICT AS $$
+BEGIN
+  RAISE EXCEPTION 'function raise_an_error()';
+  RETURN 1;
+END;
+$$ LANGUAGE plpgsql;
+
+DECLARE cursor_error CURSOR FOR SELECT raise_an_error();
+\echo 'error:' :ERROR
+\echo 'error code:' :SQLSTATE
+\echo 'number of rows:' :ROW_COUNT
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+\echo 'error message:' :LAST_ERROR_MESSAGE
+
+ROLLBACK;
+
+-- working description
+SELECT 3 AS three \gdesc
+\echo 'error:' :ERROR
+\echo 'error code:' :SQLSTATE
+\echo 'number of rows:' :ROW_COUNT
+
+-- describe with an error
+SELECT 4 AS \gdesc
+\echo 'error:' :ERROR
+\echo 'error code:' :SQLSTATE
+\echo 'number of rows:' :ROW_COUNT
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+\echo 'error message:' :LAST_ERROR_MESSAGE
+
+-- cleanup all
+\unset ERROR
+\unset SQLSTATE
+\unset ROW_COUNT
+\unset LAST_ERROR_SQLSTATE
+\unset LAST_ERROR_MESSAGE
+
 -- \g and \gx
 
 SELECT 1 as one, 2 as two \g
#32Robert Haas
robertmhaas@gmail.com
In reply to: Fabien COELHO (#31)
Re: psql - add special variable to reflect the last query status

On Tue, Sep 12, 2017 at 1:23 PM, Fabien COELHO <coelho@cri.ensmp.fr> wrote:

I added two error codes, which is debatable. One is used hardcoded by libpq
if no diagnostic is found, and the other by psql if libpq returned something
empty, which might happen if psql is linked with an older libpq, maybe. I do
not know how to trigger such errors anyway, so this is rather academic.

I think this is a bad plan. Right now, libpq sets no SQLSTATE for
internally generated errors; it is almost certain that there are
applications testing for an empty SQLSTATE to notice when they're
getting an error from libpq. EnterpriseDB had a support ticket quite
recently where this precise behavior was at issue. Changing it will
break stuff, so we shouldn't do it unless there's a really compelling
benefit. Universally returning PQ000 is not a sufficient improvement
over universally returning the empty string to justify the risk of
application breakage.

--
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

#33Pavel Stehule
pavel.stehule@gmail.com
In reply to: Robert Haas (#32)
Re: psql - add special variable to reflect the last query status

2017-09-12 20:43 GMT+02:00 Robert Haas <robertmhaas@gmail.com>:

On Tue, Sep 12, 2017 at 1:23 PM, Fabien COELHO <coelho@cri.ensmp.fr>
wrote:

I added two error codes, which is debatable. One is used hardcoded by

libpq

if no diagnostic is found, and the other by psql if libpq returned

something

empty, which might happen if psql is linked with an older libpq, maybe.

I do

not know how to trigger such errors anyway, so this is rather academic.

I think this is a bad plan. Right now, libpq sets no SQLSTATE for
internally generated errors; it is almost certain that there are
applications testing for an empty SQLSTATE to notice when they're
getting an error from libpq. EnterpriseDB had a support ticket quite
recently where this precise behavior was at issue. Changing it will
break stuff, so we shouldn't do it unless there's a really compelling
benefit. Universally returning PQ000 is not a sufficient improvement
over universally returning the empty string to justify the risk of
application breakage.

+1

Pavel

Show quoted text

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

#34Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#32)
Re: psql - add special variable to reflect the last query status

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Sep 12, 2017 at 1:23 PM, Fabien COELHO <coelho@cri.ensmp.fr> wrote:

I added two error codes, which is debatable. One is used hardcoded by libpq
if no diagnostic is found, and the other by psql if libpq returned something
empty, which might happen if psql is linked with an older libpq, maybe. I do
not know how to trigger such errors anyway, so this is rather academic.

I think this is a bad plan. Right now, libpq sets no SQLSTATE for
internally generated errors; it is almost certain that there are
applications testing for an empty SQLSTATE to notice when they're
getting an error from libpq. EnterpriseDB had a support ticket quite
recently where this precise behavior was at issue. Changing it will
break stuff, so we shouldn't do it unless there's a really compelling
benefit. Universally returning PQ000 is not a sufficient improvement
over universally returning the empty string to justify the risk of
application breakage.

I don't think I want to buy this argument, because the logical conclusion
of it is that we can never fix libpq to offer proper SQLSTATEs for
client-side errors. Admittedly, the fact that nobody's bothered to do so
in ~15 years may indicate that nobody cares ... but I would think that
at least it'd be useful to distinguish, say, ENOMEM from connection loss.
Saying we can't do it for compatibility reasons doesn't sound great
to me. Especially when you've not provided any hard evidence as to why
the current lack-of-information is useful.

regards, tom lane

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

#35Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#34)
Re: psql - add special variable to reflect the last query status

On Tue, Sep 12, 2017 at 3:12 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I think this is a bad plan. Right now, libpq sets no SQLSTATE for
internally generated errors; it is almost certain that there are
applications testing for an empty SQLSTATE to notice when they're
getting an error from libpq. EnterpriseDB had a support ticket quite
recently where this precise behavior was at issue. Changing it will
break stuff, so we shouldn't do it unless there's a really compelling
benefit. Universally returning PQ000 is not a sufficient improvement
over universally returning the empty string to justify the risk of
application breakage.

I don't think I want to buy this argument, because the logical conclusion
of it is that we can never fix libpq to offer proper SQLSTATEs for
client-side errors. Admittedly, the fact that nobody's bothered to do so
in ~15 years may indicate that nobody cares ... but I would think that
at least it'd be useful to distinguish, say, ENOMEM from connection loss.
Saying we can't do it for compatibility reasons doesn't sound great
to me. Especially when you've not provided any hard evidence as to why
the current lack-of-information is useful.

Well, if we provided a different SQLSTATE for each qualitatively
different type of libpq error, that might well be useful enough to
justify some risk of application breakage. But replacing a constant
string that we've had for ~15 years with a different constraint string
isn't doing anything about the lack-of-information problem you're
complaining about.

--
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

#36Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#35)
Re: psql - add special variable to reflect the last query status

Robert Haas <robertmhaas@gmail.com> writes:

Well, if we provided a different SQLSTATE for each qualitatively
different type of libpq error, that might well be useful enough to
justify some risk of application breakage. But replacing a constant
string that we've had for ~15 years with a different constraint string
isn't doing anything about the lack-of-information problem you're
complaining about.

True. Well, the original point here was whether psql ought to be doing
something to mask libpq's (mis) behavior. I'm inclined to think not:
if it doesn't get a SQLSTATE from the PGresult, it should just set the
sqlstate variables to empty strings.

regards, tom lane

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

#37Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Tom Lane (#36)
1 attachment(s)
Re: psql - add special variable to reflect the last query status

Well, if we provided a different SQLSTATE for each qualitatively
different type of libpq error, that might well be useful enough to
justify some risk of application breakage. But replacing a constant
string that we've had for ~15 years with a different constraint string
isn't doing anything about the lack-of-information problem you're
complaining about.

True. Well, the original point here was whether psql ought to be doing
something to mask libpq's (mis) behavior. I'm inclined to think not:
if it doesn't get a SQLSTATE from the PGresult, it should just set the
sqlstate variables to empty strings.

See v9 attached.

--
Fabien.

Attachments:

psql-result-status-9.patchtext/x-diff; name=psql-result-status-9.patchDownload
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index a74caf8..b994fcd 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -3518,6 +3518,16 @@ bar
       </varlistentry>
 
       <varlistentry>
+       <term><varname>ERROR</varname></term>
+       <listitem>
+        <para>
+         Whether the last query failed, as a boolean.
+         See also <varname>SQLSTATE</>.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
         <term><varname>FETCH_COUNT</varname></term>
         <listitem>
         <para>
@@ -3654,6 +3664,18 @@ bar
       </varlistentry>
 
       <varlistentry>
+       <term><varname>LAST_ERROR_SQLSTATE</varname></term>
+       <term><varname>LAST_ERROR_MESSAGE</varname></term>
+       <listitem>
+        <para>
+         The error code and associated error message of the last
+         error, or "00000" and empty strings if no error occured
+         since the beginning of the script.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
       <term>
        <varname>ON_ERROR_ROLLBACK</varname>
        <indexterm>
@@ -3722,6 +3744,25 @@ bar
       </varlistentry>
 
       <varlistentry>
+       <term><varname>ROW_COUNT</varname></term>
+       <listitem>
+        <para>
+         How many rows were returned or affected by the last query.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
+       <term><varname>SQLSTATE</varname></term>
+       <listitem>
+        <para>
+         The error code associated to the last query, or
+         <literal>00000</> if no error occured.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
         <term><varname>QUIET</varname></term>
         <listitem>
         <para>
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index b997058..cc7e3aa 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -493,7 +493,6 @@ ResetCancelConn(void)
 #endif
 }
 
-
 /*
  * AcceptResult
  *
@@ -971,6 +970,45 @@ loop_exit:
 	return success;
 }
 
+/*
+ * Set special variables
+ * - ERROR: true/false, whether an error occurred
+ * - SQLSTATE: code of error, or "00000", or ""
+ * - LAST_ERROR_SQLSTATE: same for last error
+ * - LAST_ERROR_MESSAGE: message of last error
+ * - ROW_COUNT: how many rows were returned or affected, or "0"
+ */
+static void
+SetResultVariables(PGresult *results, bool success)
+{
+	if (success)
+	{
+		char   *ntuples = PQcmdTuples(results);
+		SetVariable(pset.vars, "ERROR", "false");
+		SetVariable(pset.vars, "SQLSTATE", "00000");
+		SetVariable(pset.vars, "ROW_COUNT", *ntuples ? ntuples : "0");
+	}
+	else
+	{
+		char 		   *code = PQresultErrorField(results, PG_DIAG_SQLSTATE);
+		char 		   *mesg = PQresultErrorField(results, PG_DIAG_MESSAGE_PRIMARY);
+
+		SetVariable(pset.vars, "ERROR", "true");
+
+		/*
+		 * if there is no code, use an empty string?
+		 * libpq may return such thing on internal errors
+		 * (lost connection, EOM).
+		 */
+		if (code == NULL)
+			code = "" ;
+
+		SetVariable(pset.vars, "SQLSTATE", code);
+		SetVariable(pset.vars, "LAST_ERROR_SQLSTATE", code);
+		SetVariable(pset.vars, "LAST_ERROR_MESSAGE", mesg ? mesg : "");
+		SetVariable(pset.vars, "ROW_COUNT", "0");
+	}
+}
 
 /*
  * ProcessResult: utility function for use by SendQuery() only
@@ -1107,6 +1145,8 @@ ProcessResult(PGresult **results)
 		first_cycle = false;
 	}
 
+	SetResultVariables(*results, success);
+
 	/* may need this to recover from conn loss during COPY */
 	if (!first_cycle && !CheckConnection())
 		return false;
@@ -1214,7 +1254,6 @@ PrintQueryResults(PGresult *results)
 	return success;
 }
 
-
 /*
  * SendQuery: send the query string to the backend
  * (and print out results)
@@ -1523,7 +1562,11 @@ DescribeQuery(const char *query, double *elapsed_msec)
 	 * good thing because libpq provides no easy way to do that.)
 	 */
 	results = PQprepare(pset.db, "", query, 0, NULL);
-	if (PQresultStatus(results) != PGRES_COMMAND_OK)
+	OK = PQresultStatus(results) == PGRES_COMMAND_OK;
+
+	SetResultVariables(results, OK);
+
+	if (!OK)
 	{
 		psql_error("%s", PQerrorMessage(pset.db));
 		ClearOrSaveResult(results);
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 4d1c0ec..ae951f5 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -337,7 +337,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(147, pager ? &(pset.popt.topt) : NULL);
+	output = PageOutput(155, pager ? &(pset.popt.topt) : NULL);
 
 	fprintf(output, _("List of specially treated variables\n\n"));
 
@@ -360,6 +360,8 @@ helpVariables(unsigned short int pager)
 					  "    if set to \"noexec\", just show them without execution\n"));
 	fprintf(output, _("  ENCODING\n"
 					  "    current client character set encoding\n"));
+	fprintf(output, _("  ERROR\n"
+					  "    whether the last query failed\n"));
 	fprintf(output, _("  FETCH_COUNT\n"
 					  "    the number of result rows to fetch and display at a time (0 = unlimited)\n"));
 	fprintf(output, _("  HISTCONTROL\n"
@@ -374,6 +376,9 @@ helpVariables(unsigned short int pager)
 					  "    number of EOFs needed to terminate an interactive session\n"));
 	fprintf(output, _("  LASTOID\n"
 					  "    value of the last affected OID\n"));
+	fprintf(output, _("  LAST_ERROR_SQLSTATE\n"
+					  "  LAST_ERROR_MESSAGE\n"
+					  "    error code and message of last error, or \"00000\" and empty if none\n"));
 	fprintf(output, _("  ON_ERROR_ROLLBACK\n"
 					  "    if set, an error doesn't stop a transaction (uses implicit savepoints)\n"));
 	fprintf(output, _("  ON_ERROR_STOP\n"
@@ -388,6 +393,8 @@ helpVariables(unsigned short int pager)
 					  "    specifies the prompt used during COPY ... FROM STDIN\n"));
 	fprintf(output, _("  QUIET\n"
 					  "    run quietly (same as -q option)\n"));
+	fprintf(output, _("  ROW_COUNT\n"
+					  "    number of rows of last query, or 0\n"));
 	fprintf(output, _("  SERVER_VERSION_NAME\n"
 					  "  SERVER_VERSION_NUM\n"
 					  "    server's version (in short string or numeric format)\n"));
@@ -397,6 +404,8 @@ helpVariables(unsigned short int pager)
 					  "    if set, end of line terminates SQL commands (same as -S option)\n"));
 	fprintf(output, _("  SINGLESTEP\n"
 					  "    single-step mode (same as -s option)\n"));
+	fprintf(output, _("  SQLSTATE\n"
+					  "    error code of last query, or \"00000\" if no error\n"));
 	fprintf(output, _("  USER\n"
 					  "    the currently connected database user\n"));
 	fprintf(output, _("  VERBOSITY\n"
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index 1e48f4a..d020f3f 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -165,6 +165,10 @@ main(int argc, char *argv[])
 	SetVariable(pset.vars, "VERSION_NAME", PG_VERSION);
 	SetVariable(pset.vars, "VERSION_NUM", CppAsString2(PG_VERSION_NUM));
 
+	/* Create variables for last error */
+	SetVariable(pset.vars, "LAST_ERROR_SQLSTATE", "00000");
+	SetVariable(pset.vars, "LAST_ERROR_MESSAGE", "");
+
 	/* Default values for variables (that don't match the result of \unset) */
 	SetVariableBool(pset.vars, "AUTOCOMMIT");
 	SetVariable(pset.vars, "PROMPT1", DEFAULT_PROMPT1);
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index bda8960..51a98bf 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -28,6 +28,197 @@ on
 \unset ON_ERROR_ROLLBACK
 \echo :ON_ERROR_ROLLBACK
 off
+-- special result variables
+-- these tests are performed early to check that for values after startup
+-- 3 initially unset variables
+\echo 'error:' :ERROR
+error: :ERROR
+\echo 'error code:' :SQLSTATE
+error code: :SQLSTATE
+\echo 'number of rows:' :ROW_COUNT
+number of rows: :ROW_COUNT
+-- variables with default values
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+last error code: 00000
+\echo 'last error message:' :LAST_ERROR_MESSAGE
+last error message: 
+-- first working query, 2 rows select
+SELECT 1 AS stuff UNION SELECT 2;
+ stuff 
+-------
+     1
+     2
+(2 rows)
+
+\if :ERROR
+  \echo 'MUST NOT SHOW'
+\else
+  \echo 'ERROR is FALSE as expected'
+ERROR is FALSE as expected
+\endif
+\echo 'error code:' :SQLSTATE
+error code: 00000
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 2
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+last error code: 00000
+\echo 'last error message:' :LAST_ERROR_MESSAGE
+last error message: 
+-- syntax error
+SELECT 1 UNION;
+ERROR:  syntax error at or near ";"
+LINE 1: SELECT 1 UNION;
+                      ^
+\if :ERROR
+  \echo 'ERROR is TRUE as expected'
+ERROR is TRUE as expected
+\else
+  \echo 'MUST NOT SHOW'
+\endif
+\echo 'error code:' :SQLSTATE
+error code: 42601
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+last error code: 42601
+\echo 'error message:' :LAST_ERROR_MESSAGE
+error message: syntax error at or near ";"
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
+-- empty query
+;
+\echo 'error:' :ERROR
+error: false
+\echo 'error code:' :SQLSTATE
+error code: 00000
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
+-- must have kept previous values
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+last error code: 42601
+\echo 'error message:' :LAST_ERROR_MESSAGE
+error message: syntax error at or near ";"
+-- other query error
+DROP TABLE this_table_does_not_exist;
+ERROR:  table "this_table_does_not_exist" does not exist
+\echo 'error:' :ERROR
+error: true
+\echo 'error code:' :SQLSTATE
+error code: 42P01
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
+-- new values
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+last error code: 42P01
+\echo 'error message:' :LAST_ERROR_MESSAGE
+error message: table "this_table_does_not_exist" does not exist
+-- cleanup
+\unset LAST_ERROR_SQLSTATE
+\unset LAST_ERROR_MESSAGE
+-- working CURSOR
+DECLARE one CURSOR WITH HOLD FOR SELECT 1 AS one;
+\echo 'error:' :ERROR
+error: false
+\echo 'error code:' :SQLSTATE
+error code: 00000
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
+FETCH 2 FROM one;
+ one 
+-----
+   1
+(1 row)
+
+\echo 'error:' :ERROR
+error: false
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 1
+FETCH NEXT FROM one;
+ one 
+-----
+(0 rows)
+
+\echo 'error:' :ERROR
+error: false
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
+CLOSE one;
+\echo 'error:' :ERROR
+error: false
+-- CURSOR with syntax error
+DECLARE two CURSOR WITH HOLD FOR SELECT 1 + 1 +;
+ERROR:  syntax error at or near ";"
+LINE 1: DECLARE two CURSOR WITH HOLD FOR SELECT 1 + 1 +;
+                                                       ^
+\echo 'error:' :ERROR
+error: true
+\echo 'error code:' :SQLSTATE
+error code: 42601
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+last error code: 42601
+\echo 'error message:' :LAST_ERROR_MESSAGE
+error message: syntax error at or near ";"
+-- cleanup
+\unset LAST_ERROR_SQLSTATE
+\unset LAST_ERROR_MESSAGE
+-- CURSOR with execution error
+BEGIN;
+CREATE FUNCTION raise_an_error()
+RETURNS INTEGER
+IMMUTABLE STRICT AS $$
+BEGIN
+  RAISE EXCEPTION 'function raise_an_error()';
+  RETURN 1;
+END;
+$$ LANGUAGE plpgsql;
+DECLARE cursor_error CURSOR FOR SELECT raise_an_error();
+ERROR:  function raise_an_error()
+CONTEXT:  PL/pgSQL function raise_an_error() line 3 at RAISE
+\echo 'error:' :ERROR
+error: true
+\echo 'error code:' :SQLSTATE
+error code: P0001
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+last error code: P0001
+\echo 'error message:' :LAST_ERROR_MESSAGE
+error message: function raise_an_error()
+ROLLBACK;
+-- working description
+SELECT 3 AS three \gdesc
+ Column |  Type   
+--------+---------
+ three  | integer
+(1 row)
+
+\echo 'error:' :ERROR
+error: false
+\echo 'error code:' :SQLSTATE
+error code: 00000
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
+-- describe with an error
+SELECT 4 AS \gdesc
+ERROR:  syntax error at end of input
+LINE 1: SELECT 4 AS 
+                    ^
+\echo 'error:' :ERROR
+error: true
+\echo 'error code:' :SQLSTATE
+error code: 42601
+\echo 'number of rows:' :ROW_COUNT
+number of rows: 0
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+last error code: 42601
+\echo 'error message:' :LAST_ERROR_MESSAGE
+error message: syntax error at end of input
+-- cleanup all
+\unset ERROR
+\unset SQLSTATE
+\unset ROW_COUNT
+\unset LAST_ERROR_SQLSTATE
+\unset LAST_ERROR_MESSAGE
 -- \g and \gx
 SELECT 1 as one, 2 as two \g
  one | two 
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index 0556b7c..c3a8844 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -21,6 +21,130 @@
 \unset ON_ERROR_ROLLBACK
 \echo :ON_ERROR_ROLLBACK
 
+-- special result variables
+-- these tests are performed early to check that for values after startup
+
+-- 3 initially unset variables
+\echo 'error:' :ERROR
+\echo 'error code:' :SQLSTATE
+\echo 'number of rows:' :ROW_COUNT
+
+-- variables with default values
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+\echo 'last error message:' :LAST_ERROR_MESSAGE
+
+-- first working query, 2 rows select
+SELECT 1 AS stuff UNION SELECT 2;
+\if :ERROR
+  \echo 'MUST NOT SHOW'
+\else
+  \echo 'ERROR is FALSE as expected'
+\endif
+\echo 'error code:' :SQLSTATE
+\echo 'number of rows:' :ROW_COUNT
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+\echo 'last error message:' :LAST_ERROR_MESSAGE
+
+-- syntax error
+SELECT 1 UNION;
+\if :ERROR
+  \echo 'ERROR is TRUE as expected'
+\else
+  \echo 'MUST NOT SHOW'
+\endif
+\echo 'error code:' :SQLSTATE
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+\echo 'error message:' :LAST_ERROR_MESSAGE
+\echo 'number of rows:' :ROW_COUNT
+
+-- empty query
+;
+\echo 'error:' :ERROR
+\echo 'error code:' :SQLSTATE
+\echo 'number of rows:' :ROW_COUNT
+-- must have kept previous values
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+\echo 'error message:' :LAST_ERROR_MESSAGE
+
+-- other query error
+DROP TABLE this_table_does_not_exist;
+\echo 'error:' :ERROR
+\echo 'error code:' :SQLSTATE
+\echo 'number of rows:' :ROW_COUNT
+-- new values
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+\echo 'error message:' :LAST_ERROR_MESSAGE
+-- cleanup
+\unset LAST_ERROR_SQLSTATE
+\unset LAST_ERROR_MESSAGE
+
+-- working CURSOR
+DECLARE one CURSOR WITH HOLD FOR SELECT 1 AS one;
+\echo 'error:' :ERROR
+\echo 'error code:' :SQLSTATE
+\echo 'number of rows:' :ROW_COUNT
+FETCH 2 FROM one;
+\echo 'error:' :ERROR
+\echo 'number of rows:' :ROW_COUNT
+FETCH NEXT FROM one;
+\echo 'error:' :ERROR
+\echo 'number of rows:' :ROW_COUNT
+CLOSE one;
+\echo 'error:' :ERROR
+
+-- CURSOR with syntax error
+DECLARE two CURSOR WITH HOLD FOR SELECT 1 + 1 +;
+\echo 'error:' :ERROR
+\echo 'error code:' :SQLSTATE
+\echo 'number of rows:' :ROW_COUNT
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+\echo 'error message:' :LAST_ERROR_MESSAGE
+-- cleanup
+\unset LAST_ERROR_SQLSTATE
+\unset LAST_ERROR_MESSAGE
+
+-- CURSOR with execution error
+BEGIN;
+
+CREATE FUNCTION raise_an_error()
+RETURNS INTEGER
+IMMUTABLE STRICT AS $$
+BEGIN
+  RAISE EXCEPTION 'function raise_an_error()';
+  RETURN 1;
+END;
+$$ LANGUAGE plpgsql;
+
+DECLARE cursor_error CURSOR FOR SELECT raise_an_error();
+\echo 'error:' :ERROR
+\echo 'error code:' :SQLSTATE
+\echo 'number of rows:' :ROW_COUNT
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+\echo 'error message:' :LAST_ERROR_MESSAGE
+
+ROLLBACK;
+
+-- working description
+SELECT 3 AS three \gdesc
+\echo 'error:' :ERROR
+\echo 'error code:' :SQLSTATE
+\echo 'number of rows:' :ROW_COUNT
+
+-- describe with an error
+SELECT 4 AS \gdesc
+\echo 'error:' :ERROR
+\echo 'error code:' :SQLSTATE
+\echo 'number of rows:' :ROW_COUNT
+\echo 'last error code:' :LAST_ERROR_SQLSTATE
+\echo 'error message:' :LAST_ERROR_MESSAGE
+
+-- cleanup all
+\unset ERROR
+\unset SQLSTATE
+\unset ROW_COUNT
+\unset LAST_ERROR_SQLSTATE
+\unset LAST_ERROR_MESSAGE
+
 -- \g and \gx
 
 SELECT 1 as one, 2 as two \g
#38Tom Lane
tgl@sss.pgh.pa.us
In reply to: Fabien COELHO (#37)
Re: psql - add special variable to reflect the last query status

Fabien COELHO <coelho@cri.ensmp.fr> writes:

See v9 attached.

I've pushed this with some editorialization.

I put back SetResultVariables function which is called twice, for SQL
queries and the new descriptions. It worked out of the box with DECLARE
which is just another SQL statement, so maybe I did not understood the
cursor issue you were signaling...

No, I was concerned about ExecQueryUsingCursor(), which is used when
FETCH_COUNT is enabled. It's sort of a pain because you have to
accumulate the row count across multiple PGresults. If you don't,
then FETCH_COUNT mode isn't transparent, which it's supposed to be.

I did some performance testing of my own, based on this possibly-silly
test case:

perl -e 'for($i=0;$i<9999999;$i++) {print "set enable_seqscan=0;\n";}' | psql -q

The idea was to run a trivial query and minimize all other psql overhead,
particularly results-printing. With this, "perf" told me that
SetResultVariables and its called functions accounted for 1.5% of total
CPU (including the server processes). That's kind of high, but it's
probably tolerable considering that any real application would involve
both far more server work per query and far more psql work (at least for
SELECTs).

One thing we could think about if this seems too high is to drop
ROW_COUNT. I'm unconvinced that it has a real use-case, and it seems
to be taking more than its share of the work in non-error cases, because
it turns out that PQcmdTuples() is not an amazingly cheap function.

regards, tom lane

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

#39Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Tom Lane (#38)
Re: psql - add special variable to reflect the last query status

Hello Tom,

I put back SetResultVariables function which is called twice, for SQL
queries and the new descriptions. It worked out of the box with DECLARE
which is just another SQL statement, so maybe I did not understood the
cursor issue you were signaling...

No, I was concerned about ExecQueryUsingCursor(), which is used when
FETCH_COUNT is enabled. It's sort of a pain because you have to
accumulate the row count across multiple PGresults. If you don't,
then FETCH_COUNT mode isn't transparent, which it's supposed to be.

Please allow me to disagree a little with this semantics.

ISTM that the semantics of the simple previous implementation was fine,
"number of rows returned by previous statement". If you do "FETCH 3 ...",
then you get between 0 and 3 rows... Good. If you do it again, same...

I'm not sure having an accumulation semantics helps a lot, because it
creates an exception, and moreover I can think of legitimate use case
where counting only last statement rows would be useful, eg to check that
we are done with a cursor and it can be closed.

If someone really wants to accumulate, it can be done by hand quite
simply, currently as:

SELECT :ROW_COUNT + :accum AS accum \gset

or client side:

\set accum `expr :ROW_COUNT + :accum`

and maybe some day something like:

\let accum :ROW_COUNT + :accum

I did some performance testing of my own, based on this possibly-silly
test case: [...]

The idea was to run a trivial query and minimize all other psql overhead,
particularly results-printing. With this, "perf" told me that
SetResultVariables and its called functions accounted for 1.5% of total
CPU (including the server processes).

That's kind of high, but it's probably tolerable considering that any
real application would involve both far more server work per query and
far more psql work (at least for SELECTs).

This seems pretty reasonable to me, and is consistent with my 1% elapsed
time measure on a silent "SELECT;".

One thing we could think about if this seems too high is to drop
ROW_COUNT. I'm unconvinced that it has a real use-case, and it seems
to be taking more than its share of the work in non-error cases, because
it turns out that PQcmdTuples() is not an amazingly cheap function.

I do think that a small overhead on a contrived case is worth removing the
feature, as it is really insignificant on any realistic case.

--
Fabien.

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

#40Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Fabien COELHO (#39)
Re: psql - add special variable to reflect the last query status

One thing we could think about if this seems too high is to drop
ROW_COUNT. I'm unconvinced that it has a real use-case, and it seems
to be taking more than its share of the work in non-error cases, because
it turns out that PQcmdTuples() is not an amazingly cheap function.

I do think that a small overhead on a contrived case is worth removing the
feature, as it is really insignificant on any realistic case.

Please read: I do NOT think that...

--
Fabien.

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