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.
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
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to