public inbox for [email protected]
help / color / mirror / Atom feedFrom: Michael Paquier <[email protected]>
To: Julien Rouhaud <[email protected]>
Cc: Fabien COELHO <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Subject: Re: psql - add SHOW_ALL_RESULTS option
Date: Fri, 9 Apr 2021 10:06:24 +0900
Message-ID: <YG+okFpc3JaKN/[email protected]> (raw)
In-Reply-To: <20210408171135.ekhdf5nvqss554q3@nol>
References: <[email protected]>
<alpine.DEB.2.22.394.2103141046590.164573@pseudo>
<[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>
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
Attachments:
[application/pgp-signature] signature.asc (833B, ../YG+okFpc3JaKN%[email protected]/2-signature.asc)
download
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]
Subject: Re: psql - add SHOW_ALL_RESULTS option
In-Reply-To: <YG+okFpc3JaKN/[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