psql lacking clearerr()

Started by Alvaro Herreraalmost 5 years ago4 messages
#1Alvaro Herrera
alvherre@alvh.no-ip.org
1 attachment(s)

psql seems to never call clearerr() on its output file. So if it gets
an error while printing a result, it'll show

could not print result table: Success

after each and every result, even though the output file isn't in error
state anymore.

It seems that the simplest fix is just to do clearerr() at the start of
printTable(), as in the attached.

I haven't been able to find a good reproducer. Sometimes doing C-s C-c
does it, but I'm not sure it is fully reproducible.

--
�lvaro Herrera Valdivia, Chile

Attachments:

miss-clearerr.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/fe_utils/print.c b/src/fe_utils/print.c
index e8772a278c..0d0b5ef782 100644
--- a/src/fe_utils/print.c
+++ b/src/fe_utils/print.c
@@ -3322,6 +3322,9 @@ printTable(const printTableContent *cont,
 	if (cont->opt->format == PRINT_NOTHING)
 		return;
 
+	/* Clear any prior error indicator */
+	clearerr(fout);
+
 	/* print_aligned_*() handle the pager themselves */
 	if (!is_pager &&
 		cont->opt->format != PRINT_ALIGNED &&
#2Kyotaro Horiguchi
horikyota.ntt@gmail.com
In reply to: Alvaro Herrera (#1)
Re: psql lacking clearerr()

At Wed, 24 Mar 2021 11:11:41 -0300, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote in

psql seems to never call clearerr() on its output file. So if it gets
an error while printing a result, it'll show

could not print result table: Success

after each and every result, even though the output file isn't in error
state anymore.

It seems that the simplest fix is just to do clearerr() at the start of
printTable(), as in the attached.

I haven't been able to find a good reproducer. Sometimes doing C-s C-c
does it, but I'm not sure it is fully reproducible.

That worked for me:p And the following steps always raises that error.

postgres=# select 1; (just to let it into history).
postgres=# C-s -> C-p -> C-m -> C-c
postgres=# select 1;
...
could not print result table: Success

And actually the patch works and the location looks like appropriate.

By the way, I think errno is not set when f* functions fail so anyway
isn't %m in the messages is useless?

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

#3Alvaro Herrera
alvherre@alvh.no-ip.org
In reply to: Kyotaro Horiguchi (#2)
Re: psql lacking clearerr()

On 2021-Mar-25, Kyotaro Horiguchi wrote:

That worked for me:p And the following steps always raises that error.

postgres=# select 1; (just to let it into history).
postgres=# C-s -> C-p -> C-m -> C-c
postgres=# select 1;
...
could not print result table: Success

Ah, thanks! Indeed this reliably reproduces the issue. I was very
surprised to find out that this bug is new in pg13. But then I bisected
it down to this commit:

commit b03436994bcc4909dd644fd5ae6d9a9acdf30da5
Author: Peter Eisentraut <peter@eisentraut.org>
AuthorDate: Fri Mar 20 16:04:15 2020 +0100
CommitDate: Fri Mar 20 16:04:15 2020 +0100

psql: Catch and report errors while printing result table

Errors (for example I/O errors or disk full) while printing out result
tables were completely ignored, which could result in silently
truncated output in scripts, for example. Fix by adding some basic
error checking and reporting.

Author: Daniel Verite <daniel@manitou-mail.org>
Author: David Zhang <david.zhang@highgo.ca>
Discussion: /messages/by-id/9a0b3c8d-ee14-4b1d-9d0a-2c993bdabacc@manitou-mail.org

which is where this message was added. So it turns out that this has
*always* been a problem ... we just didn't know.

Due to lack of complaints, I'm inclined to apply this only back to pg13.

(And, yes, I'm to remove the %m too, because clearly that was a mistake.)

--
�lvaro Herrera 39�49'30"S 73�17'W
"Right now the sectors on the hard disk run clockwise, but I heard a rumor that
you can squeeze 0.2% more throughput by running them counterclockwise.
It's worth the effort. Recommended." (Gerry Pourwelle)

#4Alvaro Herrera
alvherre@alvh.no-ip.org
In reply to: Alvaro Herrera (#3)
Re: psql lacking clearerr()

On 2021-Mar-29, Alvaro Herrera wrote:

(And, yes, I'm to remove the %m too, because clearly that was a mistake.)

Re-reading the other thread, I think the %m should stay.

--
�lvaro Herrera Valdivia, Chile
"I think my standards have lowered enough that now I think 'good design'
is when the page doesn't irritate the living f*ck out of me." (JWZ)