Making psql error out on output failures
Hi,
When exporting data with psql -c "..." >file or select ... \g file inside
psql,
post-creation output errors are silently ignored.
The problem can be seen easily by creating a small ramdisk and
filling it over capacity:
$ sudo mount -t tmpfs -o rw,size =1M tmpfs /mnt/ramdisk
$ psql -d postgres -At \
-c "select repeat('abc', 100)" > /mnt/ramdisk/file
$ echo $?
0
$ ls -l /mnt/ramdisk/file
-rw-r--r-- 1 daniel daniel 1048576 Dec 16 15:57 /mnt/ramdisk/file
$ df -h /mnt/ramdisk/
Filesystem Size Used Avail Use% Mounted on
tmpfs 1.0M 1.0M 0 100% /mnt/ramdisk
The output that should be 3M byte long is truncated as expected, but
we got no error message and no error code from psql, which
is obviously not nice.
The reason is that PrintQuery() and the code below it in
fe_utils/print.c call puts(), putc(), fprintf(),... without checking their
return values or the result of ferror() on the output stream.
If we made them do that and had the printing bail out at the first error,
that would be a painful change, since there are a lot of such calls:
$ egrep -w '(fprintf|fputs|fputc)' fe_utils/print.c | wc -l
326
and the call sites are in functions that have no error reporting paths
anyway.
To start the discussion, here's a minimal patch that checks ferror() in
PrintQueryTuples() to raise an error when the printing is done
(better late than never).
The error message is generic as opposed to using errno, as
I don't know whether errno has been clobbered at this point.
OTOH, I think that the error indicator on the output stream is not
cleared by successful writes after some other previous writes have failed.
Are there opinions on whether this should be addressed simply like
in the attached, or a large refactoring of print.c to bail out at the first
error would be better, or other ideas on how to proceed?
Best regards,
--
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 9c0d53689e..993484d092 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -718,6 +718,7 @@ static bool
PrintQueryTuples(const PGresult *results)
{
printQueryOpt my_popt = pset.popt;
+ bool result = true;
/* one-shot expanded output requested via \gx */
if (pset.g_expanded)
@@ -736,6 +737,12 @@ PrintQueryTuples(const PGresult *results)
printQuery(results, &my_popt, fout, false, pset.logfile);
+ if (ferror(fout))
+ {
+ pg_log_error("Error printing tuples");
+ result = false;
+ }
+
if (is_pipe)
{
pclose(fout);
@@ -745,9 +752,16 @@ PrintQueryTuples(const PGresult *results)
fclose(fout);
}
else
+ {
printQuery(results, &my_popt, pset.queryFout, false, pset.logfile);
+ if (ferror(pset.queryFout))
+ {
+ pg_log_error("Error printing tuples");
+ result = false;
+ }
+ }
- return true;
+ return result;
}
Re: Making psql error out on output failures
hi,
I did some further research on this bug. Here is the summary:
1. Tried to wrap "fputs" similar to "fprintf" redefined by "pg_fprintf",
but it ended up with too much error due to "fputs" is called everywhere
in "print_unaligned_text". If add an extra static variable to track the
output status, then it will be an overkill and potentially more bugs may
be introduced.
2. Investigated some other libraries such as libexplain
(http://libexplain.sourceforge.net/), but it definitely will introduce
the complexity.
3. I think a better way to resolve this issue will still be the solution
with an extra %m, which can make the error message much more informative
to the end users. The reason is that,
3.1 Providing the reasons for errors is required by PostgreSQL document,
https://www.postgresql.org/docs/12/error-style-guide.html
"Reasons for Errors
Messages should always state the reason why an error occurred. For
example:
BAD: could not open file %s
BETTER: could not open file %s (I/O failure)
If no reason is known you better fix the code."
3.2 Adding "%m" can provide a common and easy to understand reasons
crossing many operating systems. The "%m" fix has been tested on
platforms: CentOS 7, RedHat 7, Ubuntu 18.04, Windows 7/10, and macOS
Mojave 10.14, and it works.
3.3 If multiple errors happened after the root cause "No space left on
device", such as "No such file or directory" and "Permission denied",
then it make sense to report the latest one. The end users suppose to
know the latest error and solve it first. Eventually, "No space left on
device" error will be showing up.
3.4 Test log on different operating systems.
### CentOS 7
[postgres@localhost ~]$ uname -a
Linux localhost.localdomain 3.10.0-1062.9.1.el7.x86_64 #1 SMP Fri Dec 6
15:49:49 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
[postgres@localhost ~]$ sudo mount -t tmpfs -o rw,size=1M tmpfs /mnt/ramdisk
[postgres@localhost ~]$ df -h
tmpfs 1.0M 0 1.0M 0% /mnt/ramdisk
[postgres@localhost ~]$ psql
psql (12.2)
Type "help" for help.
postgres=# select repeat('111', 100) \g /mnt/ramdisk/file
Error printing tuples (No space left on device)
[postgres@localhost ~]$ psql -d postgres -At -c "select repeat('111',
100)" -o /mnt/ramdisk/file
Error printing tuples (No space left on device)
[postgres@localhost ~]$ psql -d postgres -At -c "select repeat('111',
100)" > /mnt/ramdisk/file
Error printing tuples (No space left on device)
### RedHat 7
[root@rh7 postgres]# uname -a
Linux rh7 3.10.0-1062.el7.x86_64 #1 SMP Thu Jul 18 20:25:13 UTC 2019
x86_64 x86_64 x86_64 GNU/Linux
[postgres@rh7 ~]$ sudo mkdir -p /mnt/ramdisk
We trust you have received the usual lecture from the local System
Administrator. It usually boils down to these three things:
#1) Respect the privacy of others.
#2) Think before you type.
#3) With great power comes great responsibility.
[sudo] password for postgres:
[postgres@rh7 ~]$ sudo mount -t tmpfs -o rw,size=1M tmpfs /mnt/ramdisk
[postgres@rh7 ~]$ psql -d postgres
psql (12.2)
Type "help" for help.
postgres=# select repeat('111', 100) \g /mnt/ramdisk/file
Error printing tuples (No space left on device)
[postgres@rh7 ~]$ psql -d postgres -At -c "select repeat('111',
100)" > /mnt/ramdisk/file
Error printing tuples (No space left on device)
[postgres@rh7 ~]$ psql -d postgres -At -c "select repeat('111',
100)" -o /mnt/ramdisk/file
Error printing tuples (No space left on device)
### Ubuntu 18.04
postgres=# select repeat('111', 1000) \g /mnt/ramdisk/file
Error printing tuples (No space left on device)
postgres=# \q
david@david-VM:~$ psql -d postgres -At -c "select repeat('111',
100)" > /mnt/ramdisk/file
Error printing tuples (No space left on device)
david@david-VM:~$ psql -d postgres -At -c "select repeat('111',
100)" -o /mnt/ramdisk/file
Error printing tuples (No space left on device)
### Windows 7
postgres=# select repeat('111', 100) \g E:/file
Error printing tuples (No space left on device)
postgres=# \q
C:\pg12.1\bin>psql -d postgres -U postgres -h 172.20.14.29 -At -c
"select repeat
('111', 10)" >> E:/file
Error printing tuples (No space left on device)
C:\pg12.1\bin>psql -d postgres -U postgres -h 172.20.14.29 -At -c
"select repeat
('111', 10)" -o E:/file
Error printing tuples (No space left on device)
### Windows 10
postgres=# select repeat('111', 100) \g E:/file
Error printing tuples (No space left on device)
postgres=# \q
C:\>psql -d postgres -U postgres -h 192.168.0.19 -At -c "select
repeat('111', 1000)" -o E:/file
Error printing tuples (No space left on device)
C:\>psql -d postgres -U postgres -h 192.168.0.19 -At -c "select
repeat('111', 200)" >> E:/file
Error printing tuples (No space left on device)
### macOS Mojave 10.14
postgres=# select repeat('111', 100) \g /Volumes/sdcard/file
Error printing tuples (No space left on device)
postgres=#
Re: Making psql error out on output failures
On 2020-Feb-18, David Zhang wrote:
> 3. I think a better way to resolve this issue will still be the solution
> with an extra %m, which can make the error message much more informative to
> the end users.
Yes, agreed. However, we use a style like this:
pg_log_error("could not print tuples: %m");
--
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Making psql error out on output failures
Hi Alvaro,
Thanks for your review, now the new patch with the error message in PG
style is attached.
On 2020-02-28 8:03 a.m., Alvaro Herrera wrote:
On 2020-Feb-18, David Zhang wrote:
3. I think a better way to resolve this issue will still be the solution
with an extra %m, which can make the error message much more informative to
the end users.
Yes, agreed. However, we use a style like this:
pg_log_error("could not print tuples: %m");
--
David
Software Engineer
Highgo Software Inc. (Canada)
www.highgo.ca
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 4b2679360f..9ffb1ef10f 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -855,6 +855,7 @@ static bool
PrintQueryTuples(const PGresult *results)
{
printQueryOpt my_popt = pset.popt;
+ bool result = true;
/* one-shot expanded output requested via \gx */
if (pset.g_expanded)
@@ -872,6 +873,11 @@ PrintQueryTuples(const PGresult *results)
disable_sigpipe_trap();
printQuery(results, &my_popt, fout, false, pset.logfile);
+ if (ferror(fout))
+ {
+ pg_log_error("could not print tuples: %m");
+ result = false;
+ }
if (is_pipe)
{
@@ -882,9 +888,16 @@ PrintQueryTuples(const PGresult *results)
fclose(fout);
}
else
+ {
printQuery(results, &my_popt, pset.queryFout, false,
pset.logfile);
+ if (ferror(pset.queryFout))
+ {
+ pg_log_error("could not print tuples: %m");
+ result = false;
+ }
+ }
- return true;
+ return result;
}
Re: Making psql error out on output failures
David Zhang wrote: > Thanks for your review, now the new patch with the error message in PG > style is attached. The current status of the patch is "Needs review" at https://commitfest.postgresql.org/27/2400/ If there's no more review to do, would you consider moving it to Ready for Committer? Best regards, -- Daniel Vérité PostgreSQL-powered mailer: http://www.manitou-mail.org Twitter: @DanielVerite
Re: Making psql error out on output failures
On 2020-03-06 10:36, Daniel Verite wrote: David Zhang wrote: Thanks for your review, now the new patch with the error message in PG style is attached. The current status of the patch is "Needs review" at https://commitfest.postgresql.org/27/2400/ If there's no more review to do, would you consider moving it to Ready for Committer? committed -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Making psql error out on output failures
Peter Eisentraut wrote: > > If there's no more review to do, would you consider moving it to > > Ready for Committer? > > committed Thanks! Best regards, -- Daniel Vérité PostgreSQL-powered mailer: http://www.manitou-mail.org Twitter: @DanielVerite
Re: Making psql error out on output failures
Hi Daniel,
I agree with you if psql output doesn't indicate any error when the disk is
full, then it is obviously not nice. In some situations, people may end up lost
data permanently.
However, after I quickly applied your idea/patch to "commit
bf65f3c8871bcc95a3b4d5bcb5409d3df05c8273 (HEAD -> REL_12_STABLE,
origin/REL_12_STABLE)", and I found the behaviours/results are different.
Here is the steps and output,
$ sudo mkdir -p /mnt/ramdisk
$ sudo mount -t tmpfs -o rw,size=1M tmpfs /mnt/ramdisk
Test-1: delete the "file", and run psql command from a terminal directly,
$ rm /mnt/ramdisk/file
$ psql -d postgres -At -c "select repeat('111', 100)" > /mnt/ramdisk/file
Error printing tuples
then dump the file,
$ rm /mnt/ramdisk/file
$ hexdump -C /mnt/ramdisk/file
31 31 31 31 31 31 31 31 31 31 31 31 31 31 31 31 ||
*
0010
Test-2: delete the "file", run the command within psql console,
$ rm /mnt/ramdisk/file
$ psql -d postgres
psql (12.1)
Type "help" for help.
postgres=# select repeat('111', 100) \g /mnt/ramdisk/file
Error printing tuples
postgres=#
Then dump the file again,
$ hexdump -C /mnt/ramdisk/file
20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 ||
*
0010
As you can see the content are different after applied the patch.
David
The new status of this patch is: Waiting on Author
Re: Making psql error out on output failures
David Z wrote:
> $ psql -d postgres -At -c "select repeat('111', 100)" >
> /mnt/ramdisk/file
The -A option selects the unaligned output format and -t
switches to the "tuples only" mode (no header, no footer).
> Test-2: delete the "file", run the command within psql console,
> $ rm /mnt/ramdisk/file
> $ psql -d postgres
In this invocation there's no -A and -t, so the beginning of the
output is going to be a left padded column name that is not in the
other output.
The patch is not involved in that difference.
Best regards,
--
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite
Re: Making psql error out on output failures
Right, the file difference is caused by "-At".
On the other side, in order to keep the output message more consistent with
other tools, I did a litter bit more investigation on pg_dump to see how it
handles this situation. Here is my findings.
pg_dump using WRITE_ERROR_EXIT to throw the error message when "(bytes_written
!= size * nmemb)", where WRITE_ERROR_EXIT calls fatal("could not write to
output file: %m") and then "pg_log_generic(PG_LOG_ERROR, __VA_ARGS__)". After
ran a quick test in the same situation, I got message like below,
$ pg_dump -h localhost -p 5432 -d postgres -t psql_error -f /mnt/ramdisk/file
pg_dump: error: could not write to output file: No space left on device
If I change the error log message like below, where "%m" is used to pass the
value of strerror(errno), "could not write to output file:" is copied from
function "WRITE_ERROR_EXIT".
- pg_log_error("Error printing tuples");
+ pg_log_error("could not write to output file: %m");
then the output message is something like below, which, I believe, is more
consistent with pg_dump.
$ psql -d postgres -t -c "select repeat('111', 100)" -o /mnt/ramdisk/file
could not write to output file: No space left on device
$ psql -d postgres -t -c "select repeat('111', 100)" > /mnt/ramdisk/file
could not write to output file: No space left on device
Hope the information will help.
David
---
Highgo Software Inc. (Canada)
www.highgo.ca
Re: Making psql error out on output failures
David Zhang wrote:
> If I change the error log message like below, where "%m" is used to pass the
> value of strerror(errno), "could not write to output file:" is copied from
> function "WRITE_ERROR_EXIT".
> - pg_log_error("Error printing tuples");
> + pg_log_error("could not write to output file: %m");
> then the output message is something like below, which, I believe, is more
> consistent with pg_dump.
The problem is that errno may not be reliable to tell us what was
the problem that leads to ferror(fout) being nonzero, since it isn't
saved at the point of the error and the output code may have called
many libc functions between the first occurrence of the output error
and when pg_log_error() is called.
The linux manpage on errno warns specifically about this:
NOTES
A common mistake is to do
if (somecall() == -1) {
printf("somecall() failed\n");
if (errno == ...) { ... }
}
where errno no longer needs to have the value it had upon return
from somecall()
(i.e., it may have been changed by the printf(3)). If the value of
errno should be
preserved across a library call, it must be saved:
This other bit from the POSIX spec [1] is relevant:
"The value of errno shall be defined only after a call to a function
for which it is explicitly stated to be set and until it is changed
by the next function call or if the application assigns it a value."
To use errno in a way that complies with the above, the psql code
should be refactored. I don't know if having a more precise error
message justifies that refactoring. I've elaborated a bit about that
upthread with the initial submission. Besides, I'm not even
sure that errno is necessarily set on non-POSIX platforms when fputc
or fputs fails.
That's why this patch uses the safer approach to emit a generic
error message.
[1] https://pubs.opengroup.org/onlinepubs/9699919799/functions/errno.html
Best regards,
--
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite
Re: Making psql error out on output failures
On 2020-01-16 5:20 a.m., Daniel Verite wrote:
David Zhang wrote:
If I change the error log message like below, where "%m" is used to pass the
value of strerror(errno), "could not write to output file:" is copied from
function "WRITE_ERROR_EXIT".
- pg_log_error("Error printing tuples");
+ pg_log_error("could not write to output file: %m");
then the output message is something like below, which, I believe, is more
consistent with pg_dump.
The problem is that errno may not be reliable to tell us what was
the problem that leads to ferror(fout) being nonzero, since it isn't
saved at the point of the error and the output code may have called
many libc functions between the first occurrence of the output error
and when pg_log_error() is called.
The linux manpage on errno warns specifically about this:
NOTES
A common mistake is to do
if (somecall() == -1) {
printf("somecall() failed\n");
if (errno == ...) { ... }
}
where errno no longer needs to have the value it had upon return
from somecall()
(i.e., it may have been changed by the printf(3)). If the value of
errno should be
preserved across a library call, it must be saved:
This other bit from the POSIX spec [1] is relevant:
"The value of errno shall be defined only after a call to a function
for which it is explicitly stated to be set and until it is changed
by the next function call or if the application assigns it a value."
To use errno in a way that complies with the above, the psql code
should be refactored. I don't know if having a more precise error
message justifies that refactoring. I've elaborated a bit about that
upthread with the initial submission.
Yes, I agree with you. For case 2 "select repeat('111', 100) \g
/mnt/ramdisk/file", it can be easily fixed with more accurate error
message similar to pg_dump, one example could be something like below.
But for case 1 "psql -d postgres -At -c "select repeat('111', 100)"
> /mnt/ramdisk/file" , it may require a lot of refactoring work.
diff --git a/src/port/snprintf.c b/src/port/snprintf.c
index 8fd997553e..e6c239fd9f 100644
--- a/src/port/snprintf.c
+++ b/src/port/snprintf.c
@@ -309,8 +309,10 @@ flushbuffer(PrintfTarget *target)
written = fwrite(target->bufstart, 1, nc, target->stream);
target->nchars += written;
- if (written != nc)
+ if (written != nc) {
target->failed = true;
+ fprintf(stderr, "could not write to output file:
%s\n", strerror(errno));
+ }
Besides, I'm not even
sure that errno is necessarily set on non-POSIX platforms when fputc
or fputs fails.
Verified, fputs does set the errno at least in Ubuntu Linux.
That's why this patch uses the safer approach to emit a generic
error message.
[1] https://pubs.opengroup.org/onlinepubs/9699919799/functions/errno.html
Best regards,
--
David
Software Engineer
Highgo Software Inc. (Canada)
www.highgo.ca
Re: Making psql error out on output failures
David Zhang wrote:
> Yes, I agree with you. For case 2 "select repeat('111', 100) \g
> /mnt/ramdisk/file", it can be easily fixed with more accurate error
> message similar to pg_dump, one example could be something like below.
> But for case 1 "psql -d postgres -At -c "select repeat('111', 100)"
> > /mnt/ramdisk/file" , it may require a lot of refactoring work.
I don't quite see why you make that distinction? The relevant bits of
code are common, it's all the code in src/fe_utils/print.c called
from PrintQuery().
Best regards,
--
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite
Re: Making psql error out on output failures
On 2020-01-20 2:42 a.m., Daniel Verite wrote:
David Zhang wrote:
Yes, I agree with you. For case 2 "select repeat('111', 100) \g
/mnt/ramdisk/file", it can be easily fixed with more accurate error
message similar to pg_dump, one example could be something like below.
But for case 1 "psql -d postgres -At -c "select repeat('111', 100)"
/mnt/ramdisk/file" , it may require a lot of refactoring work.
I don't quite see why you make that distinction? The relevant bits of
code are common, it's all the code in src/fe_utils/print.c called
from PrintQuery().
The reason is that, within PrintQuery() function call, one case goes to
print_unaligned_text(), and the other case goes to print_aligned_text().
The error "No space left on device" can be logged by fprintf() which is
redefined as pg_fprintf() when print_aligned_text() is called, however
the original c fputs function is used directly when
print_unaligned_text() is called, and it is used everywhere.
Will that be a better solution if redefine fputs similar to fprintf and
log the exact error when first time discovered? The concern is that if
we can't provide a more accurate information to the end user when error
happens, sometimes the end user might got even confused.
Best regards,
--
David
Software Engineer
Highgo Software Inc. (Canada)
www.highgo.ca
Re: Making psql error out on output failures
David Zhang wrote: > The error "No space left on device" can be logged by fprintf() which is > redefined as pg_fprintf() when print_aligned_text() is called Are you sure? I don't find that redefinition. Besides print_aligned_text() also calls putc and puts. > Will that be a better solution if redefine fputs similar to fprintf and > log the exact error when first time discovered? I think we can assume it's not acceptable to have pg_fprintf() to print anything to stderr, or to set a flag through a global variable. So even if using pg_fprintf() for all the printing, no matter how (through #defines or otherwise), there's still the problem that the error needs to be propagated up the call chain to be processed by psql. > The concern is that if we can't provide a more accurate > information to the end user when error happens, sometimes the > end user might got even confused. It's better to have a more informative message, but I'm for not having the best be the enemy of the good. The first concern is that at the moment, we have no error report at all in the case when the output can be opened but the error happens later along the writes. Best regards, -- Daniel Vérité PostgreSQL-powered mailer: http://www.manitou-mail.org Twitter: @DanielVerite
Re: Making psql error out on output failures
On 2020-01-28 4:14 a.m., Daniel Verite wrote:
David Zhang wrote:
The error "No space left on device" can be logged by fprintf() which is
redefined as pg_fprintf() when print_aligned_text() is called
Are you sure? I don't find that redefinition. Besides
print_aligned_text() also calls putc and puts.
Yes, below is the gdb debug message when psql first time detects the
error "No space left on device". Test case, "postgres=# select
repeat('111', 100) \g /mnt/ramdisk/file"
bt
#0 flushbuffer (target=0x7ffd6a709ad0) at snprintf.c:313
#1 0x55ba90b88b6c in dopr_outchmulti (c=32, slen=447325,
target=0x7ffd6a709ad0) at snprintf.c:1435
#2 0x55ba90b88d5e in trailing_pad (padlen=-147,
target=0x7ffd6a709ad0) at snprintf.c:1514
#3 0x55ba90b87f36 in fmtstr (value=0x55ba90bb4f9a "", leftjust=1,
minlen=147, maxwidth=0, pointflag=0, target=0x7ffd6a709ad0) at
snprintf.c:994
#4 0x55ba90b873c6 in dopr (target=0x7ffd6a709ad0,
format=0x55ba90bb5083 "%s%-*s", args=0x7ffd6a709f40) at snprintf.c:675
#5 0x55ba90b865b5 in pg_vfprintf (stream=0x55ba910cf240,
fmt=0x55ba90bb507f "%-*s%s%-*s", args=0x7ffd6a709f40) at snprintf.c:257
#6 0x55ba90b866aa in pg_fprintf (stream=0x55ba910cf240,
fmt=0x55ba90bb507f "%-*s%s%-*s") at snprintf.c:270
#7 0x55ba90b75d22 in print_aligned_text (cont=0x7ffd6a70a210,
fout=0x55ba910cf240, is_pager=false) at print.c:937
#8 0x55ba90b7ba19 in printTable (cont=0x7ffd6a70a210,
fout=0x55ba910cf240, is_pager=false, flog=0x0) at print.c:3378
#9 0x55ba90b7bedc in printQuery (result=0x55ba910f9860,
opt=0x7ffd6a70a2c0, fout=0x55ba910cf240, is_pager=false, flog=0x0) at
print.c:3496
#10 0x55ba90b39560 in PrintQueryTuples (results=0x55ba910f9860) at
common.c:874
#11 0x55ba90b39d55 in PrintQueryResults (results=0x55ba910f9860) at
common.c:1262
#12 0x55ba90b3a343 in SendQuery (query=0x55ba910f2590 "select
repeat('111', 100) ") at common.c:1446
#13 0x55ba90b51f36 in MainLoop (source=0x7f1623a9ea00
<_IO_2_1_stdin_>) at mainloop.c:505
#14 0x55ba90b5c4da in main (argc=3, argv=0x7ffd6a70a738) at
startup.c:445
(gdb) l
308 size_t written;
309
310 written = fwrite(target->bufstart, 1, nc,
target->stream);
311 target->nchars += written;
312 if (written != nc)
313 target->failed = true;
314 }
315 target->bufptr = target->bufstart;
316 }
317
(gdb) p written
$2 = 1023
(gdb) p nc
$3 = 1024
(gdb) p strerror(errno)
$4 = 0x7f16238672c9 "No space left on device"
(gdb)
Will that be a better solution if redefine fputs similar to fprintf and
log the exact error when first time discovered?
I think we can assume it's not acceptable to have pg_fprintf()
to print anything to stderr, or to set a flag through a global
variable. So even if using pg_fprintf() for all the printing, no matter
how (through #defines or otherwise), there's still the problem that the
error needs to be propagated up the call chain to be processed by psql.
The concern is that if we can't provide a more accurate
information to the end user when error happens, sometimes the
end user might got even confused.
It's better to have a more informative message, but I'm for
not having the best be the enemy of the good.
The first concern is that at the moment, we have no error
report at all in the case when the output can be opened
but the error happens later along the writes.
Best regards,
--
David
Software Engineer
Highgo Software Inc. (Canada)
www.highgo.ca
Re: Making psql error out on output failures
David Zhang wrote:
> > Are you sure? I don't find that redefinition. Besides
> > print_aligned_text() also calls putc and puts.
> Yes, below is the gdb debug message when psql first time detects the
> error "No space left on device". Test case, "postgres=# select
> repeat('111', 100) \g /mnt/ramdisk/file"
> bt
> #0 flushbuffer (target=0x7ffd6a709ad0) at snprintf.c:313
Indeed. For some reason gdb won't let me step into these fprintf()
calls, but you're right they're redefined (through include/port.h):
#define vsnprintf pg_vsnprintf
#define snprintfpg_snprintf
#define vsprintfpg_vsprintf
#define sprintf pg_sprintf
#define vfprintfpg_vfprintf
#define fprintf pg_fprintf
#define vprintf pg_vprintf
#define printf(...) pg_printf(__VA_ARGS__)
Anyway, I don't see it leading to an actionable way to reliably keep
errno, as discussed upthread.
Best regards,
--
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite
