Message-ID: <5BF23697.6020806@lab.ntt.co.jp>
Date: Mon, 19 Nov 2018 13:05:43 +0900
From: Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64;
 rv:11.0) Gecko/20120327 Thunderbird/11.0.1
MIME-Version: 1.0
Subject: Re: BUG #15449: file_fdw using program cause exit code error when
 using LIMIT
References: <5BE4318F.4040002@lab.ntt.co.jp>
 <20181109.143931.243136889.horiguchi.kyotaro@lab.ntt.co.jp>
 <5BE552ED.4040304@lab.ntt.co.jp>
 <20181112.185214.76711162.horiguchi.kyotaro@lab.ntt.co.jp>
 <5BE966F4.1040702@lab.ntt.co.jp> <5BEA9E82.1080709@lab.ntt.co.jp>
 <30744.1542388851@sss.pgh.pa.us> <31355.1542389631@sss.pgh.pa.us>
 <7865.1542407597@sss.pgh.pa.us> <9660.1542409841@sss.pgh.pa.us>
In-Reply-To: <9660.1542409841@sss.pgh.pa.us>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>,
        thomas.munro@enterprisedb.com, eric.cyr@gmail.com,
        pgsql-hackers@lists.postgresql.org
Precedence: bulk

(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