On Fri, Jul 29, 2016 at 11:58 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Maybe the real problem here is that the abstraction layer is so incomplete
> and apparently haphazard, so that it's not very obvious where you should
> use a pq_xxx name and where you should refer to socket_xxx. For some of
> the functions in pqcomm.c, such as internal_flush, it's impossible to tell
> which side of the abstraction boundary they're supposed to be on.
> (I suspect that that particular function has simply been ignored on the
> undocumented assumption that a bgworker could never call it, but that's
> the kind of half-baked work that I don't find acceptable.)
>
> I think the core work that needs to be done here is to clearly identify
> each function in pqcomm.c as either above or below the PqCommMethods
> abstraction boundary. Maybe we should split one set or the other out
> to a new source file. In any case, the naming convention ought to
> reflect that hierarchy.
>
> I withdraw the idea that we should try to revert all these functions back
> to their old names, since that would obscure the layering distinction
> between socket-specific and general-usage functions. I now think that
> the problem is that that distinction hasn't been drawn sharply enough.
If memory serves, there was some discussion of this at the time the
patch that changed this was originally submitted. The original patch,
IIRC, just installed one or two hooks and it was argued that I should
instead create some kind of abstraction layer. The present coding is
the result of my attempt to do just that. I have to admit that I
wasn't very eager to churn the contents of this file more than
necessary. It seems like old, crufty code to me. I don't object if
you want to refactor it, but I'm not sure what problem it solves.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
