Hello Kyotaro-san,
Thanks. I looked this more closely.
Indeed! Thanks for this detailed review.
+ * Marshal the COPY data. Either subroutine will get the
+ * connection out of its COPY state, then call PQresultStatus()
+ * once and report any error.
This comment doesn't explain what the result value means.
Ok, added.
+ * 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.
I think this is not needed. This phrase was needed to explain why
we need to loop over subsequent results after PQexec in the
current code, but in this patch PQsendQuery is used instead,
which doesn't suffer somewhat confusing behavior. All results are
handled without needing an unusual processing.
Hmmm. More or less. "COPY" commands have two results, one for taking over
the input or output streams more or less directly at the protocol level,
and one for the final summary, which is quite special compared to other
commands, all that managed in "copy.c". So ISTM that the comment is
somehow still appropriate.
The difference with the previous behavior is that other results could be
immediately discarded but these ones, while now they are all processed.
I've kept this comment and added another one to try to make that clear.
+ * 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.)
It seems that the purpose of the returned PGresult is only
printing status of this COPY. If it is true, I'd like to see
something like the following example.
| Returns result in the case where queryFout is safe to output
| result status. That is, in the case of COPY IN, or in the case
| where COPY OUT is written to other than pset.queryFout.
I have tried to improved the comment based on your suggestion.
+ if (!AcceptResult(result, false))
+ {
+ /* some error occured, record that */
+ ShowNoticeMessage(¬es);
The comment in the original code was:
- /*
- * 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.
- */
The first half of the comment seems to be true for this
patch. Don't we preserve that comment?
Ok. Some form put back.
+ success = handleCopyOut(pset.db,
+ copystream,
+ ©_result)
+ && success
+ && (copystream != NULL);
success is always true at thie point so "&& success" is no longer
useful.
Ok.
(It is same for the COPY IN case).
Ok.
+ /* must handle COPY before changing the current result */
+ result_status = PQresultStatus(result);
+ if (result_status == PGRES_COPY_IN ||
+ result_status == PGRES_COPY_OUT)
I didn't get "before changing the curren result" in the comment. Isn't
"handle COPY stream if any" enough?
Alas, I think not.
The issue is that I need to know whether this is the last result (eg \gset
applies only on the last result), so I'll call PQgetResult() to get that.
However, on COPY, this is the second "final" result which says how much
was copied. If I have not send/received the data, the count will not be
right.
+ if (result_status == PGRES_COPY_IN ||
+ result_status == PGRES_COPY_OUT)
+ {
+ ShowNoticeMessage(¬es);
+ HandleCopyResult(&result);
+ }
It seems that it is wrong that this ignores the return value of
HandleCopyResult().
Yep. Fixed.
+ /* timing measure before printing the last result */
+ if (last && pset.timing)
I'm not sure whether we reached any consensus with ths
behavior. This means the timing includes result-printing time of
other than the last one. If we don't want include printing time
at all, we can exclude it with a small amount of additional
complexity.
I think that this point is desperate, because the way timing is
implemented client-side.
Although we could try to stop timing before each result processing, it
would not prevent the server to go on with other queries and send back
results, psql to receive further results (next_result), so the final
figures would be stupid anyway, just another form of stupid.
Basically the approach cannot work with combined queries: It only worked
before because the intermediate results were coldly discarded.
Maybe the server to report its execution times for each query somehow, but
then the communication time would not be included.
I have added a comment about why timing does not make much sense with
combined queries.
Attached a v5.
--
Fabien.
diff --git a/contrib/pg_stat_statements/expected/pg_stat_statements.out b/contrib/pg_stat_statements/expected/pg_stat_statements.out
index 6787ec1efd..de59a5cf65 100644
--- a/contrib/pg_stat_statements/expected/pg_stat_statements.out
+++ b/contrib/pg_stat_statements/expected/pg_stat_statements.out
@@ -49,17 +49,42 @@ BEGIN \;
SELECT 2.0 AS "float" \;
SELECT 'world' AS "text" \;
COMMIT;
+ float
+-------
+ 2.0
+(1 row)
+
+ text
+-------
+ world
+(1 row)
+
-- compound with empty statements and spurious leading spacing
-\;\; SELECT 3 + 3 \;\;\; SELECT ' ' || ' !' \;\; SELECT 1 + 4 \;;
- ?column?
-----------
- 5
+\;\; SELECT 3 + 3 AS "+" \;\;\; SELECT ' ' || ' !' AS "||" \;\; SELECT 1 + 4 AS "+" \;;
+ +
+---
+ 6
+(1 row)
+
+ ||
+-----
+ !
+(1 row)
+
+ +
+---
+ 5
(1 row)
-- non ;-terminated statements
SELECT 1 + 1 + 1 AS "add" \gset
SELECT :add + 1 + 1 AS "add" \;
SELECT :add + 1 + 1 AS "add" \gset
+ add
+-----
+ 5
+(1 row)
+
-- set operator
SELECT 1 AS i UNION SELECT 2 ORDER BY i;
i
@@ -102,12 +127,12 @@ SELECT query, calls, rows FROM pg_stat_statements ORDER BY query COLLATE "C";
SELECT $1 +| 4 | 4
+| |
AS "text" | |
- SELECT $1 + $2 | 2 | 2
SELECT $1 + $2 + $3 AS "add" | 3 | 3
+ SELECT $1 + $2 AS "+" | 2 | 2
SELECT $1 AS "float" | 1 | 1
SELECT $1 AS "int" | 2 | 2
SELECT $1 AS i UNION SELECT $2 ORDER BY i | 1 | 2
- SELECT $1 || $2 | 1 | 1
+ SELECT $1 || $2 AS "||" | 1 | 1
SELECT pg_stat_statements_reset() | 1 | 1
WITH t(f) AS ( +| 1 | 2
VALUES ($1), ($2) +| |
diff --git a/contrib/pg_stat_statements/sql/pg_stat_statements.sql b/contrib/pg_stat_statements/sql/pg_stat_statements.sql
index 8b527070d4..ea3a31176e 100644
--- a/contrib/pg_stat_statements/sql/pg_stat_statements.sql
+++ b/contrib/pg_stat_statements/sql/pg_stat_statements.sql
@@ -27,7 +27,7 @@ SELECT 'world' AS "text" \;
COMMIT;
-- compound with empty statements and spurious leading spacing
-\;\; SELECT 3 + 3 \;\;\; SELECT ' ' || ' !' \;\; SELECT 1 + 4 \;;
+\;\; SELECT 3 + 3 AS "+" \;\;\; SELECT ' ' || ' !' AS "||" \;\; SELECT 1 + 4 AS "+" \;;
-- non ;-terminated statements
SELECT 1 + 1 + 1 AS "add" \gset
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 7789fc6177..4e6ab5a0a5 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -127,18 +127,11 @@ echo '\x \\ SELECT * FROM foo;' | psql
commands included in the string to divide it into multiple
transactions. (See <xref linkend="protocol-flow-multi-statement"/>
for more details about how the server handles multi-query strings.)
- Also, <application>psql</application> only prints the
- result of the last <acronym>SQL</acronym> command in the string.
- This is different from the behavior when the same string is read from
- a file or fed to <application>psql</application>'s standard input,
- because then <application>psql</application> sends
- each <acronym>SQL</acronym> command separately.
</para>
<para>
- Because of this behavior, putting more than one SQL command in a
- single <option>-c</option> string often has unexpected results.
- It's better to use repeated <option>-c</option> commands or feed
- multiple commands to <application>psql</application>'s standard input,
+ If having several commands executed in one transaction is not desired,
+ use repeated <option>-c</option> commands or feed multiple commands to
+ <application>psql</application>'s standard input,
either using <application>echo</application> as illustrated above, or
via a shell here-document, for example:
<programlisting>
@@ -3355,10 +3348,8 @@ select 1\; select 2\; select 3;
commands included in the string to divide it into multiple
transactions. (See <xref linkend="protocol-flow-multi-statement"/>
for more details about how the server handles multi-query strings.)
- <application>psql</application> prints only the last query result
- it receives for each request; in this example, although all
- three <command>SELECT</command>s are indeed executed, <application>psql</application>
- only prints the <literal>3</literal>.
+ <application>psql</application> prints all results it receives, one
+ after the other.
</para>
</listitem>
</varlistentry>
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 44a782478d..94a2f2d96d 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -472,6 +472,16 @@ ResetCancelConn(void)
#endif
}
+static void
+ShowErrorMessage(const PGresult *result)
+{
+ const char *error = PQerrorMessage(pset.db);
+
+ if (strlen(error))
+ pg_log_info("%s", error);
+
+ CheckConnection();
+}
/*
* AcceptResult
@@ -482,7 +492,7 @@ ResetCancelConn(void)
* Returns true for valid result, false for error state.
*/
static bool
-AcceptResult(const PGresult *result)
+AcceptResult(const PGresult *result, bool show_error)
{
bool OK;
@@ -513,15 +523,8 @@ AcceptResult(const PGresult *result)
break;
}
- if (!OK)
- {
- const char *error = PQerrorMessage(pset.db);
-
- if (strlen(error))
- pg_log_info("%s", error);
-
- CheckConnection();
- }
+ if (!OK && show_error)
+ ShowErrorMessage(result);
return OK;
}
@@ -701,7 +704,7 @@ PSQLexec(const char *query)
ResetCancelConn();
- if (!AcceptResult(res))
+ if (!AcceptResult(res, true))
{
ClearOrSaveResult(res);
res = NULL;
@@ -743,7 +746,7 @@ PSQLexecWatch(const char *query, const printQueryOpt *opt)
ResetCancelConn();
- if (!AcceptResult(res))
+ if (!AcceptResult(res, true))
{
ClearOrSaveResult(res);
return 0;
@@ -999,199 +1002,114 @@ 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. Return whether all was ok.
+ *
+ * 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, or NULL otherwise.
+ * Return a result when queryFout can safely output a result status:
+ * on COPY IN, or on COPY OUT if written to something other han pset.queryFout.
+ * Returning NULL prevents the command status from being printed, which
+ * we want if 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;
+ bool success;
+ 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)
+ && (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);
+ }
+ 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 +1137,44 @@ 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
- success = PrintQueryTuples(results);
+ success = PrintQueryTuples(result);
+
/* if it's INSERT/UPDATE/DELETE RETURNING, also print status */
- cmdstatus = PQcmdStatus(results);
+ cmdstatus = PQcmdStatus(result);
if (strncmp(cmdstatus, "INSERT", 6) == 0 ||
strncmp(cmdstatus, "UPDATE", 6) == 0 ||
strncmp(cmdstatus, "DELETE", 6) == 0)
- PrintQueryStatus(results);
+ PrintQueryStatus(result);
+
break;
case PGRES_COMMAND_OK:
- PrintQueryStatus(results);
+ PrintQueryStatus(result);
success = true;
break;
@@ -1265,7 +1184,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 +1197,7 @@ PrintQueryResults(PGresult *results)
default:
success = false;
pg_log_error("unexpected PQresultStatus: %d",
- PQresultStatus(results));
+ PQresultStatus(result));
break;
}
@@ -1287,6 +1206,170 @@ PrintQueryResults(PGresult *results)
return success;
}
+/*
+ * Data structure and functions to record notices while they are
+ * emitted, so that they can be shown later.
+ *
+ * We need to know which result is last, which requires to extract
+ * one result in advance, hence two buffers are needed.
+ */
+typedef struct {
+ bool in_flip;
+ PQExpBufferData flip;
+ PQExpBufferData flop;
+} t_notice_messages;
+
+static void
+AppendNoticeMessage(void *arg, const char *msg)
+{
+ t_notice_messages *notes = (t_notice_messages*) arg;
+ appendPQExpBufferStr(notes->in_flip ? ¬es->flip : ¬es->flop, msg);
+}
+
+static void
+ShowNoticeMessage(t_notice_messages *notes)
+{
+ PQExpBufferData *current = notes->in_flip ? ¬es->flip : ¬es->flop;
+ if (current->data != NULL && *current->data != '\0')
+ pg_log_info("%s", current->data);
+ resetPQExpBuffer(current);
+}
+
+/*
+ * SendQueryAndProcessResults: utility function for use by SendQuery() only
+ *
+ * Sends query and 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 by providing
+ * an input or output stream. In that event, we'll marshal data for the COPY.
+ *
+ * For other commands, the results are processed normally, depending on their
+ * status.
+ *
+ * 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
+SendQueryAndProcessResults(const char *query, double *pelapsed_msec)
+{
+ bool success;
+ instr_time before;
+ PGresult *result;
+ t_notice_messages notes;
+
+ if (pset.timing)
+ INSTR_TIME_SET_CURRENT(before);
+
+ success = PQsendQuery(pset.db, query);
+ ResetCancelConn();
+
+ if (!success)
+ return false;
+
+ /* intercept notices */
+ notes.in_flip = true;
+ initPQExpBuffer(¬es.flip);
+ initPQExpBuffer(¬es.flop);
+ PQsetNoticeProcessor(pset.db, AppendNoticeMessage, ¬es);
+
+ /* first result */
+ result = PQgetResult(pset.db);
+
+ while (result != NULL)
+ {
+ ExecStatusType result_status;
+ PGresult *next_result;
+ bool last;
+
+ if (!AcceptResult(result, false))
+ {
+ /*
+ * Some error occured, either a server-side failure or
+ * a failure to submit the command string. Record that.
+ */
+ ShowNoticeMessage(¬es);
+ ShowErrorMessage(result);
+ SetResultVariables(result, false);
+ ClearOrSaveResult(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)
+ {
+ ShowNoticeMessage(¬es);
+ success &= 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.
+ */
+ notes.in_flip = !notes.in_flip;
+ next_result = PQgetResult(pset.db);
+ notes.in_flip = !notes.in_flip;
+ last = (next_result == NULL);
+
+ /*
+ * Get timing measure before printing the last result.
+ *
+ * It will include the display of previous results, if any.
+ * This cannot be helped because the server goes on processing
+ * further queries anyway while the previous ones are being displayed.
+ * The parallel execution of the client display hides the server time
+ * when it is shorter.
+ *
+ * With combined queries, timing must be understood as an upper bound
+ * of the time spent processing them.
+ */
+ if (last && pset.timing)
+ {
+ instr_time now;
+ INSTR_TIME_SET_CURRENT(now);
+ INSTR_TIME_SUBTRACT(now, before);
+ *pelapsed_msec = INSTR_TIME_GET_MILLISEC(now);
+ }
+
+ /* notices already show above for copy */
+ ShowNoticeMessage(¬es);
+
+ /* 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);
+
+ ClearOrSaveResult(result);
+ notes.in_flip = !notes.in_flip;
+ result = next_result;
+ }
+
+ /* reset notice hook */
+ PQsetNoticeProcessor(pset.db, NoticeProcessor, NULL);
+ termPQExpBuffer(¬es.flip);
+ termPQExpBuffer(¬es.flop);
+
+ /* may need this to recover from conn loss during COPY */
+ if (!CheckConnection())
+ return false;
+
+ return success;
+}
+
/*
* SendQuery: send the query string to the backend
@@ -1303,9 +1386,9 @@ PrintQueryResults(PGresult *results)
bool
SendQuery(const char *query)
{
- PGresult *results;
+ PGresult *results = NULL;
PGTransactionStatusType transaction_status;
- double elapsed_msec = 0;
+ double elapsed_msec = 0.0;
bool OK = false;
int i;
bool on_error_rollback_savepoint = false;
@@ -1408,28 +1491,7 @@ SendQuery(const char *query)
pset.crosstab_flag || !is_select_command(query))
{
/* Default fetch-it-all-and-print mode */
- instr_time before,
- after;
-
- if (pset.timing)
- INSTR_TIME_SET_CURRENT(before);
-
- results = PQexec(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);
+ OK = SendQueryAndProcessResults(query, &elapsed_msec);
}
else
{
@@ -1606,7 +1668,7 @@ DescribeQuery(const char *query, double *elapsed_msec)
PQclear(results);
results = PQdescribePrepared(pset.db, "");
- OK = AcceptResult(results) &&
+ OK = AcceptResult(results, true) &&
(PQresultStatus(results) == PGRES_COMMAND_OK);
if (OK && results)
{
@@ -1654,7 +1716,7 @@ DescribeQuery(const char *query, double *elapsed_msec)
PQclear(results);
results = PQexec(pset.db, buf.data);
- OK = AcceptResult(results);
+ OK = AcceptResult(results, true);
if (pset.timing)
{
@@ -1664,7 +1726,7 @@ DescribeQuery(const char *query, double *elapsed_msec)
}
if (OK && results)
- OK = PrintQueryResults(results);
+ OK = HandleQueryResult(results, true);
termPQExpBuffer(&buf);
}
@@ -1723,7 +1785,7 @@ ExecQueryUsingCursor(const char *query, double *elapsed_msec)
if (PQtransactionStatus(pset.db) == PQTRANS_IDLE)
{
results = PQexec(pset.db, "BEGIN");
- OK = AcceptResult(results) &&
+ OK = AcceptResult(results, true) &&
(PQresultStatus(results) == PGRES_COMMAND_OK);
ClearOrSaveResult(results);
if (!OK)
@@ -1737,7 +1799,7 @@ ExecQueryUsingCursor(const char *query, double *elapsed_msec)
query);
results = PQexec(pset.db, buf.data);
- OK = AcceptResult(results) &&
+ OK = AcceptResult(results, true) &&
(PQresultStatus(results) == PGRES_COMMAND_OK);
if (!OK)
SetResultVariables(results, OK);
@@ -1814,7 +1876,7 @@ ExecQueryUsingCursor(const char *query, double *elapsed_msec)
is_pager = false;
}
- OK = AcceptResult(results);
+ OK = AcceptResult(results, true);
Assert(!OK);
SetResultVariables(results, OK);
ClearOrSaveResult(results);
@@ -1923,7 +1985,7 @@ cleanup:
results = PQexec(pset.db, "CLOSE _psql_cursor");
if (OK)
{
- OK = AcceptResult(results) &&
+ OK = AcceptResult(results, true) &&
(PQresultStatus(results) == PGRES_COMMAND_OK);
ClearOrSaveResult(results);
}
@@ -1933,7 +1995,7 @@ cleanup:
if (started_txn)
{
results = PQexec(pset.db, OK ? "COMMIT" : "ROLLBACK");
- OK &= AcceptResult(results) &&
+ OK &= AcceptResult(results, true) &&
(PQresultStatus(results) == PGRES_COMMAND_OK);
ClearOrSaveResult(results);
}
diff --git a/src/test/regress/expected/copy2.out b/src/test/regress/expected/copy2.out
index c53ed3ebf5..9c8c8361f8 100644
--- a/src/test/regress/expected/copy2.out
+++ b/src/test/regress/expected/copy2.out
@@ -475,10 +475,10 @@ copy check_con_tbl from stdin;
NOTICE: input = {"f1":1}
NOTICE: input = {"f1":null}
copy check_con_tbl from stdin;
-NOTICE: input = {"f1":0}
ERROR: new row for relation "check_con_tbl" violates check constraint "check_con_tbl_check"
DETAIL: Failing row contains (0).
CONTEXT: COPY check_con_tbl, line 1: "0"
+NOTICE: input = {"f1":0}
select * from check_con_tbl;
f1
----
diff --git a/src/test/regress/expected/copydml.out b/src/test/regress/expected/copydml.out
index 1b533962c6..b5a225628f 100644
--- a/src/test/regress/expected/copydml.out
+++ b/src/test/regress/expected/copydml.out
@@ -84,10 +84,10 @@ drop rule qqq on copydml_test;
create function qqq_trig() returns trigger as $$
begin
if tg_op in ('INSERT', 'UPDATE') then
- raise notice '% %', tg_op, new.id;
+ raise notice '% % %', tg_when, tg_op, new.id;
return new;
else
- raise notice '% %', tg_op, old.id;
+ raise notice '% % %', tg_when, tg_op, old.id;
return old;
end if;
end
@@ -97,16 +97,16 @@ create trigger qqqbef before insert or update or delete on copydml_test
create trigger qqqaf after insert or update or delete on copydml_test
for each row execute procedure qqq_trig();
copy (insert into copydml_test (t) values ('f') returning id) to stdout;
-NOTICE: INSERT 8
+NOTICE: BEFORE INSERT 8
8
-NOTICE: INSERT 8
+NOTICE: AFTER INSERT 8
copy (update copydml_test set t = 'g' where t = 'f' returning id) to stdout;
-NOTICE: UPDATE 8
+NOTICE: BEFORE UPDATE 8
8
-NOTICE: UPDATE 8
+NOTICE: AFTER UPDATE 8
copy (delete from copydml_test where t = 'g' returning id) to stdout;
-NOTICE: DELETE 8
+NOTICE: BEFORE DELETE 8
8
-NOTICE: DELETE 8
+NOTICE: AFTER DELETE 8
drop table copydml_test;
drop function qqq_trig();
diff --git a/src/test/regress/expected/copyselect.out b/src/test/regress/expected/copyselect.out
index 72865fe1eb..39ab8fc87a 100644
--- a/src/test/regress/expected/copyselect.out
+++ b/src/test/regress/expected/copyselect.out
@@ -126,7 +126,7 @@ copy (select 1) to stdout\; select 1/0; -- row, then error
ERROR: division by zero
select 1/0\; copy (select 1) to stdout; -- error only
ERROR: division by zero
-copy (select 1) to stdout\; copy (select 2) to stdout\; select 0\; select 3; -- 1 2 3
+copy (select 1) to stdout\; copy (select 2) to stdout\; select 3\; select 4; -- 1 2 3 4
1
2
?column?
@@ -134,8 +134,18 @@ copy (select 1) to stdout\; copy (select 2) to stdout\; select 0\; select 3; --
3
(1 row)
+ ?column?
+----------
+ 4
+(1 row)
+
create table test3 (c int);
select 0\; copy test3 from stdin\; copy test3 from stdin\; select 1; -- 1
+ ?column?
+----------
+ 0
+(1 row)
+
?column?
----------
1
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index ef534a36a0..a93736187a 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -4760,3 +4760,87 @@ Owning table: "pg_catalog.pg_statistic"
Indexes:
"pg_toast_2619_index" PRIMARY KEY, btree (chunk_id, chunk_seq)
+--
+-- combined queries
+--
+\echo '# combined queries tests'
+# combined queries tests
+CREATE FUNCTION warn(msg TEXT) RETURNS BOOLEAN AS $$
+ BEGIN RAISE NOTICE 'warn %', msg ; RETURN TRUE ; END
+$$ LANGUAGE plpgsql;
+-- show both
+SELECT 1 AS one \; SELECT warn('1.5') \; SELECT 2 AS two ;
+ one
+-----
+ 1
+(1 row)
+
+NOTICE: warn 1.5
+CONTEXT: PL/pgSQL function warn(text) line 2 at RAISE
+ warn
+------
+ t
+(1 row)
+
+ two
+-----
+ 2
+(1 row)
+
+-- \gset applies to last query only
+SELECT 3 AS three \; SELECT warn('3.5') \; SELECT 4 AS four \gset
+ three
+-------
+ 3
+(1 row)
+
+NOTICE: warn 3.5
+CONTEXT: PL/pgSQL function warn(text) line 2 at RAISE
+ warn
+------
+ t
+(1 row)
+
+\echo :three :four
+:three 4
+-- syntax error stops all processing
+SELECT 5 \; SELECT 6 + \; SELECT warn('6.5') \; SELECT 7 ;
+ERROR: syntax error at or near ";"
+LINE 1: SELECT 5 ; SELECT 6 + ; SELECT warn('6.5') ; 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;
+-- misc SQL commands
+-- (non SELECT output is sent to stderr, thus is not shown in expected results)
+SELECT 'ok' AS "begin" \;
+CREATE TABLE psql_comics(s TEXT) \;
+INSERT INTO psql_comics VALUES ('Calvin'), ('hobbes') \;
+COPY psql_comics FROM STDIN \;
+UPDATE psql_comics SET s = 'Hobbes' WHERE s = 'hobbes' \;
+DELETE FROM psql_comics WHERE s = 'Moe' \;
+COPY psql_comics TO STDOUT \;
+TRUNCATE psql_comics \;
+DROP TABLE psql_comics \;
+SELECT 'ok' AS "done" ;
+ begin
+-------
+ ok
+(1 row)
+
+Calvin
+Susie
+Hobbes
+ done
+------
+ ok
+(1 row)
+
+DROP FUNCTION warn(TEXT);
diff --git a/src/test/regress/expected/transactions.out b/src/test/regress/expected/transactions.out
index 1b316cc9b8..3e62f83918 100644
--- a/src/test/regress/expected/transactions.out
+++ b/src/test/regress/expected/transactions.out
@@ -855,11 +855,21 @@ DROP TABLE abc;
-- tests rely on the fact that psql will not break SQL commands apart at a
-- backslash-quoted semicolon, but will send them as one Query.
create temp table i_table (f1 int);
--- psql will show only the last result in a multi-statement Query
-SELECT 1\; SELECT 2\; SELECT 3;
- ?column?
-----------
- 3
+-- psql will show all results of a multi-statement Query
+SELECT 1 AS one\; SELECT 2 AS two\; SELECT 3 AS three;
+ one
+-----
+ 1
+(1 row)
+
+ two
+-----
+ 2
+(1 row)
+
+ three
+-------
+ 3
(1 row)
-- this implicitly commits:
@@ -871,6 +881,12 @@ insert into i_table values(1)\; select * from i_table;
-- 1/0 error will cause rolling back the whole implicit transaction
insert into i_table values(2)\; select * from i_table\; select 1/0;
+ f1
+----
+ 1
+ 2
+(2 rows)
+
ERROR: division by zero
select * from i_table;
f1
@@ -889,9 +905,19 @@ rollback; -- we are not in a transaction at this point
WARNING: there is no transaction in progress
-- begin converts implicit transaction into a regular one that
-- can extend past the end of the Query
-select 1\; begin\; insert into i_table values(5);
+select 1 AS one\; begin\; insert into i_table values(5);
+ one
+-----
+ 1
+(1 row)
+
commit;
-select 1\; begin\; insert into i_table values(6);
+select 1 AS one\; begin\; insert into i_table values(6);
+ one
+-----
+ 1
+(1 row)
+
rollback;
-- commit in implicit-transaction state commits but issues a warning.
insert into i_table values(7)\; commit\; insert into i_table values(8)\; select 1/0;
@@ -917,23 +943,53 @@ select * from i_table;
rollback; -- we are not in a transaction at this point
WARNING: there is no transaction in progress
-- implicit transaction block is still a transaction block, for e.g. VACUUM
-SELECT 1\; VACUUM;
+SELECT 1 AS one\; VACUUM;
+ one
+-----
+ 1
+(1 row)
+
ERROR: VACUUM cannot run inside a transaction block
-SELECT 1\; COMMIT\; VACUUM;
+SELECT 2 AS two\; COMMIT\; VACUUM;
WARNING: there is no transaction in progress
+ two
+-----
+ 2
+(1 row)
+
ERROR: VACUUM cannot run inside a transaction block
-- we disallow savepoint-related commands in implicit-transaction state
-SELECT 1\; SAVEPOINT sp;
+SELECT 3 AS three\; SAVEPOINT sp;
+ three
+-------
+ 3
+(1 row)
+
ERROR: SAVEPOINT can only be used in transaction blocks
-SELECT 1\; COMMIT\; SAVEPOINT sp;
+SELECT 4 AS four\; COMMIT\; SAVEPOINT sp;
WARNING: there is no transaction in progress
+ four
+------
+ 4
+(1 row)
+
ERROR: SAVEPOINT can only be used in transaction blocks
-ROLLBACK TO SAVEPOINT sp\; SELECT 2;
+ROLLBACK TO SAVEPOINT sp\; SELECT 5 AS five;
ERROR: ROLLBACK TO SAVEPOINT can only be used in transaction blocks
-SELECT 2\; RELEASE SAVEPOINT sp\; SELECT 3;
+SELECT 6 AS six\; RELEASE SAVEPOINT sp\; SELECT 7 AS seven;
+ six
+-----
+ 6
+(1 row)
+
ERROR: RELEASE SAVEPOINT can only be used in transaction blocks
-- but this is OK, because the BEGIN converts it to a regular xact
-SELECT 1\; BEGIN\; SAVEPOINT sp\; ROLLBACK TO SAVEPOINT sp\; COMMIT;
+SELECT 8\; BEGIN\; SAVEPOINT sp\; ROLLBACK TO SAVEPOINT sp\; COMMIT;
+ ?column?
+----------
+ 8
+(1 row)
+
-- Test for successful cleanup of an aborted transaction at session exit.
-- THIS MUST BE THE LAST TEST IN THIS FILE.
begin;
diff --git a/src/test/regress/sql/copydml.sql b/src/test/regress/sql/copydml.sql
index 9a29f9c9ac..4578342253 100644
--- a/src/test/regress/sql/copydml.sql
+++ b/src/test/regress/sql/copydml.sql
@@ -70,10 +70,10 @@ drop rule qqq on copydml_test;
create function qqq_trig() returns trigger as $$
begin
if tg_op in ('INSERT', 'UPDATE') then
- raise notice '% %', tg_op, new.id;
+ raise notice '% % %', tg_when, tg_op, new.id;
return new;
else
- raise notice '% %', tg_op, old.id;
+ raise notice '% % %', tg_when, tg_op, old.id;
return old;
end if;
end
diff --git a/src/test/regress/sql/copyselect.sql b/src/test/regress/sql/copyselect.sql
index 1d98dad3c8..abc33904c0 100644
--- a/src/test/regress/sql/copyselect.sql
+++ b/src/test/regress/sql/copyselect.sql
@@ -84,7 +84,7 @@ drop table test1;
-- psql handling of COPY in multi-command strings
copy (select 1) to stdout\; select 1/0; -- row, then error
select 1/0\; copy (select 1) to stdout; -- error only
-copy (select 1) to stdout\; copy (select 2) to stdout\; select 0\; select 3; -- 1 2 3
+copy (select 1) to stdout\; copy (select 2) to stdout\; select 3\; select 4; -- 1 2 3 4
create table test3 (c int);
select 0\; copy test3 from stdin\; copy test3 from stdin\; select 1; -- 1
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index 2e37984962..4cf3d6eb9d 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -1134,3 +1134,38 @@ drop role regress_partitioning_role;
-- \d on toast table (use pg_statistic's toast table, which has a known name)
\d pg_toast.pg_toast_2619
+
+--
+-- combined queries
+--
+\echo '# combined queries tests'
+CREATE FUNCTION warn(msg TEXT) RETURNS BOOLEAN AS $$
+ BEGIN RAISE NOTICE 'warn %', msg ; RETURN TRUE ; END
+$$ LANGUAGE plpgsql;
+-- show both
+SELECT 1 AS one \; SELECT warn('1.5') \; SELECT 2 AS two ;
+-- \gset applies to last query only
+SELECT 3 AS three \; SELECT warn('3.5') \; SELECT 4 AS four \gset
+\echo :three :four
+-- syntax error stops all processing
+SELECT 5 \; SELECT 6 + \; SELECT warn('6.5') \; 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;
+-- misc SQL commands
+-- (non SELECT output is sent to stderr, thus is not shown in expected results)
+SELECT 'ok' AS "begin" \;
+CREATE TABLE psql_comics(s TEXT) \;
+INSERT INTO psql_comics VALUES ('Calvin'), ('hobbes') \;
+COPY psql_comics FROM STDIN \;
+UPDATE psql_comics SET s = 'Hobbes' WHERE s = 'hobbes' \;
+DELETE FROM psql_comics WHERE s = 'Moe' \;
+COPY psql_comics TO STDOUT \;
+TRUNCATE psql_comics \;
+DROP TABLE psql_comics \;
+SELECT 'ok' AS "done" ;
+Moe
+Susie
+\.
+DROP FUNCTION warn(TEXT);
diff --git a/src/test/regress/sql/transactions.sql b/src/test/regress/sql/transactions.sql
index 812e40a1a3..5d12fa8e86 100644
--- a/src/test/regress/sql/transactions.sql
+++ b/src/test/regress/sql/transactions.sql
@@ -489,8 +489,8 @@ DROP TABLE abc;
create temp table i_table (f1 int);
--- psql will show only the last result in a multi-statement Query
-SELECT 1\; SELECT 2\; SELECT 3;
+-- psql will show all results of a multi-statement Query
+SELECT 1 AS one\; SELECT 2 AS two\; SELECT 3 AS three;
-- this implicitly commits:
insert into i_table values(1)\; select * from i_table;
@@ -508,9 +508,9 @@ rollback; -- we are not in a transaction at this point
-- begin converts implicit transaction into a regular one that
-- can extend past the end of the Query
-select 1\; begin\; insert into i_table values(5);
+select 1 AS one\; begin\; insert into i_table values(5);
commit;
-select 1\; begin\; insert into i_table values(6);
+select 1 AS one\; begin\; insert into i_table values(6);
rollback;
-- commit in implicit-transaction state commits but issues a warning.
@@ -523,17 +523,17 @@ select * from i_table;
rollback; -- we are not in a transaction at this point
-- implicit transaction block is still a transaction block, for e.g. VACUUM
-SELECT 1\; VACUUM;
-SELECT 1\; COMMIT\; VACUUM;
+SELECT 1 AS one\; VACUUM;
+SELECT 2 AS two\; COMMIT\; VACUUM;
-- we disallow savepoint-related commands in implicit-transaction state
-SELECT 1\; SAVEPOINT sp;
-SELECT 1\; COMMIT\; SAVEPOINT sp;
-ROLLBACK TO SAVEPOINT sp\; SELECT 2;
-SELECT 2\; RELEASE SAVEPOINT sp\; SELECT 3;
+SELECT 3 AS three\; SAVEPOINT sp;
+SELECT 4 AS four\; COMMIT\; SAVEPOINT sp;
+ROLLBACK TO SAVEPOINT sp\; SELECT 5 AS five;
+SELECT 6 AS six\; RELEASE SAVEPOINT sp\; SELECT 7 AS seven;
-- but this is OK, because the BEGIN converts it to a regular xact
-SELECT 1\; BEGIN\; SAVEPOINT sp\; ROLLBACK TO SAVEPOINT sp\; COMMIT;
+SELECT 8\; BEGIN\; SAVEPOINT sp\; ROLLBACK TO SAVEPOINT sp\; COMMIT;
-- Test for successful cleanup of an aborted transaction at session exit.
