Fabien COELHO <coe...@cri.ensmp.fr> writes: > Fixing the problem envolves deciding what is the right behavior, and > update the documentation and the implementation accordingly. Currently the > documentation suggests that :ERROR is about SQL error (subject to > interpretation), and the implementation is more or less consistent with > that view, but not always, as pointed out by Daniel.
FWIW, I think we should adopt the rule that :ERROR reflects any error condition, whether server-side or client-side. It's unlikely that scripts would really not care about client-side errors. Moreover, I do not think we can reliably tell the difference; there are always going to be corner cases that are hard to classify. Example: if we lose the server connection, is that a server-side error or client-side? Or maybe neither, maybe the network went down. Because of this concern, I find the idea of inventing separate SQL_ERROR and CLIENT_ERROR variables to be a pretty terrible one. I think we'd be creating a lot of make-work and hard choices for ourselves, to do something that most script writers won't give a fig about. If you've got subtle error-handling logic in mind, you shouldn't be trying to implement your app in a psql script in the first place, IMO anyway. It's unclear to me whether to push ahead with Daniel's existing patch or not. It doesn't look to me like it's making things any worse from the error-consistency standpoint than they were already, so I'd be inclined to consider error semantics cleanup as something to be done separately/later. BTW, it looks to me like the last patch is still not right as far as when to close copystream --- won't it incorrectly close a stream obtained from pset.copyStream? While you could fix that with - if (pset.gfname && copystream != NULL) + if (!pset.copyStream && pset.gfname && copystream != NULL) I think that's getting overly complex and unmaintainable. I'd be inclined to code things more like FILE *copystream = NULL; bool need_close = false; ... need_close = openQueryOutputFile(...); ... if (need_close) // close copystream here regards, tom lane