Re: backtrace_on_internal_error

Jelte Fennema-Nio <postgres@jeltef.nl>

From: Jelte Fennema-Nio <postgres@jeltef.nl>
To: Andres Freund <andres@anarazel.de>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Peter Eisentraut <peter@eisentraut.org>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2023-12-20T13:49:53Z
Lists: pgsql-hackers
On Wed, 20 Dec 2023 at 11:30, Andres Freund <andres@anarazel.de> wrote:
> Tom's patch imo doesn't really introduce anything really new - we already deal
> with EOF that way in other places. And it's how the standard APIs deal with
> the issue. I'd not design it this way on a green field, but given the current
> state Tom's approach seems more sensible...

Okay, while I think it's a really non-obvious way of checking for EOF,
I agree that staying consistent with this non-obvious existing pattern
is the best choice here. I also just noticed that the proposed patch
is already merged.

So I just updated my patchset to use it. For my patchset this does
introduce a slight problem though: I'm using pqReadData, instead of
pqsecure_read directly. And pqReadData has other reasons for failing
without setting an errno than just EOF. Specifically allocation
failures or passing an invalid socket.

I see three options to handle this:
1. Don't change pqReadData and simply consider all these EOF too from
PQcancelPoll
2. Set errno to something non-zero for these non EOF failures in pqReadData
3. Return -2 from pqReadData on EOF

Any preference on those? For now I went for option 1.



Commits

  1. Add GUC backtrace_on_internal_error

  2. Fix variable name and comment

  3. Be more wary about OpenSSL not setting errno on error.