I wrote:
> I just had a thought about that: suppose we add a flag to CopyState
> to indicate whether we reached EOF while reading. ...
> Then the logic in ClosePipeToProgram could be "if we did not reach
> EOF, then a SIGPIPE termination is unsurprising. If we did reach
> EOF, then SIGPIPE is an error." If the called program gets SIGPIPE
> for some reason other than us closing the pipe early, then we would
> see EOF next time we try to read, and correctly report that there's
> a problem.
Concretely, something like the attached.
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.
regards, tom lane
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 6588ebd..8975446 100644
*** a/src/backend/commands/copy.c
--- b/src/backend/commands/copy.c
*************** typedef struct CopyStateData
*** 114,120 ****
FILE *copy_file; /* used if copy_dest == COPY_FILE */
StringInfo fe_msgbuf; /* used for all dests during COPY TO, only for
* dest == COPY_NEW_FE in COPY FROM */
! bool fe_eof; /* true if detected end of copy data */
EolType eol_type; /* EOL type of input */
int file_encoding; /* file or remote side's character encoding */
bool need_transcoding; /* file encoding diff from server? */
--- 114,122 ----
FILE *copy_file; /* used if copy_dest == COPY_FILE */
StringInfo fe_msgbuf; /* used for all dests during COPY TO, only for
* dest == COPY_NEW_FE in COPY FROM */
! bool is_copy_from; /* COPY TO, or COPY FROM? */
! bool reached_eof; /* true if we read to end of copy data (not
! * all copy_dest types maintain this) */
EolType eol_type; /* EOL type of input */
int file_encoding; /* file or remote side's character encoding */
bool need_transcoding; /* file encoding diff from server? */
*************** CopyGetData(CopyState cstate, void *data
*** 575,580 ****
--- 577,584 ----
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not read from COPY file: %m")));
+ if (bytesread == 0)
+ cstate->reached_eof = true;
break;
case COPY_OLD_FE:
*************** CopyGetData(CopyState cstate, void *data
*** 595,601 ****
bytesread = minread;
break;
case COPY_NEW_FE:
! while (maxread > 0 && bytesread < minread && !cstate->fe_eof)
{
int avail;
--- 599,605 ----
bytesread = minread;
break;
case COPY_NEW_FE:
! while (maxread > 0 && bytesread < minread && !cstate->reached_eof)
{
int avail;
*************** CopyGetData(CopyState cstate, void *data
*** 623,629 ****
break;
case 'c': /* CopyDone */
/* COPY IN correctly terminated by frontend */
! cstate->fe_eof = true;
return bytesread;
case 'f': /* CopyFail */
ereport(ERROR,
--- 627,633 ----
break;
case 'c': /* CopyDone */
/* COPY IN correctly terminated by frontend */
! cstate->reached_eof = true;
return bytesread;
case 'f': /* CopyFail */
ereport(ERROR,
*************** ProcessCopyOptions(ParseState *pstate,
*** 1050,1055 ****
--- 1054,1061 ----
if (cstate == NULL)
cstate = (CopyStateData *) palloc0(sizeof(CopyStateData));
+ cstate->is_copy_from = is_from;
+
cstate->file_encoding = -1;
/* Extract options from the statement node tree */
*************** ClosePipeToProgram(CopyState cstate)
*** 1727,1737 ****
--- 1733,1755 ----
(errcode_for_file_access(),
errmsg("could not close pipe to external command: %m")));
else if (pclose_rc != 0)
+ {
+ /*
+ * If we ended a COPY FROM PROGRAM before reaching EOF, then it's
+ * expectable for the called program to fail with SIGPIPE, and we
+ * should not report that as an error. Otherwise, SIGPIPE indicates a
+ * problem.
+ */
+ if (cstate->is_copy_from && !cstate->reached_eof &&
+ WIFSIGNALED(pclose_rc) && WTERMSIG(pclose_rc) == SIGPIPE)
+ return;
+
ereport(ERROR,
(errcode(ERRCODE_EXTERNAL_ROUTINE_EXCEPTION),
errmsg("program \"%s\" failed",
cstate->filename),
errdetail_internal("%s", wait_result_to_str(pclose_rc))));
+ }
}
/*
*************** BeginCopyFrom(ParseState *pstate,
*** 3169,3175 ****
oldcontext = MemoryContextSwitchTo(cstate->copycontext);
/* Initialize state variables */
! cstate->fe_eof = false;
cstate->eol_type = EOL_UNKNOWN;
cstate->cur_relname = RelationGetRelationName(cstate->rel);
cstate->cur_lineno = 0;
--- 3187,3193 ----
oldcontext = MemoryContextSwitchTo(cstate->copycontext);
/* Initialize state variables */
! cstate->reached_eof = false;
cstate->eol_type = EOL_UNKNOWN;
cstate->cur_relname = RelationGetRelationName(cstate->rel);
cstate->cur_lineno = 0;
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 2d75773..6bc0244 100644
*** a/src/backend/storage/file/fd.c
--- b/src/backend/storage/file/fd.c
*************** OpenTransientFilePerm(const char *fileNa
*** 2271,2281 ****
--- 2271,2287 ----
* Routines that want to initiate a pipe stream should use OpenPipeStream
* rather than plain popen(). This lets fd.c deal with freeing FDs if
* necessary. When done, call ClosePipeStream rather than pclose.
+ *
+ * This function also ensures that the popen'd program is run with default
+ * SIGPIPE and SIGUSR2 processing, rather than the SIG_IGN settings the
+ * backend normally uses. This ensures desirable response to, eg, closing
+ * a read pipe early.
*/
FILE *
OpenPipeStream(const char *command, const char *mode)
{
FILE *file;
+ int save_errno;
DO_DB(elog(LOG, "OpenPipeStream: Allocated %d (%s)",
numAllocatedDescs, command));
*************** OpenPipeStream(const char *command, cons
*** 2293,2300 ****
TryAgain:
fflush(stdout);
fflush(stderr);
errno = 0;
! if ((file = popen(command, mode)) != NULL)
{
AllocateDesc *desc = &allocatedDescs[numAllocatedDescs];
--- 2299,2313 ----
TryAgain:
fflush(stdout);
fflush(stderr);
+ pqsignal(SIGPIPE, SIG_DFL);
+ pqsignal(SIGUSR2, SIG_DFL);
errno = 0;
! file = popen(command, mode);
! save_errno = errno;
! pqsignal(SIGPIPE, SIG_IGN);
! pqsignal(SIGUSR2, SIG_IGN);
! errno = save_errno;
! if (file != NULL)
{
AllocateDesc *desc = &allocatedDescs[numAllocatedDescs];
*************** TryAgain:
*** 2307,2314 ****
if (errno == EMFILE || errno == ENFILE)
{
- int save_errno = errno;
-
ereport(LOG,
(errcode(ERRCODE_INSUFFICIENT_RESOURCES),
errmsg("out of file descriptors: %m; release and retry")));
--- 2320,2325 ----
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index a3b9757..cfa416a 100644
*** a/src/backend/tcop/postgres.c
--- b/src/backend/tcop/postgres.c
*************** PostgresMain(int argc, char *argv[],
*** 3769,3774 ****
--- 3769,3778 ----
* handler in the postmaster to reserve the signal. (Of course, this isn't
* an issue for signals that are locally generated, such as SIGALRM and
* SIGPIPE.)
+ *
+ * Also note: signals that are set to SIG_IGN here should be reset in
+ * OpenPipeStream, so that exec'd programs see a standard signal
+ * environment.
*/
if (am_walsender)
WalSndSignals();
