Message-ID: <63350f81-6a43-f536-24e6-b30f48797732@enterprisedb.com>
Date: Tue, 22 Mar 2022 15:52:21 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.7.0
Subject: Re: psql - add SHOW_ALL_RESULTS option
Content-Language: en-US
To: Fabien COELHO <coelho@cri.ensmp.fr>
Cc: "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
References: <18c78a01-4a34-9dd4-f78b-6860f1420c8e@enterprisedb.com>
 <alpine.DEB.2.22.394.2107221655020.25741@pseudo>
 <6954328d-96f2-77f7-735f-7ce493a40949@enterprisedb.com>
 <alpine.DEB.2.22.394.2109251058000.3216312@pseudo>
 <alpine.DEB.2.22.394.2110021622530.2817726@pseudo>
 <2570e2ae-fa0f-aac9-f72f-bb59a9983a20@enterprisedb.com>
 <alpine.DEB.2.22.394.2112230703530.2668598@pseudo>
 <46259d13-5f90-1475-3c85-d7a597a2b862@enterprisedb.com>
 <alpine.DEB.2.22.394.2201040851220.3857182@pseudo>
 <alpine.DEB.2.22.394.2201081931170.731324@pseudo>
 <20220113054433.wdjhidtm7gvyhfts@alap3.anarazel.de>
 <alpine.DEB.2.22.394.2201150938470.3378943@pseudo>
 <805c0496-d60d-802b-3d61-73fed38dea16@enterprisedb.com>
 <alpine.DEB.2.22.394.2203041443080.538147@pseudo>
 <alpine.DEB.2.22.394.2203121726590.3679190@pseudo>
 <f54c68a7-a8ac-d538-ec7c-ee7e090fefd8@enterprisedb.com>
 <alpine.DEB.2.22.394.2203171859410.1198186@pseudo>
From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
In-Reply-To: <alpine.DEB.2.22.394.2203171859410.1198186@pseudo>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/63350f81-6a43-f536-24e6-b30f48797732%40enterprisedb.com>
Precedence: bulk

On 17.03.22 19:04, Fabien COELHO wrote:
> 
> Hello Peter,
> 
>>> See attached v16 which removes the libpq workaround.
>>
>> I suppose this depends on
>>
>> https://www.postgresql.org/message-id/flat/ab4288f8-be5c-57fb-2400-e3e857f53e46%40enterprisedb.com 
>>
>>
>> getting committed, because right now this makes the psql TAP tests 
>> fail because of the duplicate error message.
>>
>> How should we handle that?
> 
> Ok, it seems I got the patch wrong.
> 
> Attached v17 is another try. The point is to record the current status, 
> whatever it is, buggy or not, and to update the test when libpq fixes 
> things, whenever this is done.

Your patch contains this test case:

+# Test voluntary crash
+my ($ret, $out, $err) = $node->psql(
+   'postgres',
+   "SELECT 'before' AS running;\n" .
+   "SELECT pg_terminate_backend(pg_backend_pid());\n" .
+   "SELECT 'AFTER' AS not_running;\n");
+
+is($ret, 2, "server stopped");
+like($out, qr/before/, "output before crash");
+ok($out !~ qr/AFTER/, "no output after crash");
+is($err, 'psql:<stdin>:2: FATAL:  terminating connection due to 
administrator command
+psql:<stdin>:2: FATAL:  terminating connection due to administrator command
+server closed the connection unexpectedly
+   This probably means the server terminated abnormally
+   before or while processing the request.
+psql:<stdin>:2: fatal: connection to server was lost', "expected error 
message");

The expected output (which passes) contains this line twice:

psql:<stdin>:2: FATAL:  terminating connection due to administrator command
psql:<stdin>:2: FATAL:  terminating connection due to administrator command

If I paste this test case into current master without your patch, I only 
get this line once.  So your patch is changing this output.  The whole 
point of the libpq fixes was to not have this duplicate output.  So I 
think something is still wrong somewhere.