Re: PGEventProcs must not be allowed to break libpq
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Date: 2022-02-16T18:11:50Z
Lists: pgsql-hackers
Attachments
- 0001-make-events-pure-observers-of-PGresults.patch (text/x-diff) patch 0001
- 0002-make-events-pure-observers-of-PQreset.patch (text/x-diff) patch 0002
I wrote: > ... more generally, it seems to me that allowing a failing PGEventProc > to cause this to happen is just not sane. It breaks absolutely > every guarantee you might think we have about how libpq will behave. > As an example that seems very plausible currently, if an event proc > doesn't know what a PGRES_PIPELINE_SYNC result is and fails on it, > will the application see behavior that's even a little bit sane? > I don't think so --- it will think the error results are server > failures, and then be very confused when answers arrive anyway. Attached are two proposed patches addressing this. The first one turns RESULTCREATE and RESULTCOPY events into pure observers, ie failure of an event procedure doesn't affect the overall processing of a PGresult. I think this is necessary if we want to be able to reason at all about how libpq behaves. Event procedures do still have the option to report failure out to the application in some out-of-band way, such as via their passThrough argument. But they can't break what libpq itself does. The second patch turns CONNRESET events into pure observers. While I'm slightly less hot about making that change, the existing behavior seems very poorly thought-out, not to mention untested. Notably, the code there changes conn->status to CONNECTION_BAD without closing the socket, which is unlike any other post-connection failure path; so I wonder just how well that'd work if it were exercised in anger. Comments, objections? regards, tom lane
Commits
-
Don't let libpq PGEVT_CONNRESET callbacks break a PGconn.
- 2e372869aa38 15.0 landed
-
Don't let libpq "event" procs break the state of PGresult objects.
- ce1e7a2f7169 15.0 landed