Skip site navigation (1) Skip section navigation (2)

Re: libpq object hooks

From: "Merlin Moncure" <mmoncure(at)gmail(dot)com>
To: "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: "Andrew Chernow" <ac(at)esilo(dot)com>, "Andrew Dunstan" <andrew(at)dunslane(dot)net>, "Alvaro Herrera" <alvherre(at)commandprompt(dot)com>, "Patches (PostgreSQL)" <pgsql-patches(at)postgresql(dot)org>
Subject: Re: libpq object hooks
Date: 2008-05-16 15:13:43
Message-ID: b42b73150805160813x7fba1ccaif0c7d28cc2d53f4d@mail.gmail.com (view raw or flat)
Thread:
Lists: pgsql-hackerspgsql-patches
On Fri, May 16, 2008 at 10:59 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> "Merlin Moncure" <mmoncure(at)gmail(dot)com> writes:
>>> typedef void *(*PGobjectEventProc)(PGobjectEventId evtId, ...);
>>> int PQregisterObjectEventProc(PGconn*, PGobjectEventProc);
>>> void *PQeventData(PGconn *, PGobjectEventProc);
>>> void *PQresultEventData(PGresult *, PGobjectEventProc);
>
>> This provides what we need...a key to lookup the hook data without
>> using a string. Also, it reduces the number of exports (it's a little
>> easier for us, while not essential, to not have to register each
>> callback individually).  Also, this AFAICT does not have any ABI
>> issues (no struct), and adds less exports which is nice.  We don't
>> have to 'look up' the data inside the callbacks..it's properly passed
>> through as an argument.  While vararg callbacks may be considered
>> unsafe in some scenarios, I think it's a good fit here.
>
> I don't think varargs callbacks are a good idea at all.  Please adjust
> this to something that doesn't do that.  Also, haven't you forgotten
> the passthrough void *?

We didn't...one of the functions (the init) doesn't need it so we
didn't expose it to the fixed arguments...we would just va_arg it off
in the other cases.  Regardless, your comments below make that moot:

> If you really want only one callback function, perhaps what would work
> is
>
> typedef void (*PGeventProc) (PGeventId eventId, const void *eventInfo,
>                             void *passthrough);
>
> int PQregisterEventProc(PGconn *conn, PGeventProc proc, void *passthrough);
>
> where eventInfo will point to one of several exported struct types
> depending on the value of eventId.  With this approach, you can add
> more fields to the end of any one such struct type without creating
> ABI issues.  I have little confidence in being able to make similar
> changes in a varargs environment.

this is fine.

> Also, even if varargs are safe they'd be notationally unpleasant
> in the extreme.  varargs are just a PITA to work with --- you'd have
> to do all the decoding in the first-level hook routine, even for
> items you weren't going to use.  With something like the above
> all you need is a switch() and some pointer casts.

Switch, plus struct (basically a union) will do the trick nicely.  Can
it be a formal union, or is it better as a void*?

The main issue was how what we called the 'hook data' was passed back
and forth.  We'll get a patch up.

merlin

In response to

Responses

pgsql-hackers by date

Next:From: Andrew DunstanDate: 2008-05-16 15:21:15
Subject: Re: libpq object hooks
Previous:From: Tom LaneDate: 2008-05-16 14:59:08
Subject: Re: libpq object hooks

pgsql-patches by date

Next:From: Andrew DunstanDate: 2008-05-16 15:21:15
Subject: Re: libpq object hooks
Previous:From: Bruce MomjianDate: 2008-05-16 15:04:21
Subject: Re: libpq thread-locking

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group