On 01.11.22 21:30, Peter Eisentraut wrote:
>>> There are some places where the return value is apparently intentionally
>>> ignored, such as in error recovery paths, or psql ignoring a failure to
>>> launch the pager. (The intention can usually be inferred by the kind of
>>> error checking attached to the corresponding popen() call.) But
>>> there are a
>>> few places in psql that I'm suspicious about that I have marked, but
>>> need to
>>> think about further.
>>
>> Hmm. I would leave these out, I think. setQFout() relies on the
>> result of openQueryOutputFile(). And this could make commands like
>> \watch less reliable.
>
> I don't quite understand what you are saying here. My point is that,
> for example, setQFout() thinks it's important to check the result of
> popen() and write an error message, but it doesn't check the result of
> pclose() at all. I don't think that makes sense in practice.
I have looked this over again. In these cases, if the piped-to command
is faulty, you get a "broken pipe" error anyway, so the effect of not
checking the pclose() result is negligible. So I have removed the
"FIXME" markers without further action.
There is also the question whether we want to check the exit status of a
user-supplied command, such as in pgbench's \setshell. I have dialed
back my patch there, since I don't know what the current practice in
pgbench scripts is. If the command fails badly, pgbench will probably
complain anyway about invalid output.
More important is that something like pg_upgrade does check the exit
status when it calls pg_controldata etc. That's what this patch
accomplishes.
