pg.ddx.io  pgsql-patches@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Merlin Moncure <mmoncure@gmail.com>
Cc: Andrew Chernow <ac@esilo.com>
Cc: Andrew Dunstan <andrew@dunslane.net>
Cc: Alvaro Herrera <alvherre@commandprompt.com>
Cc: Patches (PostgreSQL) <pgsql-patches@postgresql.org>
Subject: Re: libpq object hooks
Date: Fri, 16 May 2008 10:59:08 -0400
Message-ID: <17177.1210949948@sss.pgh.pa.us> (raw)
In-Reply-To: <b42b73150805160628y3c175608hc383187951206448@mail.gmail.com>
References: <b42b73150804301323i18b00019i62e765708abb65aa@mail.gmail.com>
	<482AD896.4000504@dunslane.net>
	<19342.1210776271@sss.pgh.pa.us>
	<b42b73150805150852m7b3c28fai2d7a4497c84118b2@mail.gmail.com>
	<6044.1210867771@sss.pgh.pa.us>
	<482C77E5.1060500@esilo.com>
	<9458.1210875556@sss.pgh.pa.us>
	<482C82BA.3090100@esilo.com>
	<9867.1210877054@sss.pgh.pa.us>
	<482CD76E.6000202@esilo.com>
	<b42b73150805160628y3c175608hc383187951206448@mail.gmail.com>

"Merlin Moncure" <mmoncure@gmail.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 *?

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.

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.

			regards, tom lane



view thread (45+ messages)  latest in thread

Message-ID: <17177.1210949948@sss.pgh.pa.us>
Permalink:  ../17177.1210949948@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/17177.1210949948@sss.pgh.pa.us

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-patches@postgresql.org
  Cc: tgl@sss.pgh.pa.us, mmoncure@gmail.com, ac@esilo.com, andrew@dunslane.net, alvherre@commandprompt.com
  Subject: Re: libpq object hooks
  In-Reply-To: <17177.1210949948@sss.pgh.pa.us>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox