Horton

Re: BUG #15449: file_fdw using program cause exit code error when using LIMIT

Tom Lane <tgl@sss.pgh.pa.us>

View thread View on postgr.es
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
Cc: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>, thomas.munro@enterprisedb.com, eric.cyr@gmail.com, pgsql-hackers@lists.postgresql.org
Date: 2018-11-16T23:10:41Z
Lists: pgsql-bugs, pgsql-hackers 
I wrote:
> I'm not quite sure whether the reached_eof test should be
> "if (bytesread == 0)" or "if (bytesread < maxread)".  The former
> seems more certain to indicate real EOF; are there other cases where
> the fread might return a short read?  On the other hand, if we
> support in-band EOF indicators (\. for instance) then we might
> stop without having made an extra call to CopyGetData that would
> see bytesread == 0.  On the third hand, if we do do that it's
> not quite clear to me whether SIGPIPE is ignorable or not.

After still further thought, it seems like "if (bytesread == 0)"
is the right way to proceed.  That protects us against incorrectly
setting reached_eof if the pipe is being run in unbuffered or
line-buffered mode.  (Which copy.c doesn't do, at the moment,
but I'd just as soon this code didn't need to assume that.
Also, I'm not 100% convinced that Windows or other platforms
might not act that way.)  In the case where we terminate early
because we saw an in-band EOF marker, we would also not set reached_eof,
and again that seems like a good outcome: if we see SIGPIPE it
must mean that the program kept sending data after the EOF marker,
and that seems like a good thing to complain about.  (It's only
guaranteed to fail if the program sends more than the current pipe
buffer-ful, but I don't feel a need to add extra code to check for
nonempty buffer contents as we exit.)

So I think this version is probably good, although maybe it could
use an additional comment explaining the above reasoning.

			regards, tom lane

Commits

  1. Handle EPIPE more sanely when we close a pipe reading from a program.

  2. Avoid defining SIGTTIN/SIGTTOU on Windows.

  3. Leave SIGTTIN/SIGTTOU signal handling alone in postmaster child processes.

  4. Remove some unnecessary pqsignal() calls to shave a few cycles off

  5. Fix handling of SIGCHLD, per recent pghackers discussion: on some

  6. The new files for statistical system views.

  7. Remove fork()/exec() and only do fork(). Small cleanups.