public inbox for [email protected]
help / color / mirror / Atom feedFrom: Peter Eisentraut <[email protected]>
To: Fabien COELHO <[email protected]>
Cc: [email protected] <[email protected]>
Subject: Re: psql - add SHOW_ALL_RESULTS option
Date: Fri, 8 Oct 2021 14:15:31 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <alpine.DEB.2.22.394.2110021622530.2817726@pseudo>
References: <[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<alpine.DEB.2.22.394.2104151336060.191178@pseudo>
<[email protected]>
<alpine.DEB.2.22.394.2106121130400.4105466@pseudo>
<CALDaNm0+pzGgp-M1mAoUVYRtjadbWYuB96J8RaV9iH3+HA8FTA@mail.gmail.com>
<alpine.DEB.2.22.394.2107151742300.1705285@pseudo>
<[email protected]>
<alpine.DEB.2.22.394.2107221655020.25741@pseudo>
<[email protected]>
<alpine.DEB.2.22.394.2109251058000.3216312@pseudo>
<alpine.DEB.2.22.394.2110021622530.2817726@pseudo>
On 02.10.21 16:31, Fabien COELHO wrote:
>> Attached v9 integrates your tests and makes them work.
>
> Attached v11 is a rebase.
This patch still has a few of the problems reported earlier this year.
In [0], it was reported that certain replication commands result in
infinite loops because of faulty error handling. This still happens. I
wrote a test for it, attached here. (I threw in a few more basic tests,
just to have some more coverage that was lacking, and to have a file to
put the new test in.)
In [1], it was reported that server crashes produce duplicate error
messages. This also still happens. I didn't write a test for it, maybe
you have an idea. (Obviously, we could check whether the error message
is literally there twice in the output, but that doesn't seem very
general.) But it's easy to test manually: just have psql connect, shut
down the server, then run a query.
Additionally, I looked into the Coverity issue reported in [2]. That
one is fixed, but I figured it would be good to be able to check your
patches with a static analyzer in a similar way. I don't have the
ability to run Coverity locally, so I looked at scan-build and fixed a
few minor warnings, also attached as a patch. Your current patch
appears to be okay in that regard.
[0]:
https://www.postgresql.org/message-id/[email protected]
[1]: https://www.postgresql.org/message-id/[email protected]
[2]: https://www.postgresql.org/message-id/[email protected]
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]
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