Subject: Re: psql - add SHOW_ALL_RESULTS option
To: Fabien COELHO <coelho@cri.ensmp.fr>
Cc: "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
References: <69C0B369-570C-4524-8EE4-BCCACECB6BEE@amazon.com>
 <20210412193301.GA11204@alvherre.pgsql> <YHeVHH0wVcRwdogL@paquier.xyz>
 <3738387.1618451507@sss.pgh.pa.us>
 <alpine.DEB.2.22.394.2104151336060.191178@pseudo>
 <451a2111-3757-c511-9413-8f8af531a167@enterprisedb.com>
 <alpine.DEB.2.22.394.2106121130400.4105466@pseudo>
 <CALDaNm0+pzGgp-M1mAoUVYRtjadbWYuB96J8RaV9iH3+HA8FTA@mail.gmail.com>
 <alpine.DEB.2.22.394.2107151742300.1705285@pseudo>
 <18c78a01-4a34-9dd4-f78b-6860f1420c8e@enterprisedb.com>
 <alpine.DEB.2.22.394.2107221655020.25741@pseudo>
From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Message-ID: <6954328d-96f2-77f7-735f-7ce493a40949@enterprisedb.com>
Date: Fri, 24 Sep 2021 14:42:33 +0200
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0)
 Gecko/20100101 Thunderbird/78.13.0
MIME-Version: 1.0
In-Reply-To: <alpine.DEB.2.22.394.2107221655020.25741@pseudo>
Content-Type: multipart/mixed;
 boundary="------------66AEBF40A0BC935DAADB3134"
Content-Language: en-US
Archived-At: 
 <https://www.postgresql.org/message-id/6954328d-96f2-77f7-735f-7ce493a40949%40enterprisedb.com>
Precedence: bulk

This is a multi-part message in MIME format.
--------------66AEBF40A0BC935DAADB3134
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 8bit

On 22.07.21 16:58, Fabien COELHO wrote:
>>> Ok. I noticed. The patch got significantly broken by the watch pager 
>>> commit. I also have to enhance the added tests (per Peter request).
>>
>> I wrote a test to check psql query cancel support.  I checked that it 
>> fails against the patch that was reverted.  Maybe this is useful.
> 
> Here is the updated version (v8? I'm not sure what the right count is), 
> which works for me and for "make check", including some tests added for 
> uncovered paths.

I was looking at adding test coverage for the issue complained about in 
[0].  That message said that the autocommit logic was broken, but 
actually the issue was with the ON_ERROR_ROLLBACK logic.  However, it 
turned out that neither feature had any test coverage, and they are 
easily testable using the pg_regress setup, so I wrote tests for both 
and another little thing I found nearby.

It turns out that your v8 patch still has the issue complained about in 
[0].  The issue is that after COMMIT AND CHAIN, the internal savepoint 
is gone, but the patched psql still thinks it should be there and tries 
to release it, which leads to errors.


[0]: https://www.postgresql.org/message-id/2671235.1618154047@sss.pgh.pa.us

--------------66AEBF40A0BC935DAADB3134
Content-Type: text/plain; charset=UTF-8; x-mac-type="0"; x-mac-creator="0";
 name="0001-psql-Add-various-tests.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="0001-psql-Add-various-tests.patch"