(2018/11/07 9:22), Kyotaro HORIGUCHI wrote:
At Tue, 06 Nov 2018 21:07:37 +0900, Etsuro Fujita<fujita.ets...@lab.ntt.co.jp> wrote
in<5be18409.2070...@lab.ntt.co.jp>
(2018/11/06 12:53), Kyotaro HORIGUCHI wrote:
At Fri, 02 Nov 2018 22:05:36 +0900, Etsuro
Fujita<fujita.ets...@lab.ntt.co.jp> wrote
in<5bdc4ba0.7050...@lab.ntt.co.jp>
(2018/10/29 15:58), Kyotaro HORIGUCHI wrote:
But iff we are explicitly stop reading from
the pipe before detecting an error, it can be ignored since we
aren't interested in further failure.
You mean that we should ignore other failures of the called program
when we stop reading from the pipe early?
Yes, we have received sufficient data from the pipe then closed
it successfully. The program may want write more but we don't
want it. We ragher should ignore SIGPIPE exit code of the program
rather
since closing our writing end of a pipe is likely to cause it and
I think so too.
even if it comes from another pipe, we can assume that the
SIGPIPE immediately stopped the program before returning any
garbage to us.
Sorry, I don't understand this.
Mmm. It looks confusing, sorry. In other words:
We don't care the reason for program's ending if we received
enough data from the program and we actively (= before receiving
EOF) stop reading. On the other hand if SIGPIPE (on another pipe)
or something fatal happened on the program before we end reading,
we receive EOF just after that and we are interested in the cause
of the program's death.
# Does the above make sense?
Yeah, thanks for the explanation!
In the sense of "We don't care the reason", negligible reasons
are necessariry restricted to SIGPIPE, evan SIGSEGV could be
theoretically ignored safely. "theoretically" here means it is
another issue whether we rely on the output from a program which
causes SEGV (or any reason other than SIGPIPE, which we caused).
For the SIGSEGV case, I think it would be better that we don't rely on
the output data, IMO, because I think there might be a possibility that
the program have generated that data incorrectly/unexpectedly.
Finally in the attached PoC, I set cstate->eof_reached on failure
of NextCopyFromRawFields then if eof_reached we don't ingore
SIGPIPE. For a program like
"puts("test1");fflush(stdout);kill(getpid(), SIGPIPE);", I had
the following behavior. (I got an error without the fflush() but
it is inevitable).
=# select * from ft2 limit 1;
a
-------
test1
=# select * from ft2 limit 2;
ERROR: program "/home/horiguti/work/exprog" failed
DETAIL: child process was terminated by signal 13: Broken pipe
For the original case:
=# select * from ft1 limit 1;
a
------
test
=# select * from ft1 limit 2;
a
------
test
(1 row)
I didn't confirmed whether it is complete.
Sorry, I don't understand this fully, but the reason to add the
eof_reached stuff is to avoid ignoring the SIGPIPE failure in normal
COPY FROM PROGRAM cases?
Yes, if we have received EOF, the program ended before we
finished reading from the pipe thus any error should be
reported. Elsewise we don't care at least SIGPIPE. In the
attached patch all kind of errors are ignored when pipe is closed
from reading-end on backends.
OK, I understand your idea. Thanks for the patch!
As the result it doesn't report an error for SELECT * FROM ft2
LIMIT 1 on "main(void){puts("test1"); return 1;}".
=# select * from ft limit 1;
a
-------
test1
(1 row)
limit 2 reports the error.
=# select * from ft limit 2;
ERROR: program "/home/horiguti/work/exprog" failed
DETAIL: child process exited with exit code 1
I think this would be contrary to users expectations: if the SELECT
command works for limit 1, they would expect that the command would work
for limit 2 as well. So, I think it would be better to error out that
command for limit 1 as well, as-is.
Best regards,
Etsuro Fujita
