(2018/11/17 8:10), Tom Lane wrote:
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.)
So I think this version is probably good, although maybe it could
use an additional comment explaining the above reasoning.
I agree that it's better to keep the BeginCopyFrom API as-is. Also, I
think your version would handle SIGPIPE in COPY FROM PROGRAM more
properly than what I proposed. So, +1 from me.
Thanks!
Best regards,
Etsuro Fujita
