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

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

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-16T17:33:51Z
Lists: pgsql-bugs, pgsql-hackers
I wrote:
> Are we sufficiently convinced that we must have the dont-allow-partial
> option to not fix this in the back branches?  I'm not.

I just had a thought about that: suppose we add a flag to CopyState
to indicate whether we reached EOF while reading.  This wouldn't be
hugely expensive, just something like

    switch (cstate->copy_dest)
    {
        case COPY_FILE:
            bytesread = fread(databuf, 1, maxread, cstate->copy_file);
            if (ferror(cstate->copy_file))
                ereport(ERROR,
                        (errcode_for_file_access(),
                         errmsg("could not read from COPY file: %m")));
+           cstate->reached_eof |= (bytesread < maxread);
            break;
        case COPY_OLD_FE:

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.

There are race-ish conditions in cases like the called program
getting an unrelated SIGPIPE at about the same time that we decide
to stop reading early, but I don't think it's really possible
to disambiguate that.

			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.