Re: libpq patch for pqtypes hook api and PGresult creation - Mailing list pgsql-patches

Kind of a long post, but if you take the time to read it we think it accurately
clarifies how we interrupt the current objections and how we see this working.
NOTE: any references to overhead are in regards to library size, not performance.

 >would be to insert hooks at library
 >_init() time, meaning that the mere linking of libpgtypes

Alvaro Herrera wrote:
"Maybe there's a way we can have libpqtypes adding calls into some
hypothetical libpq hooks.  So libpqtypes registers its hooks in _init()
or some such, and it gets picked up automatically by any app that links
to it."

 >the "hook name" concept
Not needed anymore if we do per-conn hooks.  I was doing library wide hooks, it
felt natural to allow them to be removed (ability not needed per-conn).  You can
only remove hooks if you have a means of referencing what you want to remove.
 From that perspective, the names served a purpose - PQremoveObjectHooks("myhook");

 >you've got it holding a process-wide lock
Okay, easy change to install per-conn. I was trying to avoid having to set these
hooks on every connection.

There are some dirty details in regards to locking.  Even if you remove the
locking from libpq hooks, you still incur a lock at every hook point inside
pqtypes.  pqtypes has to map a conn and result (we'll call this a pqobj) to
pqtypes typeData. Adding a void* to the hook funcs doesn't help because non-hook
functions like getf, paramcreate, etc. only have a ptr to a pqobj: PQgetf(res,
..), PQparamCreate(conn, ..).  Since the private storage of a pqobj is not
directly accessible, you have to either A) map pqobj addresses to typeData in a
pqtypes global array that must be locked or B) add two libpq API calls
PQtypeData(conn), PQresultTypeData(res).

 > libpgtypes calls "PQinitTypes(PGconn *conn)"
As this stands, it wouldn't work.  You need some hook funcptr arguments. Without
them, there is no way to communicate with pqtypes.

Tom Lane wrote:
"hooks that could be used by either libpgtypes or something that would like to
do something roughly similar"

I don't think PQinitTypes, private type data per conn/result or the need for
PQtypeData(conn), PQresultTypeData(res) (to avoid locking in pqtypes) keeps
things in line with this requirement (generic hook api).  Has this requirement
changed?  BTW, Tom was not the only one to suggest generic design.  That's why I
came up with object hooks - notifications of libpq object states.  Best
name/concept I can come up with.  PQinitTypes(conn) is really
PQaddObjectHook(conn, hook) -- addition of the conn argument -- to keep it generic.

In the end, the problem is that the wrong tool "hooks" is being used for
pqtypes.  Hooks normally allow a completely unrelated library to receive events.
  I think we are forcing a hook design on to something that is completely
"related" (libpqtypes is an extension of libpq, getvalue and getf are siblings).
  There is no need for hooks.  If we wanted to add
PQsetBillingMethod(PQconn*,PQbillingMethod*), then you could make a case for
hooks (obviously the billing api doesn't fit).  But that is not the case for
PQgetf, PQputf, PQparamExec, PQparamSend, ....

The argument against pqtypes being part of libpq was library size overhead.
This was verbalized by many people (issues with redhat packaging were also
brought up). I never heard a complaint about the 10 API calls we wanted to add.
  Only that those 10 API calls came at a 50K library bloat cost, and there were
no buyers.

This brings me back to the dlopen idea.  If you want to use pqtypes,
PQtypesLoad().  The guts of the library are in libpqtypes.so so the only thing
left in libpq are simple functions like below:

// libpq
PQparamExec(...)
{
   if(libpqtypes->paramExec)//typesLoad issued dlsym calls
     // we are in libpq, access to private conn storage granted
     return libpqtypes->paramExec(conn, conn->typeData, ...);
   return "library not loaded: call PQtypesLoad";
}

// end user
#include <libpqtypes.h> // pqtypes typedefs, includes libpq-fe.h
PQtypesLoad(); // call before using libpq
res = PQparamExec(conn, param, .....);

The library size issue is resolved.  I never heard any complaints about this
approach.  Andrew Dunstan said "Please make sure that any scheme you have along
these lines will work on Windows DLLs too.", which didn't sound like a complaint
to me.

#ifdef WIN32
# define dlopen(libname, flags) LoadLibraryA(libname)
# define dlsym(handle, sym) GetProcAddress(handle, sym)
#endif

Tom also weighed in but he thought I was confused about his hook idea (as the
proposed dlopen is completely different):

Tom Wrote:
"This is still 100% backwards.  My idea of a libpq hook is something that
could be used by libpgtypes *and other things*.  What you are proposing
is something where the entire API of the supposed add-on is hard-wired
into libpq."

He is 100% correct, the dlopen idea is 100% backwards from a hook concept.  It
was not an implementation idea for the hooks concept, it was a different
approach altogether.  Instead of a Hooks API, just add the pqtypes API.

What are the objections to adding 10 API calls to libpq (each around 5-10 lines,
minimal typedefs needed) and being able to dlopen behind PQtypesLoad?

hooks vs. dlopen

Hooks:
Need 5 API calls
- PQinitTypes
- PQmakeResult
- PQsetvalue
- PQtypeData - avoid locks in pqtypes
- PQresultTypeData - avoid locks in pqtypes
Need hook points in several places
Need storage in conn and result
Need a couple typedefs
Adds minimal size overhead to libpq
Two funcs are useful to other apps, makeResult & setvalue
Must expose internal type, PGresAttDesc

dlopen
Need 10 API calls
Need storage in conn and result
Need a few typedefs
Adds minimal size overhead to libpq
All funcs are useful to other apps
No internals need to be exposed

The two are not much different.  Although, the hooks idea is a bit awkward when
applied to pqtypes and adds a layer of abstraction not needed.

Another important question is: What's more useful to libpq apps, a hook api
allowing you to receive created/destroyed events about libpq objects OR the
features of pqtypes (put and get any type in binary including arrays & composites)?

Playing with the idea: PQtypesLoad could take an argument specifying a
libpqtypes version (for speaking to a particular backend).  Could even add a
PQtypesUnload so that you can unload+load a different version within the same
process.  For now, I am just proposing a load function that should be called
before using libpq.

--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/