On Thu, Mar 3, 2016 at 7:15 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
> On 2/5/16 5:04 AM, Peter Geoghegan wrote:
>> As Heikki goes into on that thread, the appropriate action seems to be
>> to constantly reset the error queue, and to make sure that we
>> ourselves clear the queue consistently. (Note that we might not have
>> consistently called ERR_get_error() in the event of an OOM within
>> SSLerrmessage(), for example). I have not changed backend code in the
>> patch, though; I felt that we had enough control of the general
>> situation there for it to be unnecessary to lock everything down.
>
> I think clearing the error after a call is not necessary. The API
> clearly requires that you should clear the error queue before a call, so
> clearing it afterwards does not accomplish anything, except maybe make
> broken code work sometimes, for a while. Also, there is nothing that
> says that an error produces exactly one entry in the error queue; it
> could be multiple. Or that errors couldn't arise at random times
> between the reset and whatever happens next.
>
> I think this is analogous to clearing errno before a C library call.
> You could clear it afterwards as well, to be nice to the next guy, but
> the next guy should really take care of that themselves, and we can't
> rely on what happens in between anyway.
>
> The places that you identified for change look correct as far as libpq
> goes. I do think that the backend should be updated in the same way,
> because it's a) correct, b) easy enough, and c) there could well be
> interactions with postgres_fdw, plproxy, plperl, or who knows what.
So what's the next step here? Peter G, are you planning to update the
patch based on this review from Peter E? If not, Peter E, do you want
to update the patch and commit? If neither, I'm going to mark this
Returned with Feedback in the CF and move on, which seems a bit of a
shame since this appears to be a bona fide bug, but if nobody's
willing to work on it, it ain't gettin' fixed.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company