Horton

Re: psql - add SHOW_ALL_RESULTS option

Michael Paquier <michael@paquier.xyz>

View thread View on postgr.es
From: Michael Paquier <michael@paquier.xyz>
To: Julien Rouhaud <rjuju123@gmail.com>
Cc: Fabien COELHO <coelho@cri.ensmp.fr>, "shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>, "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
Date: 2021-04-09T01:06:24Z
Lists: pgsql-hackers 
On Fri, Apr 09, 2021 at 01:11:35AM +0800, Julien Rouhaud wrote:
> On Thu, Apr 08, 2021 at 07:04:01PM +0200, Fabien COELHO wrote:
>> It is definitely a open item. I'm not sure where you want to add it…
>> possibly the "Pg 14 Open Items" wiki page?
> 
> Correct.

I was running a long query this morning and wondered why the
cancellation was suddenly broken.  So I am not alone, and here you are
with already a solution :)

So, studying through 3a51306, this stuff has changed the query
execution from a sync PQexec() to an async PQsendQuery().  And the
proposed fix changes back to the behavior where the cancellation
reset happens after getting a result, as there is no need to cancel
anything.

No strong objections from here if the consensus is to make
SendQueryAndProcessResults() handle the cancel reset properly though I
am not sure if this is the cleanest way to do things, but let's make
at least the whole business consistent in the code for all those code
paths.  For example, PSQLexecWatch() does an extra ResetCancelConn()
that would be useless once we are done with
SendQueryAndProcessResults().  Also, I can see that
SendQueryAndProcessResults() would not issue a cancel reset if the
query fails, for \watch when cancel is pressed, and for \watch with
COPY.  So, my opinion here would be to keep ResetCancelConn() within
PSQLexecWatch(), just add an extra one in SendQuery() to make all the
three code paths printing results consistent, and leave
SendQueryAndProcessResults() out of the cancellation logic.

>> I tried but I do not have enough
>> privileges, if you can do it please proceed. I added an entry in the next CF
>> in the bugfix section.
> 
> That's strange, I don't think you need special permission there.  It's
> working for me so I added an item with a link to the patch!

As long as you have a community account, you should have the
possibility to edit the page.  So if you feel that any change is
required, please feel free to do so, of course.
--
Michael

Commits

  1. psql: Show all query results by default

  2. psql: Refactor ProcessResult()

  3. psql: Refactor SendQuery()

  4. psql: Add test for psql behavior on server crash

  5. psql: Additional tests

  6. Improve some psql test code

  7. psql: Fix some scan-build warnings

  8. psql: Add test for handling of replication commands

  9. psql: More tests

  10. psql: Add various tests

  11. psql: Add test for query canceling

  12. Revert "psql: Show all query results by default"

  13. Extend a test case a little