Hello devs,
The attached patch implements a new SHOW_ALL_RESULTS option for psql,
which shows all results of a combined query (\;) instead of only the last
one.
This solves a frustration that intermediate results were hidden from view
for no good reason that I could think of.
For that, call PQsendQuery instead of (mostly not documented) PQexec, and
rework how results are processed afterwards.
Timing is moved to ProcessQueryResults to keep the last result handling
out of the measured time. I think it would not be a big deal to include
it, but this is the previous behavior.
In passing, refactor a little and add comments. Make function names about
results plural or singular consistently with the fact the it processes one
or several results. Change "PrintQueryResult" to "HandleQueryResult"
because it was not always printing something. Also add a HandleCopyResult
function, which makes the patch a little bigger by moving things around
but clarifies the code.
Code in "common.c" is actually a little shorter than the previous version.
From my point of view the code is clearer than before because there is
only one loop over results, not an implicit one within PQexec and another
one afterwards to handle copy.
Add a few tests for the new feature.
IMHO this new setting should be on by default: few people know about \; so
it would not change anything for most, and I do not see why those who use
it would not be interested by the results of all the queries they asked
for.
--
Fabien.
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 636df6c0ec..0633117c1b 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -3915,6 +3915,17 @@ bar
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><varname>SHOW_ALL_RESULTS</varname></term>
+ <listitem>
+ <para>
+ When this variable is set to <literal>on</literal>, all results of a combined
+ (<literal>\;</literal>) query are shown instead of just the last one.
+ Default is <literal>off</literal>.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><varname>SHOW_CONTEXT</varname></term>
<listitem>
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index bd284446f8..928f117a63 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -999,199 +999,113 @@ loop_exit:
return success;
}
-
/*
- * ProcessResult: utility function for use by SendQuery() only
- *
- * When our command string contained a COPY FROM STDIN or COPY TO STDOUT,
- * PQexec() has stopped at the PGresult associated with the first such
- * command. In that event, we'll marshal data for the COPY and then cycle
- * through any subsequent PGresult objects.
- *
- * When the command string contained no such COPY command, this function
- * degenerates to an AcceptResult() call.
- *
- * Changes its argument to point to the last PGresult of the command string,
- * or NULL if that result was for a COPY TO STDOUT. (Returning NULL prevents
- * the command status from being printed, which we want in that case so that
- * the status line doesn't get taken as part of the COPY data.)
- *
- * Returns true on complete success, false otherwise. Possible failure modes
- * include purely client-side problems; check the transaction status for the
- * server-side opinion.
+ * Marshal the COPY data. Either subroutine will get the
+ * connection out of its COPY state, then call PQresultStatus()
+ * once and report any error.
+ *
+ * For COPY OUT, direct the output to pset.copyStream if it's set,
+ * otherwise to pset.gfname if it's set, otherwise to queryFout.
+ * For COPY IN, use pset.copyStream as data source if it's set,
+ * otherwise cur_cmd_source.
+ *
+ * Update result if further processing is necessary. (Returning NULL
+ * prevents the command status from being printed, which we want in that
+ * case so that the status line doesn't get taken as part of the COPY data.)
*/
static bool
-ProcessResult(PGresult **results)
+HandleCopyResult(PGresult **result)
{
bool success = true;
- bool first_cycle = true;
+ FILE *copystream;
+ PGresult *copy_result;
+ ExecStatusType result_status = PQresultStatus(*result);
- for (;;)
+ Assert(result_status == PGRES_COPY_OUT ||
+ result_status == PGRES_COPY_IN);
+
+ SetCancelConn();
+ if (result_status == PGRES_COPY_OUT)
{
- ExecStatusType result_status;
- bool is_copy;
- PGresult *next_result;
+ bool need_close = false;
+ bool is_pipe = false;
- if (!AcceptResult(*results))
+ if (pset.copyStream)
{
- /*
- * Failure at this point is always a server-side failure or a
- * failure to submit the command string. Either way, we're
- * finished with this command string.
- */
- success = false;
- break;
+ /* invoked by \copy */
+ copystream = pset.copyStream;
}
-
- result_status = PQresultStatus(*results);
- switch (result_status)
+ else if (pset.gfname)
{
- case PGRES_EMPTY_QUERY:
- case PGRES_COMMAND_OK:
- case PGRES_TUPLES_OK:
- is_copy = false;
- break;
-
- case PGRES_COPY_OUT:
- case PGRES_COPY_IN:
- is_copy = true;
- break;
-
- default:
- /* AcceptResult() should have caught anything else. */
- is_copy = false;
- pg_log_error("unexpected PQresultStatus: %d", result_status);
- break;
- }
-
- if (is_copy)
- {
- /*
- * Marshal the COPY data. Either subroutine will get the
- * connection out of its COPY state, then call PQresultStatus()
- * once and report any error.
- *
- * For COPY OUT, direct the output to pset.copyStream if it's set,
- * otherwise to pset.gfname if it's set, otherwise to queryFout.
- * For COPY IN, use pset.copyStream as data source if it's set,
- * otherwise cur_cmd_source.
- */
- FILE *copystream;
- PGresult *copy_result;
-
- SetCancelConn();
- if (result_status == PGRES_COPY_OUT)
+ /* invoked by \g */
+ if (openQueryOutputFile(pset.gfname,
+ ©stream, &is_pipe))
{
- bool need_close = false;
- bool is_pipe = false;
-
- if (pset.copyStream)
- {
- /* invoked by \copy */
- copystream = pset.copyStream;
- }
- else if (pset.gfname)
- {
- /* invoked by \g */
- if (openQueryOutputFile(pset.gfname,
- ©stream, &is_pipe))
- {
- need_close = true;
- if (is_pipe)
- disable_sigpipe_trap();
- }
- else
- copystream = NULL; /* discard COPY data entirely */
- }
- else
- {
- /* fall back to the generic query output stream */
- copystream = pset.queryFout;
- }
-
- success = handleCopyOut(pset.db,
- copystream,
- ©_result)
- && success
- && (copystream != NULL);
-
- /*
- * Suppress status printing if the report would go to the same
- * place as the COPY data just went. Note this doesn't
- * prevent error reporting, since handleCopyOut did that.
- */
- if (copystream == pset.queryFout)
- {
- PQclear(copy_result);
- copy_result = NULL;
- }
-
- if (need_close)
- {
- /* close \g argument file/pipe */
- if (is_pipe)
- {
- pclose(copystream);
- restore_sigpipe_trap();
- }
- else
- {
- fclose(copystream);
- }
- }
+ need_close = true;
+ if (is_pipe)
+ disable_sigpipe_trap();
}
else
- {
- /* COPY IN */
- copystream = pset.copyStream ? pset.copyStream : pset.cur_cmd_source;
- success = handleCopyIn(pset.db,
- copystream,
- PQbinaryTuples(*results),
- ©_result) && success;
- }
- ResetCancelConn();
-
- /*
- * Replace the PGRES_COPY_OUT/IN result with COPY command's exit
- * status, or with NULL if we want to suppress printing anything.
- */
- PQclear(*results);
- *results = copy_result;
+ copystream = NULL; /* discard COPY data entirely */
}
- else if (first_cycle)
+ else
{
- /* fast path: no COPY commands; PQexec visited all results */
- break;
+ /* fall back to the generic query output stream */
+ copystream = pset.queryFout;
}
+ success = handleCopyOut(pset.db,
+ copystream,
+ ©_result)
+ && success
+ && (copystream != NULL);
+
/*
- * Check PQgetResult() again. In the typical case of a single-command
- * string, it will return NULL. Otherwise, we'll have other results
- * to process that may include other COPYs. We keep the last result.
+ * Suppress status printing if the report would go to the same
+ * place as the COPY data just went. Note this doesn't
+ * prevent error reporting, since handleCopyOut did that.
*/
- next_result = PQgetResult(pset.db);
- if (!next_result)
- break;
+ if (copystream == pset.queryFout)
+ {
+ PQclear(copy_result);
+ copy_result = NULL;
+ }
- PQclear(*results);
- *results = next_result;
- first_cycle = false;
+ if (need_close)
+ {
+ /* close \g argument file/pipe */
+ if (is_pipe)
+ {
+ pclose(copystream);
+ restore_sigpipe_trap();
+ }
+ else
+ {
+ fclose(copystream);
+ }
+ }
}
+ else
+ {
+ /* COPY IN */
+ copystream = pset.copyStream ? pset.copyStream : pset.cur_cmd_source;
+ success = handleCopyIn(pset.db,
+ copystream,
+ PQbinaryTuples(*result),
+ ©_result) && success;
+ }
+ ResetCancelConn();
- SetResultVariables(*results, success);
-
- /* may need this to recover from conn loss during COPY */
- if (!first_cycle && !CheckConnection())
- return false;
+ PQclear(*result);
+ *result = copy_result;
return success;
}
-
/*
* PrintQueryStatus: report command status as required
*
- * Note: Utility function for use by PrintQueryResults() only.
+ * Note: Utility function for use by HandleQueryResult() only.
*/
static void
PrintQueryStatus(PGresult *results)
@@ -1219,43 +1133,49 @@ PrintQueryStatus(PGresult *results)
/*
- * PrintQueryResults: print out (or store or execute) query results as required
- *
- * Note: Utility function for use by SendQuery() only.
+ * HandleQueryResult: print out, store or execute one query result
+ * as required.
*
* Returns true if the query executed successfully, false otherwise.
*/
static bool
-PrintQueryResults(PGresult *results)
+HandleQueryResult(PGresult *result, bool last)
{
bool success;
const char *cmdstatus;
- if (!results)
+ if (result == NULL)
return false;
- switch (PQresultStatus(results))
+ switch (PQresultStatus(result))
{
case PGRES_TUPLES_OK:
/* store or execute or print the data ... */
- if (pset.gset_prefix)
- success = StoreQueryTuple(results);
- else if (pset.gexec_flag)
- success = ExecQueryTuples(results);
- else if (pset.crosstab_flag)
- success = PrintResultsInCrosstab(results);
+ if (last && pset.gset_prefix)
+ success = StoreQueryTuple(result);
+ else if (last && pset.gexec_flag)
+ success = ExecQueryTuples(result);
+ else if (last && pset.crosstab_flag)
+ success = PrintResultsInCrosstab(result);
+ else if (last || pset.show_all_results)
+ success = PrintQueryTuples(result);
+ /* if it's INSERT/UPDATE/DELETE RETURNING, also print status */
else
- success = PrintQueryTuples(results);
- /* if it's INSERT/UPDATE/DELETE RETURNING, also print status */
- cmdstatus = PQcmdStatus(results);
- if (strncmp(cmdstatus, "INSERT", 6) == 0 ||
- strncmp(cmdstatus, "UPDATE", 6) == 0 ||
- strncmp(cmdstatus, "DELETE", 6) == 0)
- PrintQueryStatus(results);
+ success = true;
+
+ if (last || pset.show_all_results)
+ {
+ cmdstatus = PQcmdStatus(result);
+ if (strncmp(cmdstatus, "INSERT", 6) == 0 ||
+ strncmp(cmdstatus, "UPDATE", 6) == 0 ||
+ strncmp(cmdstatus, "DELETE", 6) == 0)
+ PrintQueryStatus(result);
+ }
break;
case PGRES_COMMAND_OK:
- PrintQueryStatus(results);
+ if (last || pset.show_all_results)
+ PrintQueryStatus(result);
success = true;
break;
@@ -1265,7 +1185,7 @@ PrintQueryResults(PGresult *results)
case PGRES_COPY_OUT:
case PGRES_COPY_IN:
- /* nothing to do here */
+ /* nothing to do here: already processed */
success = true;
break;
@@ -1278,7 +1198,7 @@ PrintQueryResults(PGresult *results)
default:
success = false;
pg_log_error("unexpected PQresultStatus: %d",
- PQresultStatus(results));
+ PQresultStatus(result));
break;
}
@@ -1288,6 +1208,89 @@ PrintQueryResults(PGresult *results)
}
+/*
+ * ProcessResults: utility function for use by SendQuery() only
+ *
+ * Cycles through PGresult objects.
+ *
+ * When our command string contained a COPY FROM STDIN or COPY TO STDOUT,
+ * the PGresult associated with these commands must be processed. In that
+ * event, we'll marshal data for the COPY.
+ *
+ * Returns true on complete success, false otherwise. Possible failure modes
+ * include purely client-side problems; check the transaction status for the
+ * server-side opinion.
+ *
+ * Note that on a combined query, failure does not mean that nothing was
+ * committed.
+ */
+static bool
+ProcessResults(instr_time before, double *pelapsed_msec)
+{
+ bool success = true;
+ PGresult *result = PQgetResult(pset.db);
+
+ while (result != NULL)
+ {
+ ExecStatusType result_status;
+ PGresult *next_result;
+ bool last;
+
+ if (!AcceptResult(result))
+ {
+ /* some error occured, record that */
+ SetResultVariables(result, false);
+ PQclear(result);
+ success = false;
+
+ /* and switch to next result */
+ result = PQgetResult(pset.db);
+ continue;
+ }
+
+ /* must handle COPY before changing the current result */
+ result_status = PQresultStatus(result);
+ if (result_status == PGRES_COPY_IN ||
+ result_status == PGRES_COPY_OUT)
+ HandleCopyResult(&result);
+
+ /*
+ * Check PQgetResult() again. In the typical case of a single-command
+ * string, it will return NULL. Otherwise, we'll have other results
+ * to process that may include other COPYs.
+ */
+ next_result = PQgetResult(pset.db);
+ last = (next_result == NULL);
+
+ /* timing measure before printing the last result */
+ if (last && pset.timing)
+ {
+ instr_time now;
+ INSTR_TIME_SET_CURRENT(now);
+ INSTR_TIME_SUBTRACT(now, before);
+ *pelapsed_msec = INSTR_TIME_GET_MILLISEC(now);
+ }
+
+ /* this may or may not print something depending on settings */
+ if (result != NULL)
+ success &= HandleQueryResult(result, last);
+
+ /* set variables on last result if all went well */
+ if (last && success)
+ SetResultVariables(result, true);
+
+ PQclear(result);
+ result = next_result;
+ }
+
+ /* may need this to recover from conn loss during COPY */
+ if (!CheckConnection())
+ return false;
+
+ return success;
+}
+
+
/*
* SendQuery: send the query string to the backend
* (and print out results)
@@ -1303,7 +1306,7 @@ PrintQueryResults(PGresult *results)
bool
SendQuery(const char *query)
{
- PGresult *results;
+ PGresult *results = NULL;
PGTransactionStatusType transaction_status;
double elapsed_msec = 0;
bool OK = false;
@@ -1408,28 +1411,18 @@ SendQuery(const char *query)
pset.crosstab_flag || !is_select_command(query))
{
/* Default fetch-it-all-and-print mode */
- instr_time before,
- after;
+ instr_time before;
if (pset.timing)
INSTR_TIME_SET_CURRENT(before);
- results = PQexec(pset.db, query);
+ OK = PQsendQuery(pset.db, query);
- /* these operations are included in the timing result: */
ResetCancelConn();
- OK = ProcessResult(&results);
- if (pset.timing)
- {
- INSTR_TIME_SET_CURRENT(after);
- INSTR_TIME_SUBTRACT(after, before);
- elapsed_msec = INSTR_TIME_GET_MILLISEC(after);
- }
-
- /* but printing results isn't: */
- if (OK && results)
- OK = PrintQueryResults(results);
+ if (OK)
+ /* timing is stop before the last display */
+ OK = ProcessResults(before, &elapsed_msec);
}
else
{
@@ -1664,7 +1657,7 @@ DescribeQuery(const char *query, double *elapsed_msec)
}
if (OK && results)
- OK = PrintQueryResults(results);
+ OK = HandleQueryResult(results, true);
termPQExpBuffer(&buf);
}
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
index 5be5091f0e..f26bf7fc6b 100644
--- a/src/bin/psql/settings.h
+++ b/src/bin/psql/settings.h
@@ -127,6 +127,7 @@ typedef struct _psqlSettings
bool quiet;
bool singleline;
bool singlestep;
+ bool show_all_results;
bool hide_tableam;
int fetch_count;
int histsize;
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index 855133bbcb..24b2fc8c28 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -882,6 +882,12 @@ singlestep_hook(const char *newval)
return ParseVariableBool(newval, "SINGLESTEP", &pset.singlestep);
}
+static bool
+show_all_results_hook(const char *newval)
+{
+ return ParseVariableBool(newval, "SHOW_ALL_RESULTS", &pset.show_all_results);
+}
+
static char *
fetch_count_substitute_hook(char *newval)
{
@@ -1182,6 +1188,9 @@ EstablishVariableSpace(void)
SetVariableHooks(pset.vars, "SINGLESTEP",
bool_substitute_hook,
singlestep_hook);
+ SetVariableHooks(pset.vars, "SHOW_ALL_RESULTS",
+ bool_substitute_hook,
+ show_all_results_hook);
SetVariableHooks(pset.vars, "FETCH_COUNT",
fetch_count_substitute_hook,
fetch_count_hook);
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index bcddc7601e..b66e5db9e2 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -3662,7 +3662,7 @@ psql_completion(const char *text, int start, int end)
else if (TailMatchesCS("\\set", MatchAny))
{
if (TailMatchesCS("AUTOCOMMIT|ON_ERROR_STOP|QUIET|"
- "SINGLELINE|SINGLESTEP"))
+ "SINGLELINE|SINGLESTEP|SHOW_ALL_RESULTS"))
COMPLETE_WITH_CS("on", "off");
else if (TailMatchesCS("COMP_KEYWORD_CASE"))
COMPLETE_WITH_CS("lower", "upper",
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index 23e540d2bb..778c66b819 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -4729,3 +4729,46 @@ drop schema testpart;
set search_path to default;
set role to default;
drop role testrole_partitioning;
+--
+-- combined queries & SHOW_ALL_RESULTS option
+--
+\echo '# SHOW_ALL_RESULTS tests'
+# SHOW_ALL_RESULTS tests
+\set SHOW_ALL_RESULTS on
+-- show both
+SELECT 1 AS one \; SELECT 2 AS two ;
+ one
+-----
+ 1
+(1 row)
+
+ two
+-----
+ 2
+(1 row)
+
+-- \gset applies to last query only
+SELECT 3 AS three \; SELECT 4 AS four \gset
+ three
+-------
+ 3
+(1 row)
+
+\echo :three :four
+:three 4
+-- syntax error stops all processing
+SELECT 5 \; SELECT 6 + \; SELECT 7 ;
+ERROR: syntax error at or near ";"
+LINE 1: SELECT 5 ; SELECT 6 + ; SELECT 7 ;
+ ^
+-- with aborted transaction, stop on first error
+BEGIN \; SELECT 8 AS eight \; SELECT 9/0 AS nine \; ROLLBACK \; SELECT 10 AS ten ;
+ eight
+-------
+ 8
+(1 row)
+
+ERROR: division by zero
+-- close previously aborted transaction
+ROLLBACK;
+\set SHOW_ALL_RESULTS off
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index 78f4b5d7d5..d0caaf47dc 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -1115,3 +1115,21 @@ set search_path to default;
set role to default;
drop role testrole_partitioning;
+
+--
+-- combined queries & SHOW_ALL_RESULTS option
+--
+\echo '# SHOW_ALL_RESULTS tests'
+\set SHOW_ALL_RESULTS on
+-- show both
+SELECT 1 AS one \; SELECT 2 AS two ;
+-- \gset applies to last query only
+SELECT 3 AS three \; SELECT 4 AS four \gset
+\echo :three :four
+-- syntax error stops all processing
+SELECT 5 \; SELECT 6 + \; SELECT 7 ;
+-- with aborted transaction, stop on first error
+BEGIN \; SELECT 8 AS eight \; SELECT 9/0 AS nine \; ROLLBACK \; SELECT 10 AS ten ;
+-- close previously aborted transaction
+ROLLBACK;
+\set SHOW_ALL_RESULTS off
