[PATCHES] libpq events patch
This is an updated version pf the libpqevents patch. See
http://archives.postgresql.org/pgsql-hackers/2008-09/msg00153.php
for details. The only change I didn't make yet is the event 'name'. I
have put it in and taken it out twice now, so a firm 'put it in there'
would be appreciated.
Go here for libpqtypes using events. pgfoundry is still using the older
object hooks version.
http://libpqtypes.esilo.com/libpqtypes-events.tar.gz
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
Index: src/interfaces/libpq/Makefile
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/Makefile,v
retrieving revision 1.166
diff -C6 -r1.166 Makefile
*** src/interfaces/libpq/Makefile 16 Apr 2008 14:19:56 - 1.166
--- src/interfaces/libpq/Makefile 3 Sep 2008 16:06:49 -
***
*** 29,41
# the object files from libpgport, this would not be true on all
# platforms.
LIBS := $(LIBS:-lpgport=)
OBJS= fe-auth.o fe-connect.o fe-exec.o fe-misc.o fe-print.o fe-lobj.o \
fe-protocol2.o fe-protocol3.o pqexpbuffer.o pqsignal.o fe-secure.o \
! md5.o ip.o wchar.o encnames.o noblock.o pgstrcasecmp.o thread.o \
$(filter crypt.o getaddrinfo.o inet_aton.o open.o snprintf.o strerror.o
strlcpy.o win32error.o, $(LIBOBJS))
ifeq ($(PORTNAME), cygwin)
override shlib = cyg$(NAME)$(DLSUFFIX)
endif
--- 29,41
# the object files from libpgport, this would not be true on all
# platforms.
LIBS := $(LIBS:-lpgport=)
OBJS= fe-auth.o fe-connect.o fe-exec.o fe-misc.o fe-print.o fe-lobj.o \
fe-protocol2.o fe-protocol3.o pqexpbuffer.o pqsignal.o fe-secure.o \
! md5.o ip.o wchar.o encnames.o noblock.o pgstrcasecmp.o thread.o
libpq-events.o \
$(filter crypt.o getaddrinfo.o inet_aton.o open.o snprintf.o strerror.o
strlcpy.o win32error.o, $(LIBOBJS))
ifeq ($(PORTNAME), cygwin)
override shlib = cyg$(NAME)$(DLSUFFIX)
endif
***
*** 103,114
--- 103,115
$(top_builddir)/src/port/pg_config_paths.h:
$(MAKE) -C $(top_builddir)/src/port pg_config_paths.h
install: all installdirs install-lib
$(INSTALL_DATA) $(srcdir)/libpq-fe.h '$(DESTDIR)$(includedir)'
+ $(INSTALL_DATA) $(srcdir)/libpq-events.h '$(DESTDIR)$(includedir)'
$(INSTALL_DATA) $(srcdir)/libpq-int.h '$(DESTDIR)$(includedir_internal)'
$(INSTALL_DATA) $(srcdir)/pqexpbuffer.h
'$(DESTDIR)$(includedir_internal)'
$(INSTALL_DATA) $(srcdir)/pg_service.conf.sample
'$(DESTDIR)$(datadir)/pg_service.conf.sample'
installdirs: installdirs-lib
$(mkinstalldirs) '$(DESTDIR)$(includedir)'
'$(DESTDIR)$(includedir_internal)'
Index: src/interfaces/libpq/exports.txt
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/exports.txt,v
retrieving revision 1.19
diff -C6 -r1.19 exports.txt
*** src/interfaces/libpq/exports.txt19 Mar 2008 00:39:33 - 1.19
--- src/interfaces/libpq/exports.txt3 Sep 2008 16:06:49 -
***
*** 138,143
--- 138,154
PQsendDescribePortal 136
lo_truncate 137
PQconnectionUsedPassword 138
pg_valid_server_encoding_id 139
PQconnectionNeedsPassword 140
lo_import_with_oid 141
+ PQcopyResult 142
+ PQsetResultAttrs 143
+ PQsetvalue144
+ PQresultAlloc 145
+ PQregisterEventProc 146
+ PQinstanceData147
+ PQsetInstanceData 148
+ PQresultInstanceData 149
+ PQresultSetInstanceData 150
+ PQpassThroughData 151
+ PQresultPassThroughData 152
Index: src/interfaces/libpq/fe-connect.c
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.359
diff -C6 -r1.359 fe-connect.c
*** src/interfaces/libpq/fe-connect.c 29 May 2008 22:02:44 - 1.359
--- src/interfaces/libpq/fe-connect.c 3 Sep 2008 16:06:49 -
***
*** 1971,1982
--- 1971,2000
* release data that is to be held for the life of the PGconn structure.
* If a value ought to be cleared/freed during PQreset(), do it there not
here.
*/
static void
freePGconn(PGconn *conn)
{
+ int i;
+ PGEventConnDestroy evt;
+
+ /* Let the event procs cleanup their state data */
+ for(i=0; i conn-nEvents; i++)
+ {
+ evt.conn = conn;
+ (void)conn-events[i].proc(PGEVT_CONNDESTROY, evt);
+ }
+
+ /* free the PGEvent array */
+ if(conn-events)
+ {
+ free(conn-events);
+ conn-events = NULL;
+ conn-nEvents = conn-eventArrSize = 0;
+ }
+
if (conn-pghost)
free(conn-pghost);
if (conn-pghostaddr)
free(conn-pghostaddr
Re: [PATCHES] libpq events patch
Andrew Chernow wrote: This is an updated version pf the libpqevents patch. See http://archives.postgresql.org/pgsql-hackers/2008-09/msg00153.php for details. The only change I didn't make yet is the event 'name'. I have put it in and taken it out twice now, so a firm 'put it in there' would be appreciated. Go here for libpqtypes using events. pgfoundry is still using the older object hooks version. http://libpqtypes.esilo.com/libpqtypes-events.tar.gz Patch update again. This one includes an optional event name for debugging purposes. It also includes the changes made by Alvaro that I missed. PQregisterEventProc now takes a name argumet. If the name is NULL, the error message will identify the event procedure by its address ... addr:%p. If its not NULL, error messages will indicate the provided name. I updated the styling in libpq-events.c and a couple places in fe-exec.c that Alvaro missed. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.com/ Index: src/interfaces/libpq/Makefile === RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/Makefile,v retrieving revision 1.166 diff -C6 -r1.166 Makefile *** src/interfaces/libpq/Makefile 16 Apr 2008 14:19:56 - 1.166 --- src/interfaces/libpq/Makefile 3 Sep 2008 21:55:06 - *** *** 29,41 # the object files from libpgport, this would not be true on all # platforms. LIBS := $(LIBS:-lpgport=) OBJS= fe-auth.o fe-connect.o fe-exec.o fe-misc.o fe-print.o fe-lobj.o \ fe-protocol2.o fe-protocol3.o pqexpbuffer.o pqsignal.o fe-secure.o \ ! md5.o ip.o wchar.o encnames.o noblock.o pgstrcasecmp.o thread.o \ $(filter crypt.o getaddrinfo.o inet_aton.o open.o snprintf.o strerror.o strlcpy.o win32error.o, $(LIBOBJS)) ifeq ($(PORTNAME), cygwin) override shlib = cyg$(NAME)$(DLSUFFIX) endif --- 29,41 # the object files from libpgport, this would not be true on all # platforms. LIBS := $(LIBS:-lpgport=) OBJS= fe-auth.o fe-connect.o fe-exec.o fe-misc.o fe-print.o fe-lobj.o \ fe-protocol2.o fe-protocol3.o pqexpbuffer.o pqsignal.o fe-secure.o \ ! md5.o ip.o wchar.o encnames.o noblock.o pgstrcasecmp.o thread.o libpq-events.o \ $(filter crypt.o getaddrinfo.o inet_aton.o open.o snprintf.o strerror.o strlcpy.o win32error.o, $(LIBOBJS)) ifeq ($(PORTNAME), cygwin) override shlib = cyg$(NAME)$(DLSUFFIX) endif *** *** 103,123 $(top_builddir)/src/port/pg_config_paths.h: $(MAKE) -C $(top_builddir)/src/port pg_config_paths.h install: all installdirs install-lib $(INSTALL_DATA) $(srcdir)/libpq-fe.h '$(DESTDIR)$(includedir)' $(INSTALL_DATA) $(srcdir)/libpq-int.h '$(DESTDIR)$(includedir_internal)' $(INSTALL_DATA) $(srcdir)/pqexpbuffer.h '$(DESTDIR)$(includedir_internal)' $(INSTALL_DATA) $(srcdir)/pg_service.conf.sample '$(DESTDIR)$(datadir)/pg_service.conf.sample' installdirs: installdirs-lib $(mkinstalldirs) '$(DESTDIR)$(includedir)' '$(DESTDIR)$(includedir_internal)' uninstall: uninstall-lib ! rm -f '$(DESTDIR)$(includedir)/libpq-fe.h' '$(DESTDIR)$(includedir_internal)/libpq-int.h' '$(DESTDIR)$(includedir_internal)/pqexpbuffer.h' '$(DESTDIR)$(datadir)/pg_service.conf.sample' clean distclean: clean-lib rm -f $(OBJS) pg_config_paths.h crypt.c getaddrinfo.c inet_aton.c noblock.c open.c pgstrcasecmp.c snprintf.c strerror.c strlcpy.c thread.c md5.c ip.c encnames.c wchar.c win32error.c pgsleep.c pthread.h libpq.rc # Might be left over from a Win32 client-only build rm -f pg_config_paths.h --- 103,124 $(top_builddir)/src/port/pg_config_paths.h: $(MAKE) -C $(top_builddir)/src/port pg_config_paths.h install: all installdirs install-lib $(INSTALL_DATA) $(srcdir)/libpq-fe.h '$(DESTDIR)$(includedir)' + $(INSTALL_DATA) $(srcdir)/libpq-events.h '$(DESTDIR)$(includedir)' $(INSTALL_DATA) $(srcdir)/libpq-int.h '$(DESTDIR)$(includedir_internal)' $(INSTALL_DATA) $(srcdir)/pqexpbuffer.h '$(DESTDIR)$(includedir_internal)' $(INSTALL_DATA) $(srcdir)/pg_service.conf.sample '$(DESTDIR)$(datadir)/pg_service.conf.sample' installdirs: installdirs-lib $(mkinstalldirs) '$(DESTDIR)$(includedir)' '$(DESTDIR)$(includedir_internal)' uninstall: uninstall-lib ! rm -f '$(DESTDIR)$(includedir)/libpq-fe.h' '$(DESTDIR)$(includedir)/libpq-events.h' '$(DESTDIR)$(includedir_internal)/libpq-int.h' '$(DESTDIR)$(includedir_internal)/pqexpbuffer.h' '$(DESTDIR)$(datadir)/pg_service.conf.sample' clean distclean: clean-lib rm -f $(OBJS) pg_config_paths.h crypt.c getaddrinfo.c inet_aton.c noblock.c open.c pgstrcasecmp.c snprintf.c strerror.c strlcpy.c thread.c md5.c ip.c encnames.c wchar.c win32error.c pgsleep.c pthread.h libpq.rc # Might be left over from a Win32 client
Re: [PATCHES] libpq object hooks (libpq events)
Will make all of those changes. We appreciate the help. 1. remove global registration :( 2. New Name: PGCallback 3. use instanceData and passThrough names (passThrough with upper 'T') 3. separate getters for conn/result instanceData and passthrough 4. add a setter for result instance data - There should also be a PQsetInstanceData(PGconn*, ...) - I see no need for a passThrough setter 5. move callback stuff to its own header, maybe pgcallback.h? No issue with any of them. Although, small issue below: Maybe instead of having the ResultCreate callback scribble on the event data structure, provide an additional API routine to store the pointer: PQresultSetInstanceData(PGresult *res, PGeventProc proc, void *instanceData); Adding PQresultSetInstanceData doesn't removes the need for a resultcreate callback event. This is an event the callbacks are informed about (instanceData or not). It does remove the need for an instance data member in all event info structures, just use the getter/setter when desired. If the passThrough is needed, one can use the public getters. hooks registered. Also, meseems you need such a callback anyway: what if the hook library desires to realloc its instance data larger? With your suggestions, this would work: res = PQexec(conn, blah); data = PQresultInstanceData(res, cbfunc); data = realloc(data, 1024); PQresultSetInstanceData(res, cbfunc, data); The API user should have a valid instanceData whenever libpq returns a result, assuming they registered a callback that allocates instance data during a resultcreate event. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.com/ -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq object hooks (libpq events)
Tom Lane wrote: Andrew Chernow [EMAIL PROTECTED] writes: 4. add a setter for result instance data - There should also be a PQsetInstanceData(PGconn*, ...) - I see no need for a passThrough setter Check, though I assume we're not expecting PQsetInstanceData to propagate to previously created PGresults? No, not at all. Already created results are on their own. If you want to modify the instanceData, you can use PQresultSetInstanceData. 5. move callback stuff to its own header, maybe pgcallback.h? Should be libpq-something. I was considering libpq-hooks.h or libpq-events.h, but libpq-callback.h would be OK too. I like events. It sounds like you wanted PGCallback, although I am starting to think PGEventProc is better than PGcallback, only because it is more consistent with the term events. BTW, my suggestion to call this libpq events was not directly referring to the callback/proc. It was a term for describing the whole #! I think the idea is to notify interested parties about libpq events, the callback is just an implementaion for doing that. Adding PQresultSetInstanceData doesn't removes the need for a resultcreate callback event. No, of course not. What I was imagining was that the resultcreate callback would call PQresultSetInstanceData. Sorry, my mistake. You were actually very clear, my reading skills are in question. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.com/ -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq object hooks (libpq events)
Attached is the latest patch. It has addressed the requested changes found
here: http://archives.postgresql.org/pgsql-patches/2008-05/msg00389.php
Its a tarball because there are two new files, libpq-events.c and
libpq-events.h. The patch is in the tarball as well as attached to the email.
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
libpq-events.tgz
Description: application/compressed
Index: src/interfaces/libpq/Makefile
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/Makefile,v
retrieving revision 1.166
diff -C6 -r1.166 Makefile
*** src/interfaces/libpq/Makefile 16 Apr 2008 14:19:56 - 1.166
--- src/interfaces/libpq/Makefile 20 May 2008 04:18:07 -
***
*** 29,41
# the object files from libpgport, this would not be true on all
# platforms.
LIBS := $(LIBS:-lpgport=)
OBJS= fe-auth.o fe-connect.o fe-exec.o fe-misc.o fe-print.o fe-lobj.o \
fe-protocol2.o fe-protocol3.o pqexpbuffer.o pqsignal.o fe-secure.o \
! md5.o ip.o wchar.o encnames.o noblock.o pgstrcasecmp.o thread.o \
$(filter crypt.o getaddrinfo.o inet_aton.o open.o snprintf.o strerror.o strlcpy.o win32error.o, $(LIBOBJS))
ifeq ($(PORTNAME), cygwin)
override shlib = cyg$(NAME)$(DLSUFFIX)
endif
--- 29,41
# the object files from libpgport, this would not be true on all
# platforms.
LIBS := $(LIBS:-lpgport=)
OBJS= fe-auth.o fe-connect.o fe-exec.o fe-misc.o fe-print.o fe-lobj.o \
fe-protocol2.o fe-protocol3.o pqexpbuffer.o pqsignal.o fe-secure.o \
! md5.o ip.o wchar.o encnames.o noblock.o pgstrcasecmp.o thread.o libpq-events.o \
$(filter crypt.o getaddrinfo.o inet_aton.o open.o snprintf.o strerror.o strlcpy.o win32error.o, $(LIBOBJS))
ifeq ($(PORTNAME), cygwin)
override shlib = cyg$(NAME)$(DLSUFFIX)
endif
***
*** 103,114
--- 103,115
$(top_builddir)/src/port/pg_config_paths.h:
$(MAKE) -C $(top_builddir)/src/port pg_config_paths.h
install: all installdirs install-lib
$(INSTALL_DATA) $(srcdir)/libpq-fe.h '$(DESTDIR)$(includedir)'
+ $(INSTALL_DATA) $(srcdir)/libpq-events.h '$(DESTDIR)$(includedir)'
$(INSTALL_DATA) $(srcdir)/libpq-int.h '$(DESTDIR)$(includedir_internal)'
$(INSTALL_DATA) $(srcdir)/pqexpbuffer.h '$(DESTDIR)$(includedir_internal)'
$(INSTALL_DATA) $(srcdir)/pg_service.conf.sample '$(DESTDIR)$(datadir)/pg_service.conf.sample'
installdirs: installdirs-lib
$(mkinstalldirs) '$(DESTDIR)$(includedir)' '$(DESTDIR)$(includedir_internal)'
Index: src/interfaces/libpq/exports.txt
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/exports.txt,v
retrieving revision 1.19
diff -C6 -r1.19 exports.txt
*** src/interfaces/libpq/exports.txt 19 Mar 2008 00:39:33 - 1.19
--- src/interfaces/libpq/exports.txt 20 May 2008 04:18:07 -
***
*** 138,143
--- 138,153
PQsendDescribePortal 136
lo_truncate 137
PQconnectionUsedPassword 138
pg_valid_server_encoding_id 139
PQconnectionNeedsPassword 140
lo_import_with_oid 141
+ PQcopyResult 142
+ PQsetvalue143
+ PQresultAlloc 144
+ PQregisterEventProc 145
+ PQinstanceData146
+ PQsetInstanceData 147
+ PQresultInstanceData 148
+ PQresultSetInstanceData 149
+ PQpassThroughData 150
+ PQresultPassThroughData 151
\ No newline at end of file
Index: src/interfaces/libpq/fe-connect.c
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.358
diff -C6 -r1.358 fe-connect.c
*** src/interfaces/libpq/fe-connect.c 16 May 2008 18:30:53 - 1.358
--- src/interfaces/libpq/fe-connect.c 20 May 2008 04:18:08 -
***
*** 1970,1981
--- 1970,1999
* release data that is to be held for the life of the PGconn structure.
* If a value ought to be cleared/freed during PQreset(), do it there not here.
*/
static void
freePGconn(PGconn *conn)
{
+ int i;
+ PGEventConnDestroy evt;
+
+ /* Let the event procs cleanup their state data */
+ for(i=0; i conn-nEvents; i++)
+ {
+ evt.conn = conn;
+ (void)conn-events[i].proc(PGEVT_CONNDESTROY, evt);
+ }
+
+ /* free the PGEvent array */
+ if(conn-events)
+ {
+ free(conn-events);
+ conn-events = NULL;
+ conn-nEvents = conn-eventArrSize = 0;
+ }
+
if (conn-pghost)
free(conn-pghost);
if (conn-pghostaddr)
free(conn-pghostaddr);
if (conn-pgport)
free(conn-pgport);
***
*** 2151,2164
PQreset(PGconn *conn)
{
if (conn)
{
closePGconn(conn);
! if (connectDBStart(conn))
! (void) connectDBComplete(conn);
}
}
/*
* PQresetStart:
--- 2169,2199
PQreset(PGconn *conn)
{
if (conn)
{
closePGconn(conn);
! if (connectDBStart
Re: [PATCHES] libpq object hooks (libpq events)
Merlin Moncure wrote: On Sat, May 17, 2008 at 8:28 AM, Andrew Chernow [EMAIL PROTECTED] wrote: Here is an updated patch for what was called object hooks. This is now called libpq events. If someone has a better name or hates ours, let us know. Let's decide where to go with this. We have no objections to pushing this back to the next fest (tom's comments on the wiki were noted). If that's the case though, we would like to wrap up a consenus on the general implementation in the meantime. Just give us a heads up and I'll update the wiki. merlin Yeah, it would be nice to avoid another push back into July by getting some feedback now for June; it would be great to squeeze it into May. I'm not talking about a review, but maybe a couple of I hate this or This works well or Give up coding :) The implementation of the events (as well as when they were object hooks) is actually very simple, so a quick glance can probably raise most concerns. There is very little going on. In the end, something like this can be done several different ways (choice here is a matter of taste style). It would be nice to iron out the API and focus on other aspects; namely is this general concept appealing enough. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.com/ -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq object hooks (libpq events)
Here is an updated patch for what was called object hooks. This is now
called libpq events. If someone has a better name or hates ours, let us
know.
I am continuing to use the object hooks thread to avoid confusing anyone.
Terminology:
I got rid of calling it object events because it is possible to add
other non-object-related events in the future; maybe a query event.
This system can be used for any type of event, I think it is pretty generic.
event proc - the callback procedure/function implemented outside of
libpq ... PGEventProc. The address of the event proc is used as a
lookup key for getting a conn/result's event data.
event data - the state data managed by the event proc but tracked by
libpq. This allows the event proc implementor to basically add a
dynamic struct member to a conn or result. This is an instance based
value, unlike the arg pointer.
arg pointer - this is the passthrough/user pointer. I called it 'arg'
as other libpq callbacks used this term for this type of value. This
value can be retrieved via PQeventData, PQresultEventData ... its an
optional double pointer argument.
event state - an internal structure, PGEventState, which contains the
event data, event proc and the 'arg' pointer. Internally, PGconn and
PGresult contain arrays of event states.
event id - PGEventId enum values, PGEVT_xxx. This tells the event proc
which event has occurred.
event info - These are the structures for event ids, like
PGEVT_RESULTDESTROY has a corresponding PGEventResultDestroy structure.
The PGEventProc function's 2nd argument is void *info. The first
argument is an event id which tells the proc implementor how to cast the
void *. This replaced the initial va_arg idea.
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
Index: exports.txt
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/exports.txt,v
retrieving revision 1.19
diff -C6 -r1.19 exports.txt
*** exports.txt 19 Mar 2008 00:39:33 - 1.19
--- exports.txt 17 May 2008 12:06:10 -
***
*** 138,143
--- 138,149
PQsendDescribePortal 136
lo_truncate 137
PQconnectionUsedPassword 138
pg_valid_server_encoding_id 139
PQconnectionNeedsPassword 140
lo_import_with_oid 141
+ PQcopyResult 142
+ PQsetvalue143
+ PQresultAlloc 144
+ PQregisterEventProc 145
+ PQeventData 146
+ PQresultEventData 147
Index: fe-connect.c
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.358
diff -C6 -r1.358 fe-connect.c
*** fe-connect.c16 May 2008 18:30:53 - 1.358
--- fe-connect.c17 May 2008 12:06:11 -
***
*** 998,1009
--- 998,1014
* We really shouldn't have been polled in these two
cases, but we
* can handle it.
*/
case CONNECTION_BAD:
return PGRES_POLLING_FAILED;
case CONNECTION_OK:
+ if(!pqRegisterGlobalEvents(conn))
+ {
+ conn-status = CONNECTION_BAD;
+ return PGRES_POLLING_FAILED;
+ }
return PGRES_POLLING_OK;
/* These are reading states */
case CONNECTION_AWAITING_RESPONSE:
case CONNECTION_AUTH_OK:
{
***
*** 1816,1827
--- 1821,1837
conn-next_eo = EnvironmentOptions;
return PGRES_POLLING_WRITING;
}
/* Otherwise, we are open for business! */
conn-status = CONNECTION_OK;
+ if(!pqRegisterGlobalEvents(conn))
+ {
+ conn-status = CONNECTION_BAD;
+ return PGRES_POLLING_FAILED;
+ }
return PGRES_POLLING_OK;
}
case CONNECTION_SETENV:
/*
***
*** 1848,1859
--- 1858,1874
default:
goto error_return;
}
/* We are open for business! */
conn-status = CONNECTION_OK;
+ if(!pqRegisterGlobalEvents(conn))
+ {
+ conn-status = CONNECTION_BAD;
+ return PGRES_POLLING_FAILED
Re: [PATCHES] libpq thread-locking
! int
pthread_mutex_init(pthread_mutex_t *mp, void *attr)
{
*mp = CreateMutex(0, 0, 0);
+ if (*mp == NULL)
+ return 1;
+ return 0;
}
Maybe it would be better to emulate what pthreads does. Instead of
returing 1 to indicate an error, return an errno. In the above case,
ENOMEM seems like a good fit.
Also, maybe you should check the passed in mutex pointer. If its NULL,
you could return EINVAL.
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq object hooks
Tom Lane wrote: typedef void (*PGeventProc) (PGeventId eventId, const void *eventInfo, void *passthrough); int PQregisterEventProc(PGconn *conn, PGeventProc proc, void *passthrough); The above prototypes will work and we will add our 'event instance pointer' to the event info structures. Should have a patch shortly. libpqtypes doesn't need a passthrough/user-pointer. The object events/hooks allocate memory when the object is created part of a conn/result object instance, it is not supplied by the API user registering the event/hook callback. I think this is where some confusion has been occurring, there are two different pointers: user pointer and event instance pointer. BTW, PQeventData and PQresultEventData return the event instance pointer, not the passthrough. At least that is how we were using these functions, being how our previous patches do not include a passthrough/user-pointer feature because libpqtypes didn't need it. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.com/ -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq object hooks
Tom Lane wrote:
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)?
The current submitted patch has 3 of its function callbacks returning a
void*, initHookData after the creation of a conn, resultCreate, and
resultCopy. We have recently changed this design so all hooks, now
called events, go through a single callback ... PGEventProc. The old
function callbacks are just eventIds now.
/
// The libpq side (looping registered event procs)
PGEventResultCreate info;
info.stateData = NULL; /* our event data ptr */
info.conn = conn;
info.result = result;
if(!result-evtState[i].proc(PGEVT_RESULTCREATE,
(void *)info, result-evtState[i].passThrough)
{
PQclear(result); // destroy result? ... not sure
return error;// previously, we ignored it
}
// assign event data created by event proc.
result-evtState[i].data = info.stateData;
///
// example of what the event proc does
int my_evtproc(PGEventId evtId, void *evtInfo, void *passThrough)
{
switch(eventId)
{
case PGEVT_RESULTCREATE:
{
void *data = makeresultdata()
if(!data)
return FALSE;
((PGEventResultCreate *)evtInfo)-stateData = data;
break;
}
}
return TRUE;
}
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq object hooks
Tom Lane wrote: ISTM the hook ought to be able to request that libpq return an out-of-memory failure for the query, just as would happen if the malloc failure had happened directly to libpq. I am working on this new patch and that is how I have been implementing it. If the eventProc function returns FALSE for creation events (not destruction events), the libpq call that triggered it will fail. For instance: for the creation of result objects PGEVT_RESULTCREATE, I am clearing the result and returning an error value. I think that is the expected behavior when one calls PQexec and an out-of-memory failure occurs. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.com/ -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq object hooks
Tom Lane wrote: Merlin Moncure [EMAIL PROTECTED] writes: The problem is the functions PQhookData(conn, hookname) and PQresultHookData(result, hookName). We need these to work in functions that are not callbacks. If we eliminate hookname completely, there is no way for libpq to know which private state we are asking for. Well, depending on a hook name for this is broken-by-design anyway, because there is no way for two independently written libraries to be sure they don't choose conflicting hook names. So the need for a hook name has to go away. It might work to use the address of the hook callback function as a key for retrieving the associated void * pointer. You'd need to not register the same callback function more than once per object, but from what I gather here you don't need to. regards, tom lane There can be cases to use the same callbacks, although unlikely. To completely avoid collisions, the below would work: Use the address of a static, maybe an 'int', as a hook hanlde. Provide the user with a macro that can make a hook handle. typedef void *PGhookHandle; // Declare an int and point tokname at it. The value doesn't // matter, its the pointer address we are interested in. #define PQ_MAKE_HOOK_HANDLE(tokname) \ static int hh__ ## tokname = 0; \ static const PGhookHandle tokname = hh__ ## tokname As an example, here is what libpqtypes would do: // libpqtypes hooks.c PQ_MAKE_HOOK_HANDLE(pqthookhandle); Now the handle replaces the hookName. The const char *hookName member of the PQobjectHooks structure is changed to const PGhookHanlde hookHandle. This allows for all the flexibility of a const char * w/o the collision issues. // these function prototypes change as well void *PQhookData(PGconn *, const PGhookHandle); void *PQresultHookData(PGresult *, const PGhookHandle); We will send in an updated patch. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.com/ -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq object hooks
Andrew Dunstan wrote:
Tom Lane wrote:
It might work to use the address of the hook callback function as
a key for retrieving the associated void * pointer. You'd need to
not register the same callback function more than once per object,
but from what I gather here you don't need to.
Or else have the library return a unique handle when registering hooks,
rather than supplying a hook name.
cheers
andrew
The problem with this is that hooks can be registered on a per-conn
basis. Is there a way to ensure the libpq returned handle would be the
unique across every registration of a given PGobjectHooks? ISTM that
the hook handle needs to be constant and unique somehow. Tom's idea
would work with the very small limitation of not being able to reuse
hook callbacks. I throw out an idea of using the address of a static,
which is constant and unique.
app_func(PGresult *res)
{
PQresultHookData(res, ?handle?);
}
app_func is not aware of what PGconn generated the result so it has no
idea what libpq returned handle to use.
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq object hooks
Tom Lane wrote:
Andrew Chernow [EMAIL PROTECTED] writes:
There can be cases to use the same callbacks, although unlikely. To
completely avoid collisions, the below would work:
Still looks like overdesign to me. If we use the hook function address
we solve the problem with no extra notation and no extra storage.
Note that if you want N fixed keys, you can just have N hook functions
(all calling a shared workhorse routine, no doubt). So your proposal
adds no functionality whatever if the usage involves a fixed number of
static handles. Now it could possibly allow a variable-at-runtime
number of handles, but I'd want to see a worked-out use case before
designing for that much complexity. In particular, it seems to me that
the problem would then shift to how do you know which handle to use for
the lookup, thus you've just introduced another layer of complexity
without buying anything.
I think the typical use case is just that you need to distinguish your
hook from anyone else's hooks, so the function address is plenty
sufficient.
It should also be noted that this whole problem *can* be solved without
any PQhookData at all: as long as you have hooks to get control at
creation and destruction of PGconns and PGresults, you can maintain your
own index data structure. I'm willing to grant some amount of extra
API-stuff to save users having to do that in simple cases, but I don't
think we need to try to support infinitely complex cases.
regards, tom lane
Okay. No problem over here.
Which callback do we use as the key? Currently, none are required (only
the name was required). We have to choose one callback that must be
provided. Maybe initHookData() must be provided? If the end-user
doesn't need it they can just return NULL.
This is what is passed to PQaddObjectHooks, along with a conn:
typedef struct
{
//char *name; REMOVED
void *data;
/* Invoked when PQsetObjectHook is called. The pointer returned
* by the hook implementation is stored in the private storage of
* the PGconn, accessible via PQhookData(PGconn*). If no
* storage is needed, return NULL or set this hook to NULL.
*/
void *(*initHookData)(const PGconn *conn);
/* Invoked on PQreset and PQresetPoll */
void (*connReset)(const PGconn *conn);
/* Invoked on PQfinish. */
void (*connDestroy)(const PGconn *conn);
/* Invoked on PQgetResult, internally called by all exec funcs */
void *(*resultCreate)(const PGconn *conn, const PGresult *result);
/* Invoked on PQcopyResult */
void *(*resultCopy)(PGresult *dest, const PGresult *src);
/* Invoked when PQclear is called */
void (*resultDestroy)(const PGresult *result);
} PGobjectHooks;
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq object hooks
Tom Lane wrote: Andrew Chernow [EMAIL PROTECTED] writes: Which callback do we use as the key? Currently, none are required (only the name was required). We have to choose one callback that must be provided. What? I thought what you wanted back was the void * pointer that had been registered with a particular callback function. So you use that callback function. If it's not actually registered, you get a NULL. This is what is passed to PQaddObjectHooks, along with a conn: This is all wrong IMHO, not least because it creates ABI problems if you want to add another hook type later. Register each hook separately, eg typedef void (*PGCRHook) (PGconn *conn, void *passthrough); void PQregisterConnResetHook(PGconn *conn, PQCRHook func, void *passthrough); ... repeat for each possible hook ... regards, tom lane One can make a case to break apart the obj hooks structure into individual register functions, but I think you have a different idea in your head than what is being proposed. For starters, there is no passthru pointer to register with a callback (there could be but that is different than hook data...your register looks more like a user_ptr). The passthru pointer, what we call hookData, is allocated with a PGconn (not provided by the user). This is the point of the initHookData callback. typedef void *(*PGinitHookData)(const PGconn *conn); PQregisterInitHookData((PGconn *)NULL, (PGinitHookData)func); PQregisterConnResetHook((PGconn *)NULL, (PGCRHook)func); //etc... conn = PQconnectdb(); When connectdb returns, initHookData has already been called. So, a call to PQhookData(conn, ) will work. BUT, what is still missing from the equation is how to uniquely reference hookData on a conn. What I was previously suggesting was to use the address of initHookData, since w/o this address there wouldn't be any hook data to get. Seemed like a logical choice. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.com/ -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq object hooks
Tom Lane wrote:
Andrew Chernow [EMAIL PROTECTED] writes:
Which callback do we use as the key? Currently, none are required (only
the name was required). We have to choose one callback that must be
provided.
What? I thought what you wanted back was the void * pointer that had
been registered with a particular callback function. So you use that
callback function. If it's not actually registered, you get a NULL.
This is what is passed to PQaddObjectHooks, along with a conn:
This is all wrong IMHO, not least because it creates ABI problems if you
want to add another hook type later. Register each hook separately, eg
typedef void (*PGCRHook) (PGconn *conn, void *passthrough);
void PQregisterConnResetHook(PGconn *conn, PQCRHook func, void *passthrough);
... repeat for each possible hook ...
regards, tom lane
I am starting to think we have not clarified what it is we are trying to
do; maybe hook is the wrong terminology.
We need to add members to a conn and result, that's pretty much it. To
do this, an api user can register callbacks to receive notifications
about created/destroyed states of objects. PQhookData is just like
PQerrorMessage in that both are public accessor functions to private
object data. The difference is that there can be more than one hookData
dynamic struct member on a conn/result at a time, unlike errorMessage;
thus the need for an additional lookup value when getting hook data
(what was hookName).
A solution is to only have one function with an eventId, instead of a
register function per event. I am playing with using the name 'event'
rather than hook.
typedef enum
{
PQEVTID_INITDATA,
PQEVTID_CONNRESET,
// resultcreate, resultcopy, etc...
} PGobjectEventId;
typedef void *(*PGobjectEventProc)(PGobjectEventId evtId, ...);
int PQregisterObjectEventProc(PGconn*, PGobjectEventProc);
// no more key (hookName), use PGobjectEventProc address
void *PQeventData(PGconn *, PGobjectEventProc);
void *PQresultEventData(PGresult *, PGobjectEventProc);
Now there is just one libpq register function and an enum; very
resilient to future additions and ABI friendly. The API user will
follow a convention of: if you don't care about an event or don't know
what it is, just return NULL from the eventProc function (ignore it).
The implementation of a PGobjectEventProc would most likely be a switch
on the PGobjectEventId with a couple va_arg() calls (which would be very
well documented).
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq object hooks
I'm wondering why the hooks need names at all. AFAICS all that libpq needs to know about a hook is a callback function address and a void * passthrough pointer. In question is: + void * + PQhookData(const PGconn *conn, const char *hookName) Basically, libpqtypes has various functions that take a PGconn that need the private data that is stored in libpq with the connection. PQhookData just does simple linear search and returns the data. [thinks] are you suggesting something like + void * + PQhookData(const PGconn *conn, const void *hookHandle) ? I would have to take a quick look at the code with Andrew C (he'll be in in a bit)...but this might be doable. The hook callback functions allow the hook implementor to receive created/destroyed events about a PGconn and PGresult (PQreset as well). The hook implementor has the option of associating some memory with either. But, that memory pointer is worthless unless there is a way of referencing it at a later time. HookName would not be needed if the libpq hook API only supported a single Object Hook to be registered per conn (or library-wide). It was requested of us to allow a list of hooks per conn. This requries a way of referencing the item. Functions outside the hook callback functions: - PQparamCreate needs libpq-hook-func void *PQhookData(conn, hookName) - PQgetf needs libpq-hook-func void *PQresultHookData(res, hookName) - Also, the void *resultCreate(...) hook callback implementation inside libpqtypes must use PQhookData on the provided conn so it can copy some conn.hookData properties to the result.hookData. The created result.hookData is returned (one can return NULL if no hookData is needed). I have no issue with merlin's idea of a void *handle, but that doesn't really change the concept at all ... just allows someone registering hooks with libpq to use something other than a string. The hookName string idea feels a little more natural and simple. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.com/ -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq object hooks
Merlin Moncure wrote: On Wed, May 14, 2008 at 10:44 AM, Tom Lane [EMAIL PROTECTED] wrote: I'm wondering why the hooks need names at all. AFAICS all that libpq needs to know about a hook is a callback function address and a void * passthrough pointer. For reference...here is what libpqtypes has to do to bind via the hooking interface: http://libpqtypes.esilo.com/browse_source.html?file=hooks.c Are you proposing something substantially different (not my handle suggestion)? How would it work exactly? merlin It is important to see how NON-hook-callback functions in libpqtypes make use of the hook data. PQparamCreate must get a pointer to the conn hook data http://libpqtypes.esilo.com/browse_source.html?file=param.c#line24 PQgetf must get a pointer to the result hook data http://libpqtypes.esilo.com/browse_source.html?file=exec.c#line65 These are NOT hook callbacks. The hook data is NOT isolated to callback functions. It is memory that is publically accessible, outside hook implementations. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.com/ -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq object hooks
Attached is an updated patch. The only change to this patch is that
hookNames are now compared with strcmp rather than strcasecmp. The
comparisons occur in fe-mics.c (bottom of file) during PQhookData and
PQresultHookData.
Not sure this needs clarification, but PQcopyResult, PQresultAlloc and
PQsetvalue are not part of the object hooks API. They are part of
libpq's result management functions (at least that is what I call them).
Hook API
- PQaddObjectHooks
- PQaddGlobalObjectHooks ** CAN BE REMOVED, SEE BELOW
- PQhookData
- PQresultHookData
Result Management (I would put PQmakeEmptyResult here)
- PQcopyResult
- PQsetvalue
- PQresultAlloc (light wrapper to internal pqResultAlloc)
PROPOSAL:
PQaddGlobalObjectHooks can be removed by handling per-conn and global
hook registeration through PQaddObjectHooks. If the provided PGconn is
NULL, add hooks to global libpq list:
int
PQaddObjectHooks(PGconn *conn, PGobjectHooks *hooks)
{
if(conn == NULL)
;// global hook register
else
;// per-conn register
}
This would make the Object Hooks API 3 functions instead of 4. The same
rules apply to global hook registeration, do this from main() before
using libpq as the ObjectHooks list is not thread-safe (no locking).
NOTE: The patch is called objhooks.patch which is a misleading. The
patch is really comprised of the API calls needed to make libpqtypes
work. If anyone feels this should be broken into two patches (like
objhooks.patch and resmgnt.patch), let us know.
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
Index: exports.txt
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/exports.txt,v
retrieving revision 1.19
diff -C6 -r1.19 exports.txt
*** exports.txt 19 Mar 2008 00:39:33 - 1.19
--- exports.txt 14 May 2008 17:47:59 -
***
*** 138,143
--- 138,150
PQsendDescribePortal 136
lo_truncate 137
PQconnectionUsedPassword 138
pg_valid_server_encoding_id 139
PQconnectionNeedsPassword 140
lo_import_with_oid 141
+ PQcopyResult 142
+ PQsetvalue143
+ PQresultAlloc 144
+ PQaddObjectHooks 145
+ PQaddGlobalObjectHooks146
+ PQhookData147
+ PQresultHookData 148
\ No newline at end of file
Index: fe-connect.c
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.357
diff -C6 -r1.357 fe-connect.c
*** fe-connect.c31 Mar 2008 02:43:14 - 1.357
--- fe-connect.c14 May 2008 17:47:59 -
***
*** 241,253
PQExpBuffer errorMessage);
static char *pwdfMatchesString(char *buf, char *token);
static char *PasswordFromFile(char *hostname, char *port, char *dbname,
char *username);
static void default_threadlock(int acquire);
-
/* global variable because fe-auth.c needs to access it */
pgthreadlock_t pg_g_threadlock = default_threadlock;
/*
*Connecting to a Database
--- 241,252
***
*** 979,990
--- 978,990
* o If your backend wants to use Kerberos authentication then you
must
*supply both a host name and a host address, otherwise this
function
*may block on gethostname.
*
*
*/
+
PostgresPollingStatusType
PQconnectPoll(PGconn *conn)
{
PGresult *res;
charsebuf[256];
***
*** 998,1009
--- 998,1010
* We really shouldn't have been polled in these two
cases, but we
* can handle it.
*/
case CONNECTION_BAD:
return PGRES_POLLING_FAILED;
case CONNECTION_OK:
+ pqInitObjectHooks(conn);
return PGRES_POLLING_OK;
/* These are reading states */
case CONNECTION_AWAITING_RESPONSE:
case CONNECTION_AUTH_OK:
{
***
*** 1816,1827
--- 1817,1829
conn-next_eo = EnvironmentOptions;
return PGRES_POLLING_WRITING;
}
/* Otherwise, we are open for business! */
conn-status = CONNECTION_OK;
+ pqInitObjectHooks(conn);
return PGRES_POLLING_OK;
}
case CONNECTION_SETENV:
/*
***
*** 1848,1859
--- 1850,1862
default:
goto
[PATCHES] libpq object hooks patch
Here is an updated version of the object hooks patch. It now supports a
list of hooks for a PGconn, and PGresult. This had to re-introduce the
concept of hook name. Being that there is now a list, you need a way to
reference an item of that list.
Also added PQobjectHooks and PQresultObjectHooks, to get a pointer to
the conn or result hook list. PQmakeResult must allow the ability to
pass a list of object hooks in. So, PQresultObjectHooks was born.
pqtypes doesn't need (at least at this time) PQobjectHooks but leaving
it out felt unbalanced.
Note: PQhookData and PQresultHookData can be removed. There
functionality can be reproduced by an API user issuing PQobjectHooks or
PQresultObjectHooks and manually looking for there hook (normaly to get
at the hook-data). Basically, an API user would do themselves what
PQhookData is doing.
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
Index: exports.txt
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/exports.txt,v
retrieving revision 1.19
diff -C6 -r1.19 exports.txt
*** exports.txt 19 Mar 2008 00:39:33 - 1.19
--- exports.txt 15 Apr 2008 17:18:38 -
***
*** 138,143
--- 138,151
PQsendDescribePortal 136
lo_truncate 137
PQconnectionUsedPassword 138
pg_valid_server_encoding_id 139
PQconnectionNeedsPassword 140
lo_import_with_oid 141
+ PQmakeResult 142
+ PQsetvalue143
+ PQresultAlloc 144
+ PQaddObjectHooks 145
+ PQhookData146
+ PQresultHookData 147
+ PQobjectHooks 148
+ PQresultObjectHooks 149
\ No newline at end of file
Index: fe-connect.c
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.357
diff -C6 -r1.357 fe-connect.c
*** fe-connect.c31 Mar 2008 02:43:14 - 1.357
--- fe-connect.c15 Apr 2008 17:18:39 -
***
*** 241,253
PQExpBuffer errorMessage);
static char *pwdfMatchesString(char *buf, char *token);
static char *PasswordFromFile(char *hostname, char *port, char *dbname,
char *username);
static void default_threadlock(int acquire);
-
/* global variable because fe-auth.c needs to access it */
pgthreadlock_t pg_g_threadlock = default_threadlock;
/*
*Connecting to a Database
--- 241,252
***
*** 979,990
--- 978,990
* o If your backend wants to use Kerberos authentication then you
must
*supply both a host name and a host address, otherwise this
function
*may block on gethostname.
*
*
*/
+
PostgresPollingStatusType
PQconnectPoll(PGconn *conn)
{
PGresult *res;
charsebuf[256];
***
*** 1875,1887
* the connection structure must be freed).
*/
conn-status = CONNECTION_BAD;
return PGRES_POLLING_FAILED;
}
-
/*
* makeEmptyPGconn
* - create a PGconn data structure with (as yet) no interesting data
*/
static PGconn *
makeEmptyPGconn(void)
--- 1875,1886
***
*** 1970,1981
--- 1969,1998
* release data that is to be held for the life of the PGconn structure.
* If a value ought to be cleared/freed during PQreset(), do it there not
here.
*/
static void
freePGconn(PGconn *conn)
{
+ int i;
+
+ for(i=0; i conn-objHooksCount; i++)
+ {
+ if(conn-objHooks[i].connDestroy)
+ {
+ conn-objHooks[i].connDestroy((const PGconn *)conn);
+ free(conn-objHooks[i].name);
+ }
+ }
+
+ if(conn-objHooks)
+ {
+ free(conn-objHooks);
+ conn-objHooks = NULL;
+ conn-objHooksCount = conn-objHooksSize = 0;
+ }
+
if (conn-pghost)
free(conn-pghost);
if (conn-pghostaddr)
free(conn-pghostaddr);
if (conn-pgport)
free(conn-pgport);
***
*** 2152,2164
--- 2169,2189
{
if (conn)
{
closePGconn(conn);
if (connectDBStart(conn))
+ {
+ int i;
+
(void) connectDBComplete(conn);
+
+ for(i=0; i conn-objHooksCount; i++)
+ if(conn-objHooks[i].connReset)
+ conn-objHooks[i].connReset((const
PGconn *)conn);
+ }
}
}
/*
* PQresetStart:
***
*** 2176,2198
return connectDBStart(conn);
}
return 0
Re: [PATCHES] libpq object hooks patch
Andrew Chernow wrote:
Here is an updated version of the object hooks patch. It now supports a
list of hooks for a PGconn, and PGresult. This had to re-introduce the
concept of hook name. Being that there is now a list, you need a way to
reference an item of that list.
Also added PQobjectHooks and PQresultObjectHooks, to get a pointer to
the conn or result hook list. PQmakeResult must allow the ability to
pass a list of object hooks in. So, PQresultObjectHooks was born.
pqtypes doesn't need (at least at this time) PQobjectHooks but leaving
it out felt unbalanced.
Note: PQhookData and PQresultHookData can be removed. There
functionality can be reproduced by an API user issuing PQobjectHooks or
PQresultObjectHooks and manually looking for there hook (normaly to get
at the hook-data). Basically, an API user would do themselves what
PQhookData is doing.
Made some changes:
1. Removed the hookName argument to PQaddObjectHooks, since its in the
supplied PQobjectHooks struct. I think the argument was lingering from
a previous patch.
2. Added the ability to install global object hooks:
PQaddGlobalObjectHooks(PGobjectHooks*). The header docs for this
function warns that it should only be used before calling libpq
functions or creating application threads. There are no management
functions for global hooks, like get or remove. If you add global
object hooks, your stuck with them until process death.
3. There is a new internal pqInitObjectHooks(PGconn *) that installs the
global object hooks on new conns in the CONNECTION_OK status. I call
this function within PQconnectPoll (3 different locations). This will
call PQaddObjectHooks(conn) for each global hook (so global hooks are
always at the front of a conn's hook list). pqInitObjectHooks checks to
see if conn-objHooksCount 0 and if it is, the request is ignored and
the function returns success. This only happens during a PQreset and
PQresetPoll.
// global
PQaddGlobalObjectHooks(libpq_debugger_hook);
// per-conn
PQaddObjectHooks(conn, session_manager_hook);
Since the existing list of object hooks is scanned for duplicate names
when adding them, you will never run into duplicate object hooks in a
connection (or in the global list). If the two examples above were both
called by an application, the per-conn call would fail.
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
? exports.list
? libpq.so.5.2
? object_hooks.patch
Index: exports.txt
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/exports.txt,v
retrieving revision 1.19
diff -C6 -r1.19 exports.txt
*** exports.txt 19 Mar 2008 00:39:33 - 1.19
--- exports.txt 16 Apr 2008 01:14:40 -
***
*** 138,143
--- 138,152
PQsendDescribePortal 136
lo_truncate 137
PQconnectionUsedPassword 138
pg_valid_server_encoding_id 139
PQconnectionNeedsPassword 140
lo_import_with_oid 141
+ PQmakeResult 142
+ PQsetvalue143
+ PQresultAlloc 144
+ PQaddObjectHooks 145
+ PQhookData146
+ PQresultHookData 147
+ PQobjectHooks 148
+ PQresultObjectHooks 149
+ PQaddGlobalObjectHooks150
\ No newline at end of file
Index: fe-connect.c
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.357
diff -C6 -r1.357 fe-connect.c
*** fe-connect.c31 Mar 2008 02:43:14 - 1.357
--- fe-connect.c16 Apr 2008 01:14:40 -
***
*** 241,253
PQExpBuffer errorMessage);
static char *pwdfMatchesString(char *buf, char *token);
static char *PasswordFromFile(char *hostname, char *port, char *dbname,
char *username);
static void default_threadlock(int acquire);
-
/* global variable because fe-auth.c needs to access it */
pgthreadlock_t pg_g_threadlock = default_threadlock;
/*
*Connecting to a Database
--- 241,252
***
*** 979,990
--- 978,990
* o If your backend wants to use Kerberos authentication then you
must
*supply both a host name and a host address, otherwise this
function
*may block on gethostname.
*
*
*/
+
PostgresPollingStatusType
PQconnectPoll(PGconn *conn)
{
PGresult *res;
charsebuf[256];
***
*** 998,1009
--- 998,1010
* We really shouldn't have been polled in these two
cases, but we
* can handle it.
*/
case CONNECTION_BAD:
return PGRES_POLLING_FAILED;
case CONNECTION_OK:
+ pqInitObjectHooks
Re: [PATCHES] libpq patch for pqtypes hook api and PGresult creation
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/ -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq patch for pqtypes hook api and PGresult creation
Merlin Moncure wrote: On Fri, Apr 11, 2008 at 1:47 PM, Andrew Chernow [EMAIL PROTECTED] wrote: Here are the changes to libpq. It adds the ability to register an Object Hook and create a home-grown result. Patch adds 4 functions. We changed the name of PQresultSetFieldValue to PQsetvalue, which better compliments PQgetvalue. If this patch is acceptable, we will move on to making the required changes to pqtypes; some work has already been done. Whoops! One missing thing here...we forgot to make pqResultAlloc pubilc...pqResultAlloc - PQresultAlloc (see discussion in -hackers). Also, we could use pqResultStrdup (or keep it private, in which case we can re-implement in libpqtypes). merlin The connCreate and resultCreate hooks are in the wrong place. I didn't realize this until I started to implement the hook functions in pqtypes. void (*connCreate)(const char *hookName, const PGconn *conn); The requirements for the connCreate hook are that the PGconn is ready for use. I am currently hooking in connectDBStart, which is dead wrong. After some poking around, it looks like the correct place to hook would be in PQconnectPoll ... does this sound correct? There are 3 places PQconnectPoll returns PGRES_POLLING_OK: one is at the top of the function and the other two are further down with We are open for business! comments. Would I be correct to hook in at these 3 places in PQconnectPoll? void (*resultCreate)(const char *hookName, const PGconn *conn, const PGresult *res); The requirements for resultCreate are that the result is fully constructed. I am currently hooked in PQmakeEmptyResult, again dead wrong. Does PQgetResult sound correct? Also, pqtypes is only interested in CONNECTION_OK and successfull results. But, these hooks are available to more than just pqtypes. What should the when to call connCreate and resultCreate policy be? Should the hook only be called when the conn or result was successfull or should the hooks be called for failed connections/commands as well? -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.com/ -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq patch for pqtypes hook api and PGresult creation
Should the hook only be called when the conn or result was
successfull or should the hooks be called for failed
connections/commands as well?
ISTM that the hooks should be called on success and error (doesn't
include cases where a NULL conn or result is returned). I think this
makes things more useful. If the hooker (pun intended) isn't interested
in error results or conns, it can easily ignore them.
connCreate: The best solution I found was to hook into PQconnectPoll.
This required making the current PQconnectPoll a static named
connectPoll and making PQconnectPoll a wrapper to it. The wrapper just
calls connectPoll and checks the status for hook points.
resultCreate: PQgetResult seemed like the best place. I only notify the
hooks if the resultStatus is NOT copyin or copyout.
I diff'd fe-connect.c and fe-exec.c against cvs which shows these changes.
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
Index: fe-connect.c
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.357
diff -C6 -r1.357 fe-connect.c
*** fe-connect.c31 Mar 2008 02:43:14 - 1.357
--- fe-connect.c12 Apr 2008 13:22:30 -
***
*** 240,252
static int parseServiceInfo(PQconninfoOption *options,
PQExpBuffer errorMessage);
static char *pwdfMatchesString(char *buf, char *token);
static char *PasswordFromFile(char *hostname, char *port, char *dbname,
char *username);
static void default_threadlock(int acquire);
!
/* global variable because fe-auth.c needs to access it */
pgthreadlock_t pg_g_threadlock = default_threadlock;
/*
--- 240,252
static int parseServiceInfo(PQconninfoOption *options,
PQExpBuffer errorMessage);
static char *pwdfMatchesString(char *buf, char *token);
static char *PasswordFromFile(char *hostname, char *port, char *dbname,
char *username);
static void default_threadlock(int acquire);
! static void notifyConnCreateHooks(PGconn *conn);
/* global variable because fe-auth.c needs to access it */
pgthreadlock_t pg_g_threadlock = default_threadlock;
/*
***
*** 891,903
--- 891,907
connectDBComplete(PGconn *conn)
{
PostgresPollingStatusType flag = PGRES_POLLING_WRITING;
time_t finish_time = ((time_t) -1);
if (conn == NULL || conn-status == CONNECTION_BAD)
+ {
+ if(conn)
+ notifyConnCreateHooks(conn);
return 0;
+ }
/*
* Set up a time limit, if connect_timeout isn't zero.
*/
if (conn-connect_timeout != NULL)
{
***
*** 979,992
* o If your backend wants to use Kerberos authentication then you
must
*supply both a host name and a host address, otherwise this
function
*may block on gethostname.
*
*
*/
! PostgresPollingStatusType
! PQconnectPoll(PGconn *conn)
{
PGresult *res;
charsebuf[256];
if (conn == NULL)
return PGRES_POLLING_FAILED;
--- 983,997
* o If your backend wants to use Kerberos authentication then you
must
*supply both a host name and a host address, otherwise this
function
*may block on gethostname.
*
*
*/
!
! static PostgresPollingStatusType
! connectPoll(PGconn *conn)
{
PGresult *res;
charsebuf[256];
if (conn == NULL)
return PGRES_POLLING_FAILED;
***
*** 1875,1886
--- 1880,1908
* the connection structure must be freed).
*/
conn-status = CONNECTION_BAD;
return PGRES_POLLING_FAILED;
}
+ /* See connectPoll. All this does is wrap the real PQconnectPoll so
+ * we can trap PGRES_POLLING_OK or PGRES_POLLING_FAILED statuses. This
+ * could be done in the real connectPoll, but that requires littering the
+ * function with notifyConnCreateHooks calls because there are many
+ * places the function returns a status. This solution is much less
+ * obtrusive and avoids messing with the black magic of connectPoll.
+ */
+ PostgresPollingStatusType
+ PQconnectPoll(PGconn *conn)
+ {
+ PostgresPollingStatusType status = connectPoll(conn);
+
+ if(status == PGRES_POLLING_OK || status == PGRES_POLLING_FAILED)
+ notifyConnCreateHooks(conn);
+
+ return status;
+ }
/*
* makeEmptyPGconn
* - create a PGconn data structure with (as yet) no interesting data
*/
static PGconn *
***
*** 1970,1981
--- 1992,2015
* release data that is to be held for the life of the PGconn structure
[PATCHES] libpq patch for pqtypes hook api and PGresult creation
Here are the changes to libpq. It adds the ability to register an
Object Hook and create a home-grown result. Patch adds 4 functions.
We changed the name of PQresultSetFieldValue to PQsetvalue, which better
compliments PQgetvalue. If this patch is acceptable, we will move on to
making the required changes to pqtypes; some work has already been done.
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
Index: src/interfaces/libpq/exports.txt
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/exports.txt,v
retrieving revision 1.19
diff -c -r1.19 exports.txt
*** src/interfaces/libpq/exports.txt19 Mar 2008 00:39:33 - 1.19
--- src/interfaces/libpq/exports.txt11 Apr 2008 17:36:26 -
***
*** 141,143
--- 141,147
pg_valid_server_encoding_id 139
PQconnectionNeedsPassword 140
lo_import_with_oid 141
+ PQaddObjectHook 142
+ PQremoveObjectHook143
+ PQmakeResult 144
+ PQsetvalue145
\ No newline at end of file
Index: src/interfaces/libpq/fe-connect.c
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.357
diff -c -r1.357 fe-connect.c
*** src/interfaces/libpq/fe-connect.c 31 Mar 2008 02:43:14 - 1.357
--- src/interfaces/libpq/fe-connect.c 11 Apr 2008 17:36:26 -
***
*** 866,872
--- 866,887
* we are in PGRES_POLLING_WRITING connection state.
*/
if (PQconnectPoll(conn) == PGRES_POLLING_WRITING)
+ {
+ int objHooksCount;
+ const PGobjectHooks *objHooks;
+
+ if((objHooksCount = pqGetObjectHooks(objHooks)) 0)
+ {
+ int i;
+
+ for(i=0; i objHooksCount; i++)
+ if(objHooks[i].connCreate)
+
objHooks[i].connCreate(objHooks[i].name, conn);
+ }
+ pqReleaseObjectHooks();
+
return 1;
+ }
connect_errReturn:
if (conn-sock = 0)
***
*** 1973,1978
--- 1988,2006
static void
freePGconn(PGconn *conn)
{
+ int objHooksCount;
+ const PGobjectHooks *objHooks;
+
+ if((objHooksCount = pqGetObjectHooks(objHooks)) 0)
+ {
+ int i;
+
+ for(i=0; i objHooksCount; i++)
+ if(objHooks[i].connDestroy)
+ objHooks[i].connDestroy(objHooks[i].name, conn);
+ }
+ pqReleaseObjectHooks();
+
if (conn-pghost)
free(conn-pghost);
if (conn-pghostaddr)
Index: src/interfaces/libpq/fe-exec.c
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-exec.c,v
retrieving revision 1.194
diff -c -r1.194 fe-exec.c
*** src/interfaces/libpq/fe-exec.c 1 Jan 2008 19:46:00 - 1.194
--- src/interfaces/libpq/fe-exec.c 11 Apr 2008 17:36:27 -
***
*** 63,69
static PGresult *PQexecFinish(PGconn *conn);
static int PQsendDescribe(PGconn *conn, char desc_type,
const char *desc_target);
!
/*
* Space management for PGresult.
--- 63,69
static PGresult *PQexecFinish(PGconn *conn);
static int PQsendDescribe(PGconn *conn, char desc_type,
const char *desc_target);
! static int check_field_number(const PGresult *res, int field_num);
/*
* Space management for PGresult.
***
*** 139,144
--- 139,146
PQmakeEmptyPGresult(PGconn *conn, ExecStatusType status)
{
PGresult *result;
+ int objHooksCount;
+ const PGobjectHooks *objHooks;
result = (PGresult *) malloc(sizeof(PGresult));
if (!result)
***
*** 192,200
--- 194,341
result-client_encoding = PG_SQL_ASCII;
}
+ if((objHooksCount = pqGetObjectHooks(objHooks)) 0)
+ {
+ int i;
+
+ for(i=0; i objHooksCount; i++)
+ if(objHooks[i].resultCreate)
+ objHooks[i].resultCreate(objHooks[i].name,
conn, result);
+ }
+ pqReleaseObjectHooks();
+
return result;
}
+ PGresult *PQmakeResult(
+ PGconn *conn,
+ int numAttributes,
+ PGresAttDesc *attDescs)
+ {
+ int i;
+ PGresult *res;
+
+ if(numAttributes = 0 || !attDescs)
+ return NULL;
+
+ res = PQmakeEmptyPGresult(conn, PGRES_TUPLES_OK);
+ if(!res)
+ return NULL;
+
+ res-attDescs = (PGresAttDesc *)
+ pqResultAlloc(res, numAttributes * sizeof(PGresAttDesc), TRUE);
+
+ if(!res-attDescs
[PATCHES] libpq Win32 Mutex performance patch
I noticed several months ago, and came across it again today, that
libpq's pthread-win32.c implementation is using CreateMutex rather than
CRITICAL_SECTION. CreateMutex is like a semaphore in that it is
designed to be accessible via name system-wide. Even when you don't
give it a name, thus bound to process that created it, it still carries
significant overhead compared to using win32 CRITICAL_SECTIONs.
The attached patch replaces the win32 mutex calls with critical section
calls. The change will not affect the behavior of the windows
pthread_xxx functions.
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
Index: src/port/pthread-win32.h
===
RCS file: /projects/cvsroot/pgsql/src/port/pthread-win32.h,v
retrieving revision 1.2
diff -c -r1.2 pthread-win32.h
*** src/port/pthread-win32.h18 Apr 2007 08:32:40 - 1.2
--- src/port/pthread-win32.h11 Apr 2008 18:35:33 -
***
*** 2,8
#define __PTHREAD_H
typedef ULONG pthread_key_t;
! typedef HANDLE pthread_mutex_t;
typedef int pthread_once_t;
DWORD pthread_self(void);
--- 2,8
#define __PTHREAD_H
typedef ULONG pthread_key_t;
! typedef CRITICAL_SECTION *pthread_mutex_t;
typedef int pthread_once_t;
DWORD pthread_self(void);
Index: src/interfaces/libpq/pthread-win32.c
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/pthread-win32.c,v
retrieving revision 1.15
diff -c -r1.15 pthread-win32.c
*** src/interfaces/libpq/pthread-win32.c1 Jan 2008 19:46:00 -
1.15
--- src/interfaces/libpq/pthread-win32.c11 Apr 2008 18:35:33 -
***
*** 35,51
void
pthread_mutex_init(pthread_mutex_t *mp, void *attr)
{
! *mp = CreateMutex(0, 0, 0);
}
void
pthread_mutex_lock(pthread_mutex_t *mp)
{
! WaitForSingleObject(*mp, INFINITE);
}
void
pthread_mutex_unlock(pthread_mutex_t *mp)
{
! ReleaseMutex(*mp);
}
--- 35,69
void
pthread_mutex_init(pthread_mutex_t *mp, void *attr)
{
! if(mp)
! {
! *mp = (CRITICAL_SECTION *)malloc(sizeof(CRITICAL_SECTION));
! if(*mp)
! InitializeCriticalSection(*mp);
! }
}
void
pthread_mutex_lock(pthread_mutex_t *mp)
{
! if(mp *mp)
! EnterCriticalSection(*mp);
}
void
pthread_mutex_unlock(pthread_mutex_t *mp)
{
! if(mp *mp)
! LeaveCriticalSection(*mp);
}
+
+ /* If ever needed
+ pthread_mutex_destroy(pthread_mutex_t *mp)
+ {
+ if(mp *mp)
+ {
+ DeleteCriticalSection(*mp);
+ *mp = NULL;
+ }
+ }
+ */
\ No newline at end of file
--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq Win32 Mutex performance patch
Magnus Hagander wrote: It changes the behavior when the pointer passed in is invalid from crash to silent working, right? Correct, it a Habit. I sub-consciously write code that checks pointers. We can remove the pointer checks and let the thing dump core if people prefer. Which brings up the second point - is there any actual reason for adding the pthread_mutex_destroy call? Since libpq never calls it, and it's never used from outside libpq (it's not exported outside the library even), isn't it just going to end up as dead code? //Magnus The destroy call is within a comment. I only put it there in case it is ever needeed. BTW, I just noticed the commented destroy call forgot to free(*mp) ... ooppssseee. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.com/ -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq Win32 Mutex performance patch
Tom Lane wrote: Andrew Chernow [EMAIL PROTECTED] writes: The attached patch replaces the win32 mutex calls with critical section calls. The change will not affect the behavior of the windows pthread_xxx functions. Why have you defined the lock/unlock functions as willing to fall through silently if handed a null pointer? I think a crash in such a case is what we *want*. Silently not locking is surely not very safe. regards, tom lane Yeah, both naughty. These functions were not implemented to spec. These pthread functions are all supposed to return an int (which is an errno value). I was trying not to change the existing prototypes ... should I? I can return EINVAL if something is NULL and ENOMEM if the malloc fails ... or just dump core. If you like the return value idea, I can make all occurances of pthread_xxx check the return value. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.com/ -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq Win32 Mutex performance patch
Tom Lane wrote:
Silently not locking is surely
not very safe.
Here is the dump code version of the patch. If anyone wants the return
value idea, let me know.
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
Index: src/port/pthread-win32.h
===
RCS file: /projects/cvsroot/pgsql/src/port/pthread-win32.h,v
retrieving revision 1.2
diff -c -r1.2 pthread-win32.h
*** src/port/pthread-win32.h18 Apr 2007 08:32:40 - 1.2
--- src/port/pthread-win32.h11 Apr 2008 19:22:17 -
***
*** 2,8
#define __PTHREAD_H
typedef ULONG pthread_key_t;
! typedef HANDLE pthread_mutex_t;
typedef int pthread_once_t;
DWORD pthread_self(void);
--- 2,8
#define __PTHREAD_H
typedef ULONG pthread_key_t;
! typedef CRITICAL_SECTION *pthread_mutex_t;
typedef int pthread_once_t;
DWORD pthread_self(void);
Index: src/interfaces/libpq/pthread-win32.c
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/pthread-win32.c,v
retrieving revision 1.15
diff -c -r1.15 pthread-win32.c
*** src/interfaces/libpq/pthread-win32.c1 Jan 2008 19:46:00 -
1.15
--- src/interfaces/libpq/pthread-win32.c11 Apr 2008 19:22:17 -
***
*** 35,51
void
pthread_mutex_init(pthread_mutex_t *mp, void *attr)
{
! *mp = CreateMutex(0, 0, 0);
}
void
pthread_mutex_lock(pthread_mutex_t *mp)
{
! WaitForSingleObject(*mp, INFINITE);
}
void
pthread_mutex_unlock(pthread_mutex_t *mp)
{
! ReleaseMutex(*mp);
}
--- 35,61
void
pthread_mutex_init(pthread_mutex_t *mp, void *attr)
{
! *mp = (CRITICAL_SECTION *)malloc(sizeof(CRITICAL_SECTION));
! InitializeCriticalSection(*mp);
}
void
pthread_mutex_lock(pthread_mutex_t *mp)
{
! EnterCriticalSection(*mp);
}
void
pthread_mutex_unlock(pthread_mutex_t *mp)
{
! LeaveCriticalSection(*mp);
}
+
+ /* If ever needed
+ void pthread_mutex_destroy(pthread_mutex_t *mp)
+ {
+ DeleteCriticalSection(*mp);
+ free(*mp);
+ *mp = NULL;
+ }
+ */
\ No newline at end of file
--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq Win32 Mutex performance patch
Andrew Chernow wrote: Tom Lane wrote: Silently not locking is surely not very safe. Here is the dump code version of the patch. If anyone wants the return value idea, let me know. A more graceful solution would be to print something to stderr and then exit. This allows an app's atexit funcs to run. I don't think libpq should core dump an app by choice. I'd be pretty upset if a library I was using core dumped in some cases rather than exiting. andrew -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq Win32 Mutex performance patch
daveg wrote: On Fri, Apr 11, 2008 at 06:25:53PM -0400, Tom Lane wrote: Andrew Chernow [EMAIL PROTECTED] writes: A more graceful solution would be to print something to stderr and then exit. stderr doesn't exist, or point to a useful place, in many environments. And a forced exit() is no better than a crash for most purposes. I don't think libpq should core dump an app by choice. The case that we are talking about is a bug, or would be a bug if it could happen (the fact that we've gotten along happily with no similar test in the existing code shows that it can't). Many forms of bug can result in core dumps; it's foolish to try to prevent them all. And bloating one line of code into five or more lines to defend against can't-happen cases is a good way to end up with unreadable, unmaintainable software. regards, tom lane +1 -dg okay. BTW, my real interest here is the libpq hooks patch requires a lock/unlock for every conn-create, conn-destroy, result-create, result-destroy. Currently, it looks like only the ssl stuff uses mutexes. Adding more mutex use is a good case for a more optimized approach on windows. andrew -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq type system 0.9a
Joe Conway wrote: Alvaro Herrera wrote: Merlin Moncure escribió: Yesterday, we notified -hackers of the latest version of the libpq type system. Just to be sure the right people are getting notified, we are posting the latest patch here as well. Would love to get some feedback on this. I had a look at this patch some days ago, and the first question in my mind was: why is it explicitely on libpq? Why not have it as a separate library (say libpqtypes)? That way, applications not using it would not need to link to it. Applications interested in using it would just need to add another -l switch to their link line. +1 Joe What is gained by having a separate library? Our changes don't bloat the library size so I'm curious what the benefits are to not linking with it? If someone doesn't want to use, they don't have to. Similar to the backend, there is stuff in there I personally don't use (like geo types), but I'm not sure that justifies a link option -lgeotypes. The changes we made are closely tied to libpq's functionality. Adding PQputf to simplify the parameterized API, adding PQgetf to compliment PQgetvalue and added the ability to register user-defined type handlers (used by putf and getf). PQgetf makes extensive use of PGresult's internal API, especially for arrays and composites. Breaking this into a separate library would require an external library to access the private internals of libpq. Personally, I am not really in favor of this idea because it breaks apart code that is very related. Although, it is doable. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.com/ -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq type system 0.9a
Andrew Chernow wrote: Joe Conway wrote: Alvaro Herrera wrote: Merlin Moncure escribió: Yesterday, we notified -hackers of the latest version of the libpq type system. Just to be sure the right people are getting notified, we are posting the latest patch here as well. Would love to get some feedback on this. I had a look at this patch some days ago, and the first question in my mind was: why is it explicitely on libpq? Why not have it as a separate library (say libpqtypes)? That way, applications not using it would not need to link to it. Applications interested in using it would just need to add another -l switch to their link line. +1 Joe What is gained by having a separate library? Our changes don't bloat the library size so I'm curious what the benefits are to not linking with it? If someone doesn't want to use, they don't have to. Similar to the backend, there is stuff in there I personally don't use (like geo types), but I'm not sure that justifies a link option -lgeotypes. The changes we made are closely tied to libpq's functionality. Adding PQputf to simplify the parameterized API, adding PQgetf to compliment PQgetvalue and added the ability to register user-defined type handlers (used by putf and getf). PQgetf makes extensive use of PGresult's internal API, especially for arrays and composites. Breaking this into a separate library would require an external library to access the private internals of libpq. Personally, I am not really in favor of this idea because it breaks apart code that is very related. Although, it is doable. I poked around to see how this would work. There are some problems. 1. members were added to PGconn so connection-based type handler information can be copied to PGparam and PGresult objects. 2. members were added to PGresult, referenced in #1. To properly getf values, the connection-based type handler information must be available to PGresult. Otherwise, PQgetf would require an additional argument, a PGconn, which may have been closed already. 3. PQfinish calls pqClearTypeHandler to free type info assigned to the PGconn. 4. PQclear also calls pqClearTypeHandlers It would also remove some of the simplicity. Creating a connection would no longer initialized type info, which gets copied to PGparam and PGresult. Type info includes a list of built-in handlers and backend config, like integer_datetimes, server-version, etc... That means an additional function must be called after PQconnectdb. But where would the type info be stored? It wouldn't exist in PGconn anymore? Also, this would require double frees. You have to free the result as well as the type info since they are no longer one object. Same holds true for a pgconn. There is something elegant about not requiring additional API calls to perform a putf or getf. It'll just work if you want to use it. You can use PQgetf on a result returned by PQexec and you can use PQputf, PQparamExec followed by PQgetvalue. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.com/ -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq type system 0.9a
Merlin Moncure wrote: On Wed, Mar 5, 2008 at 5:47 PM, Florian G. Pflug [EMAIL PROTECTED] wrote: Merlin Moncure wrote: Yesterday, we notified -hackers of the latest version of the libpq type system. Just to be sure the right people are getting notified, we are posting the latest patch here as well. Would love to get some feedback on this. Sorry if this has been discussed before, but why is it necessary to specify the type when calling PQgetf on a result? It seems that this formatting string *always* has to match the type list of your select statement, no? yes...it always has to match. the format string requirements could in theory be relaxed (for 'get') but this would break symmetry with 'put' and you would lose a sanity check...getf like scanf writes directly into application memory so the double-specifying (directly in the format string and indirectly in the query) isn't necessarily a bad thing. imagine if your application was 'select * from table' and one of the field types changed...disaster. merlin A few other reasons why is it necessary to specify the type when calling PQgetf on a result Unlike PQgetvalue, all values returned by PQgetf are either native C types or structures ... not C strings. When you call getf you must tell it what types to read out of the result object. Like scanf, they must be the correctly sized data types. PGdate date; int i4; PQgetf(result, tup_num, %date %int4, 0, date, 1, i4); Specifying anything other than a %date or %int4 in the above example is a programming error. You would be asking to fetch a value of the wrong type. Without the formatting string, libpq would have to va_arg(ASSUME_T) your value. // no specifier int i; PQgetf(result, tup, field, i); In the above, libpq would have to use PQftype to determine what the native C type is of your variable argument. If PQftype returned INT8OID, you begin to clobber your application's memory space ... va_arg(ap, long long) on a 32-bit value. This problem is solved by telling libpq what data type you want from a field. Also, the libpq type system enforces strict type checking when performing getf calls. This protects from mis-matches programming errors on types: For example: -- create table t (a int8); PQresult *result = PQexec(conn, SELECT a FROM t); char *val = PQgetvalue(result, ...); int a = atoi(val); // assumed its an int4 In the above example, the libpq user thinks the 'a' column of the 't' table is an int4 when in fact its an int8. The above may work most of the time but will eventually truncate the value and nip you in the butt. With PQgetf, you would get an error saying the server returned an int8 and you are asking for an int4. Thus, the programming bug would be squashed immediately. Also, user-defined types are not known to libpq so PQftype would not really work. They could if the libpq type system referenced data types by OID, but this is not portable to other servers. It is more portable to use the type name. For example, a company with 15 postgresql servers that use the same collection of company-specific user-defined data types. The type names would be the same across the 15 servers but there is no guarentee the OIDs would be. Composites and arrays caused a few issues as well. We also tried to provide as much protection as possible ... in the spirit of the backend. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.com/ -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://mail.postgresql.org/mj/mj_wwwusr?domain=postgresql.orgextra=pgsql-patches
Re: [PATCHES] PQParam version 0.5
Tom Lane wrote: Andrew Chernow [EMAIL PROTECTED] writes: Here is the lastest pgparam patch. It is patched against a fresh checkout on 2007-12-05. What is this for? Why is it a good idea? It appears to be a fairly enormous increase in the size of libpq's API, and I really don't think I want to buy into the idea that libpq should know explicitly about each and every backend datatype. The 100% lack of any documentation in the patch isn't helping you sell it, BTW. regards, tom lane enormous increase in the size of libpq's API We can dramatically reduce the exports by using macros, if preferred. The 100% lack of any documentation Okay, we will do this. For starters, take a look at test.c. Below is a brief description: 1. Managed params, rather than manually building PQexecParam arrays; which is a little error prone and tedious. PQputint4(conn, 5); PQputtextptr(conn, abc); PQparamExec(conn, INSERT INTO t VALUES ($1, $2), 1, NULL); // the NULL arg is a PGresult**, which is auto-cleared // when NULL. Otherwise *result is assigned. // or use the print-style: we changed the argument order since // our last release, it felt off. PGresult *r; PQparamExecf(conn, SELECT * FROM foo(%d, %t), 1, r, 5, abc); 2. In binary result mode, the user has no idea how the data is formatted and there are no demarshaling functions, thus making the binary parameterized API impractical. So, we made PQget functions that support text or binary results. The benefit of supporting both is that the new PQget functions can be used regardless of how the query was executed. long long i8; PGinet inet; PQgetint8(res, 0, 0, i8); PQgetinet(res, 0, 1, inet); // coming soon. Currently, no way of doing this now. PGarr arr; int item, itemlen; PQgetarr(res, 0, 0, arr); // access 2 dim 2d array - arr[2][7] itemlen = PQgetarr2d(arr, item, 2, 7); 3. Client server should both understand how data is formatted over the wire, otherwise the data received by the client is not useful. Things like int4 or even a BOX are not that tricky, but other types are or may change between versions. 4. Why do atoi(PQgetvalue(...)) everywhere? Andrew ---(end of broadcast)--- TIP 6: explain analyze is your friend
[PATCHES] PQParam version 0.5
Here is the lastest pgparam patch. It is patched against a fresh checkout on 2007-12-05. This release adds support for printf-style param puts/execs. Instead of having to do a PQputX for each param, you can use a format string and put multiple params. PQputf(), PQparamExecf() and PQparamSendf() support this. See release notes and conversion specifiers for details. Also changed PQputint8's prototype. Previously, it was using a void* as the value argument, due to a lack of a portable 64-bit type in libpq. We found an intersting way around this by using macro and variable argument tricks. Andrew Merlin pgparam-0.5-patch.tgz Description: application/compressed ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] PQParam version 0.5
Merlin Moncure wrote: On Dec 5, 2007 2:44 PM, Alvaro Herrera [EMAIL PROTECTED] wrote: Andrew Chernow escribió: Also changed PQputint8's prototype. Previously, it was using a void* as the value argument, due to a lack of a portable 64-bit type in libpq. We found an intersting way around this by using macro and variable argument tricks. I didn't read the patch, but variadic macros are not portable. FWIW uint64 should portable to all platforms that have it (and it should be 32 bits on platforms that don't), but you have to watch for INT64_IS_BUSTED. we don't use variadic macros...just a macro wrapper to a variadic function. merlin ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq Taken from libpq-fe.h #define PQputint8(conn, i8) PQputint8v(conn, sizeof(i8), i8) /* Function subject to change. Do not use directly, see PQputint8. */ extern int PQputint8v(PGconn *conn, size_t valsize, ...); // goal was pass by value, not by ptr, which was our first solution PQputint8(conn, 12345678912345LL); The problem is libpq has no public 64-bit data type to use with the PQputint8 prototype. But! if we make PQputint8 a macro that wraps a variadic function, we get around the data type issue. Since libpq doesn't have a public 64-bit portable data type, we felt this was done for a good reason. We didn't want to break that convention. andrew ---(end of broadcast)--- TIP 6: explain analyze is your friend
[PATCHES] PGparam extension version 0.4
Version 0.4 of libpq param put and PGresult get functions. Added support for inet and cidr, couple bug fixes. If you compile the test file, make sure you link with the patched libpq.so. Andrew pg_param.tgz Description: application/compressed ---(end of broadcast)--- TIP 6: explain analyze is your friend
[PATCHES] PGparam extension version 0.4
Version 0.4 of libpq param put and PGresult get functions. Added support for inet and cidr, couple bug fixes. If you compile the test file, make sure you link with the patched libpq.so. Andrew pg_param.tgz Description: application/compressed ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
[PATCHES] Patch to correct 64-bit money type in 8.3devel
Attached is a patch for the 8.3devel 64-bit money type. Bug reported
here: http://archives.postgresql.org/pgsql-bugs/2007-08/msg00137.php.
Run the test program included in the bug report to see the issue. Then
apply patch and run the test again.
Andrew Chernow
Index: src/backend/utils/adt/cash.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/utils/adt/cash.c,v
retrieving revision 1.71
diff -c -C6 -r1.71 cash.c
*** src/backend/utils/adt/cash.c12 Jul 2007 23:51:10 - 1.71
--- src/backend/utils/adt/cash.c20 Aug 2007 14:10:44 -
***
*** 369,394
*/
Datum
cash_recv(PG_FUNCTION_ARGS)
{
StringInfo buf = (StringInfo) PG_GETARG_POINTER(0);
! PG_RETURN_CASH((Cash) pq_getmsgint(buf, sizeof(Cash)));
}
/*
*cash_send - converts cash to
binary format
*/
Datum
cash_send(PG_FUNCTION_ARGS)
{
Casharg1 = PG_GETARG_CASH(0);
StringInfoData buf;
pq_begintypsend(buf);
! pq_sendint(buf, arg1, sizeof(Cash));
PG_RETURN_BYTEA_P(pq_endtypsend(buf));
}
/*
* Comparison functions
*/
--- 369,394
*/
Datum
cash_recv(PG_FUNCTION_ARGS)
{
StringInfo buf = (StringInfo) PG_GETARG_POINTER(0);
! PG_RETURN_CASH((Cash) pq_getmsgint64(buf));
}
/*
*cash_send - converts cash to
binary format
*/
Datum
cash_send(PG_FUNCTION_ARGS)
{
Casharg1 = PG_GETARG_CASH(0);
StringInfoData buf;
pq_begintypsend(buf);
! pq_sendint64(buf, arg1);
PG_RETURN_BYTEA_P(pq_endtypsend(buf));
}
/*
* Comparison functions
*/
---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings
