public inbox for [email protected]  
help / color / mirror / Atom feed
From: Bossart, Nathan <[email protected]>
To: Tom Lane <[email protected]>
To: Fabien COELHO <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Julien Rouhaud <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Subject: Re: psql - add SHOW_ALL_RESULTS option
Date: Mon, 12 Apr 2021 19:08:21 +0000
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<alpine.DEB.2.22.394.2104062323150.2555247@pseudo>
	<OSZPR01MB631017521EE6887ADC9492E8FD759@OSZPR01MB6310.jpnprd01.prod.outlook.com>
	<alpine.DEB.2.22.394.2104071516470.2724014@pseudo>
	<OSZPR01MB6310325023E8B4109BB2AB90FD749@OSZPR01MB6310.jpnprd01.prod.outlook.com>
	<20210408163050.fca7dwb37zszbsmm@nol>
	<alpine.DEB.2.22.394.2104081858500.3192110@pseudo>
	<20210408171135.ekhdf5nvqss554q3@nol>
	<YG+okFpc3JaKN/[email protected]>
	<alpine.DEB.2.22.394.2104090828330.3253600@pseudo>
	<[email protected]>
	<alpine.DEB.2.22.394.2104091947240.3439700@pseudo>
	<[email protected]>
	<alpine.DEB.2.22.394.2104121156200.244363@pseudo>
	<[email protected]>

On 4/12/21, 9:25 AM, "Tom Lane" <[email protected]> wrote:
> Fabien COELHO <[email protected]> writes:
>>> Between this and the known breakage of control-C, it seems clear
>>> to me that this patch was nowhere near ready for prime time.
>>> I think shoving it in on the last day before feature freeze was
>>> ill-advised, and it ought to be reverted.  We can try again later.
>
>> The patch has been asleep for quite a while, and was resurrected, possibly
>> too late in the process. ISTM that fixing it for 14 is manageable,
>> but this is not my call.
>
> I just observed an additional issue that I assume was introduced by this
> patch, which is that psql's response to a server crash has gotten
> repetitive:
>
> regression=# CREATE VIEW v1(c1) AS (SELECT ('4' COLLATE "C")::INT FROM generate_series(1, 10));
> server closed the connection unexpectedly
>         This probably means the server terminated abnormally
>         before or while processing the request.
> The connection to the server was lost. Attempting reset: Failed.
> The connection to the server was lost. Attempting reset: Failed.
> !?>
>
> I've never seen that before, and it's not because I don't see
> server crashes regularly.

I think I've found another issue with this patch.  If AcceptResult()
returns false in SendQueryAndProcessResults(), it seems to result in
an infinite loop of "unexpected PQresultStatus" messages.  This can be
reproduced by trying to run "START_REPLICATION" via psql.

The following patch seems to resolve the issue, although I'll admit I
haven't dug into this too deeply.  In any case, +1 for reverting the
patch for now.

diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 028a357991..abafd41763 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -1176,7 +1176,7 @@ SendQueryAndProcessResults(const char *query, double *pelapsed_msec, bool is_wat

             /* and switch to next result */
             result = PQgetResult(pset.db);
-            continue;
+            break;
         }

         /* must handle COPY before changing the current result */

Nathan



view thread (104+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: psql - add SHOW_ALL_RESULTS option
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox