On Fri, May 16, 2008 at 4:23 PM, Tom Lane <[EMAIL PROTECTED]> wrote: > "Merlin Moncure" <[EMAIL PROTECTED]> writes: >> Right. I actually overlooked the 'passthrough' in >> PQregisterEventProc. It turns out that we are still not quite on the >> same page and this needs to be clarified before we move on. The >> passthrough cannot exist... > > Yes, it *will* exist. You are assuming that the goals you have for > these hooks are the only ones anyone will ever have. There are other > possible usage patterns and many of them will need some passthrough > state data. I'd venture to say that anytime I've ever used a callback > design that did not include a passthrough, I've had reason to curse > its designer sooner or later (qsort being a pretty typical example).
right. It can exist, just not hold the event data (the extended properties of PGresult/conn). It has a meaning and scope that are not defined for libpq purposes. For the 'result' callbacks (see below), we will just use the passthrough passed in for the connection. libpqtypes will not use this, and we were getting nervous about people understanding what the different pointers were used for (we would call it a user pointer, not a pass through). > However, it's clear that we are not on the same page, because what > I thought you wanted the PQeventData function to return was the > passthrough pointer. Evidently that's not what you want it to do, > so you'd better back up a few steps and say what you do want it to do. > (I think that a function to get the passthrough value associated with > a given hook function might be useful too, but it's clear that what We will do this: void *PQeventData(PGconn*, PGeventProc, void **passthrough); void *PQresultEventData(PGresult*, PGeventProc, void **passthrough); > you are after must be something else.) The major challenge is that libpqtypes (and, presumably, other libraries) must essentially store its own data in both conn and result. We allocate our data when these structures are created and free it when they are destroyed. The idea is that libpq fires callbacks at appropriate times when conn/results are constructed/destructed. From these events we set up our private 'event' data and delete it. Each connection and result maintains a 'list' of private event states, one for each registering callback (libpqtypes only requires one). So, registering an event proc is analogous to adding one or fields to a conn or a result with a private meaning. So, libpq is defining 6 events: *) when a connection is created (put into OK status) *) when a connection is reset *) when a connection is destroyed *) when a result is created in non-error state *) when a result is copied (we are adding this to libpq w/PQcopyResult) *) when a result is destroyed In these events we set up or tear down the private state for the libpq objects. We need to sometimes look up the data from outside the library in public (non callback) code. This is what PQeventData, etc. accomplish...provide the private state for the object. The difference is that there can be more than one registered event proc on a conn/result at a time; thus the need for an additional "lookup" value when getting event data (what was hookName and is now the event proc address). >> The purpose of the callbacks is to allow a hooking library to >> establish private data that is associated with a PGconn or a PGresult. > > So you're imagining that on creation of a PGconn or PGresult, the > hook function would be allowed to create some storage and libpq would > then be responsible for tracking that storage? Okay, but that's a > separate feature from the sort of passthrough I had in mind. Where > exactly does the hook hand off the storage pointer to libpq? What > are you going to do if the hook fails to create the storage > (ie, out of memory during PGresult creation)? Right, this could happen for one of two likely reasons: OOM as you suggest or the callback fails for some reason only known to the hooking library. In either case, the callback would set the return pointer as defined by the structure to null, return FALSE, and libpq would not add the state to the array of 'event state datas' it maintains. Here is the event proc with passthrough added: int PGEventProc(PGEventId event, void *eventInfo, void *passThrough); // FALSE = failure >> Invoking PQregisterEventProc tells libpq 'I am interested in doing >> this', for the supplied PGconn, future results created with that >> connection, and (if PGconn is passed as null), all future connections. > > I am entirely serious about saying that I will not accept that last bit. > To do that we have to buy into having permanent modifiable storage > within libpq, protecting it with thread mutexes, etc etc. And I don't > like any of the possible usage scenarios for it. I think hooking into a > PGconn immediately after its creation is just as useful and a lot easier > to manage. Locking is not required so long as you follow certain conventions. Here is our warning that describes this in the header: + * WARNING: This should only be called in the application's main thread + * before using any other libpq functions. This is not thread-safe and + * should be used prior to creating any application threads. Obviously, the 'global' behavior is optional, but nice (it's kind of a 'gotcha' if you forget to re-register in a reconnect routine for example). It is like an _init() routine, and considered doing it that way. It does mean adding an array of 'global' event procs to libpq. So I would ask you to hold your judgment on this, and, once we get the new patch in, consider it and we will pull it if you still think it's too dangerous (meaning, the convention is not followed, by a api user). >> Once that is established, libpq begins telling the hooking library >> when and what needs to be allocated or deleted. > > Wait a minute --- you're trying to get between libpq and malloc? > Why? That's getting a *whole* lot more invasive than I thought > this patch intended to be, and I don't see a good reason for it. No, we don't get in between. Maybe my wording was a little misleading...libpq notifies the registered event proc implementations of when a conn or result is created/destroyed. libpq, strictly speaking, is not allocating the private states...just notifying other code it's time to do it. There is no change to the way libpq allocates things generally. When we send up the revised patch, you will see the invasion is minimal...the trickiest part was actually injecting the events in the proper places. merlin -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
