Horton

Re: psql - add SHOW_ALL_RESULTS option

Fabien COELHO <coelho@cri.ensmp.fr>

View thread View on postgr.es
From: Fabien COELHO <coelho@cri.ensmp.fr>
To: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Cc: "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
Date: 2022-03-23T12:58:36Z
Lists: pgsql-hackers 
Hello Peter,

>> 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.
>
> [...]
>
> 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.

Hmmm. Yes and no:-)

The previous path inside libpq silently ignores intermediate results, it 
skips all results to keep only the last one. The new approach does not 
discard resultss silently, hence the duplicated output, because they are 
actually there and have always been there in the first place, they were 
just ignored: The previous "good" result is really a side effect of a bad 
implementation in a corner case, which just becomes apparent when opening 
the list of results.

So my opinion is still to dissociate the libpq "bug/behavior" fix from 
this feature, as they are only loosely connected, because it is a very 
corner case anyway.

An alternative would be to remove the test case, but I'd prefer that it is 
kept.

If you want to wait for libpq to provide a solution for this corner case, 
I'm afraid that "never" is the likely result, especially as no test case 
exercices this path to show that there is a problem somewhere, so nobody 
should care to fix it. I'm not sure it is even worth it given the highly 
special situation which triggers the issue, which is not such an actual 
problem (ok, the user is told twice that there was a connection loss, no 
big deal).

-- 
Fabien.

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