Re: [HACKERS] [PATCH] Add SIGCHLD catch to psql

[Stephen Frost]

* Tom Lane (t...@sss.pgh.pa.us) wrote:
> Stephen Frost <sfr...@snowman.net> writes:
> > Attached is a patch that just checks the result from the existing
> > fflush() inside the FETCH_COUNT loop and drops out of that loop if we
> > get an error from it.
> 
> I thought it might be about that simple once you went at it the right
> way ;-).  However, I'd suggest checking ferror(pset.queryFout) as well
> as the fflush result.  It's not clear to me whether fflush should be
> counted on to report an error that actually occurred in a previous
> fwrite.  (It's also unclear why fflush isn't documented to set the stream
> error indicator on failure, but it isn't.)

Sure, I can add the ferror() check.  Patch attached.

My man page (Debian/Linux) has this to say about fflush():

DESCRIPTION
       The function fflush() forces a write of all user-space buffered
           data for the given output or update stream via the stream’s
           underlying write function.  The open status of the stream
       is unaffected.

       If the stream argument is NULL, fflush() flushes all open output
           streams.

       For a non-locking counterpart, see unlocked_stdio(3).

RETURN VALUE
       Upon successful completion 0 is returned.  Otherwise, EOF is
           returned and the global variable errno is set to indicate the
           error.

ERRORS
       EBADF  Stream is not an open stream, or is not open for writing.

       The function fflush() may also fail and set errno for any of the
           errors specified for the routine write(2).

CONFORMING TO
       C89, C99.

                Thanks,

                        Stephen

diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index f605c97..25340f6 100644
*** a/src/bin/psql/common.c
--- b/src/bin/psql/common.c
*************** ExecQueryUsingCursor(const char *query, 
*** 982,987 ****
--- 982,988 ----
  	char		fetch_cmd[64];
  	instr_time	before,
  				after;
+ 	int			flush_error;
  
  	*elapsed_msec = 0;
  
*************** ExecQueryUsingCursor(const char *query, 
*** 1098,1106 ****
  
  		/*
  		 * Make sure to flush the output stream, so intermediate results are
! 		 * visible to the client immediately.
  		 */
! 		fflush(pset.queryFout);
  
  		/* after the first result set, disallow header decoration */
  		my_popt.topt.start_table = false;
--- 1099,1109 ----
  
  		/*
  		 * Make sure to flush the output stream, so intermediate results are
! 		 * visible to the client immediately.  We check the results because
! 		 * if the pager dies/exits/etc, there's no sense throwing more data
! 		 * at it.
  		 */
! 		flush_error = fflush(pset.queryFout);
  
  		/* after the first result set, disallow header decoration */
  		my_popt.topt.start_table = false;
*************** ExecQueryUsingCursor(const char *query, 
*** 1108,1114 ****
  
  		PQclear(results);
  
! 		if (ntuples < pset.fetch_count || cancel_pressed)
  			break;
  	}
  
--- 1111,1124 ----
  
  		PQclear(results);
  
! 		/* Check if we are at the end, if a cancel was pressed, or if
! 		 * there were any errors either trying to flush out the results,
! 		 * or more generally on the output stream at all.  If we hit any
! 		 * errors writing things to the stream, we presume $PAGER has
! 		 * disappeared and stop bothring to pull down more data.
! 		 */
! 		if (ntuples < pset.fetch_count || cancel_pressed || flush_error ||
! 			ferror(pset.queryFout))
  			break;
  	}
  

signature.asc
Description: Digital signature