Thread: libpq object hooks
Attached is an updated version of 'libpq object hooks'. The primary purpose for libpq object hooks is to support our libpqtypes system (not attached), but could possibly some nice sideband features to libpq. We are hoping to sneak this into the May commit fest. regards, merlin
Attachment
Merlin Moncure wrote: > Attached is an updated version of 'libpq object hooks'. The primary > purpose for libpq object hooks is to support our libpqtypes system > (not attached), but could possibly some nice sideband features to > libpq. We are hoping to sneak this into the May commit fest. > > I've had a preliminary look at this. The first thing it needs is lots and lots of documentation. I think it probably needs a Section in the libpq chapter all on its own, preferably with some examples. I think that lack alone is enough to keep it from being committed for now. Second, the hook names are compared case insensitively and by linear search. I don't see any justification for using case insensitive names for hooks in a C program, so I think that part should go. And if we expect to keep anything other than trivial numbers of hooks we should look at some sort of binary or hashed search. The thing that is a bit disturbing is that the whole style of this scheme is very different from the fairly simple APIs that the rest of libpq presents. It's going to make libpq look rather odd, I think. I would have felt happier if the authors had been able to come up with a simple scheme to add API calls to export whatever information they needed, rather than using this callback scheme. That said, this patch doesn't look that bad to me otherwise, at least on first inspection. One might say the the ability to add tuples to a resultset arbitrarily, or to change an attribute arbitrarily, might be footguns (and if you can add one, why can't you delete one?), but then this is data in the hands of the client anyway, so they can do what they like with it after they get it out of the resultset, so I guess there's no real danger. cheers andrew
Andrew Dunstan wrote: > The first thing it needs is lots and lots of documentation. I think it > probably needs a Section in the libpq chapter all on its own, preferably > with some examples. I think that lack alone is enough to keep it from > being committed for now. > > Second, the hook names are compared case insensitively and by linear > search. I don't see any justification for using case insensitive names > for hooks in a C program, so I think that part should go. And if we > expect to keep anything other than trivial numbers of hooks we should > look at some sort of binary or hashed search. > > The thing that is a bit disturbing is that the whole style of this > scheme is very different from the fairly simple APIs that the rest of > libpq presents. It's going to make libpq look rather odd, I think. I > would have felt happier if the authors had been able to come up with a > simple scheme to add API calls to export whatever information they > needed, rather than using this callback scheme. My personal opinion is still that I would like to see a more general usefulness for these functions before adding them to libpq. The complexity of the API just mirrors my gut feeling on this. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Andrew Dunstan escribió: > The thing that is a bit disturbing is that the whole style of this > scheme is very different from the fairly simple APIs that the rest of > libpq presents. It's going to make libpq look rather odd, I think. I > would have felt happier if the authors had been able to come up with a > simple scheme to add API calls to export whatever information they > needed, rather than using this callback scheme. I'm not sure I understand this point. Remember that this is here to support the libpqtypes library. There doesn't seem to be a way for an API such as you describe to work. > Second, the hook names are compared case insensitively and by linear > search. I don't see any justification for using case insensitive names > for hooks in a C program, so I think that part should go. And if we > expect to keep anything other than trivial numbers of hooks we should > look at some sort of binary or hashed search. Keep in mind that the original patch supported a single hook being registered. Perhaps we could dream about having a couple of hooks registered, but turning into hashed search would seem to be overkill, at least for now. (If hooking into libpq is truly successful we can always improve it later -- it's not an exported detail of the API after all.) -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Alvaro Herrera wrote: > Andrew Dunstan escribió: > > >> The thing that is a bit disturbing is that the whole style of this >> scheme is very different from the fairly simple APIs that the rest of >> libpq presents. It's going to make libpq look rather odd, I think. I >> would have felt happier if the authors had been able to come up with a >> simple scheme to add API calls to export whatever information they >> needed, rather than using this callback scheme. >> > > I'm not sure I understand this point. Remember that this is here to > support the libpqtypes library. There doesn't seem to be a way for an > API such as you describe to work. > That might well be true. The issue then becomes "Do we want to add something with this flavor to libpq?" I take it Bruce's answer is "No", at least until he has seen more evidence of general usefulness. I think we need to make a decision on this before anyone wastes any more time. It should be noted that while this feels slightly foreign, it isn't hugely invasive, unlike the previous effort - it's only a few hundred lines of new code. If we reject this, presumably the authors will have no alternative than to offer libpqtypes as a patch to libpq. ISTM that we're then asking them to climb over a fairly high hurdle. On the one hand we want them to demonstrate that there's demand for their tool and on the other we make it difficult to distribute and deploy. > >> Second, the hook names are compared case insensitively and by linear >> search. I don't see any justification for using case insensitive names >> for hooks in a C program, so I think that part should go. And if we >> expect to keep anything other than trivial numbers of hooks we should >> look at some sort of binary or hashed search. >> > > Keep in mind that the original patch supported a single hook being > registered. Perhaps we could dream about having a couple of hooks > registered, but turning into hashed search would seem to be overkill, at > least for now. (If hooking into libpq is truly successful we can always > improve it later -- it's not an exported detail of the API after all.) > > Right, it was more the case insensitive part that bothered me. cheers andrew
On Wed, May 14, 2008 at 8:18 AM, Andrew Dunstan <andrew@dunslane.net> wrote: > Right, it was more the case insensitive part that bothered me. Done. We in fact had realized this was a mistake anyways following some profiling of the libpqtypes library. In some scenarios, this function gets called a lot. Regarding the other comments: *) API structure: Our major objective was to minimize exports to libpq. I think both copyresult and setvalue have some possible sideband usage (footguns or no). Additional functions could be speculated but are not required by libpqtypes. We would have no problem adding a few things to complete the api if necessary. The patch is basically the minimum libpqtypes needs and has to work more or less as written. We tried a few times to suggest implementing the split a different way (basically, more invasion into libpq). We couldn't get any action there. If the patch is rejected on general merits...that signals the death blow for libpqtypes. We have a chicken/egg problem...people can't use it without patching libpq which will really hamper its adoption, which is, uh, needed to justify the patch. For the record, we have had a couple of dozen downloads of the libpqtypes library on pgfoundry since we put it up there last week. Based on how it has simplified and improved our own code vs. libpq, we have absolutely no doubts it is a major improvement over PQexecParams. merlin
Andrew Dunstan <andrew@dunslane.net> writes:
> It should be noted that while this feels slightly foreign, it isn't
> hugely invasive, unlike the previous effort - it's only a few hundred
> lines of new code.
> If we reject this, presumably the authors will have no alternative than
> to offer libpqtypes as a patch to libpq.
No, they could revise their patch to be more stylistically in keeping
with libpq. I haven't looked at the current version of the patch yet,
but the early versions seemed quite overengineered to me, so your
criticism didn't surprise me.
>> Keep in mind that the original patch supported a single hook being
>> registered.
> Right, it was more the case insensitive part that bothered me.
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.
regards, tom lane
On Tue, May 13, 2008 at 11:52 PM, Bruce Momjian <bruce@momjian.us> wrote: > My personal opinion is still that I would like to see a more general > usefulness for these functions before adding them to libpq. The > complexity of the API just mirrors my gut feeling on this. There has been a lot of demand for the features that libpqtypes provides...a quick search of the archives demonstrates this. Here are a few examples of threads from users asking or even trying to implement some of the things that libpqtypes helps with or indirectly solves...I am speaking to your point of usefulness here. [BUGS] BUG #4053: libpq documentation should express clearly, that integers are passed in network octet order [PATCHES] [PATCH] automatic integer conversion [HACKERS] convert int to bytea [HACKERS] comunication protocol [HACKERS] PQescapeBytea* version for parameters [HACKERS] libpq and Binary Data Formats [GENERAL] binary representation of date and numeric [GENERAL] binding 64-bit integer [HACKERS] Last minute mini-proposal (I know, I know) for PQexecf() [PERFORM] Low throughput of binary inserts from windows to linux [GENERAL] Storing images as BYTEA or large objects merlin
Merlin Moncure wrote: > Regarding the other comments: > *) API structure: Our major objective was to minimize exports to > libpq. I think both copyresult and setvalue have some possible > sideband usage (footguns or no). Additional functions could be > speculated but are not required by libpqtypes. We would have no > problem adding a few things to complete the api if necessary. > > The patch is basically the minimum libpqtypes needs and has to work > more or less as written. We tried a few times to suggest implementing > the split a different way (basically, more invasion into libpq). We > couldn't get any action there. > > If the patch is rejected on general merits...that signals the death > blow for libpqtypes. We have a chicken/egg problem...people can't use > it without patching libpq which will really hamper its adoption, which > is, uh, needed to justify the patch. For the record, we have had a > couple of dozen downloads of the libpqtypes library on pgfoundry since > we put it up there last week. Based on how it has simplified and > improved our own code vs. libpq, we have absolutely no doubts it is a > major improvement over PQexecParams. One idea would be to add the libpq hooks but not document them. This way, we can modify or remove the API as needed in the future. As libpqtypes matures and we are sure what the API should be, we can document it as stable and permanent. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
On Wed, May 14, 2008 at 10:44 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > No, they could revise their patch to be more stylistically in keeping > with libpq. I haven't looked at the current version of the patch yet, > but the early versions seemed quite overengineered to me, so your > criticism didn't surprise me. I think you'll find the latest version more reasonable. We tried to keep the over-engineering, so to speak, on our side and make the libpq changes surgical. > 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. Here are the proposed API changes [aside: this is one of two breaks from your initial suggestions..the other being PQcopyResult]: + PQcopyResult 142 + PQsetvalue 143 + PQresultAlloc 144 + PQaddObjectHooks 145 + PQaddGlobalObjectHooks 146 + PQhookData 147 + PQresultHookData 148 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. merlin
On Wed, May 14, 2008 at 10:52 AM, Bruce Momjian <bruce@momjian.us> wrote: > Merlin Moncure wrote: >> Regarding the other comments: >> *) API structure: Our major objective was to minimize exports to >> libpq. I think both copyresult and setvalue have some possible >> sideband usage (footguns or no). Additional functions could be >> speculated but are not required by libpqtypes. We would have no >> problem adding a few things to complete the api if necessary. >> >> The patch is basically the minimum libpqtypes needs and has to work >> more or less as written. We tried a few times to suggest implementing >> the split a different way (basically, more invasion into libpq). We >> couldn't get any action there. >> >> If the patch is rejected on general merits...that signals the death >> blow for libpqtypes. We have a chicken/egg problem...people can't use >> it without patching libpq which will really hamper its adoption, which >> is, uh, needed to justify the patch. For the record, we have had a >> couple of dozen downloads of the libpqtypes library on pgfoundry since >> we put it up there last week. Based on how it has simplified and >> improved our own code vs. libpq, we have absolutely no doubts it is a >> major improvement over PQexecParams. > > One idea would be to add the libpq hooks but not document them. This > way, we can modify or remove the API as needed in the future. As > libpqtypes matures and we are sure what the API should be, we can > document it as stable and permanent. The API functions relating to hooks are unlikely to change once settled on...but PQsetvalue/PQcopyResult are a good fit with your idea (they introduce new behaviors that could possibly be used outside of libpqtypes context, and could require changes down the line). merlin
>> 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/
On Wed, May 14, 2008 at 10:44 AM, Tom Lane <tgl@sss.pgh.pa.us> 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
Merlin Moncure wrote: > On Wed, May 14, 2008 at 10:44 AM, Tom Lane <tgl@sss.pgh.pa.us> 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/
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 -0000 1.19
--- exports.txt 14 May 2008 17:47:59 -0000
***************
*** 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
+ PQsetvalue 143
+ PQresultAlloc 144
+ PQaddObjectHooks 145
+ PQaddGlobalObjectHooks 146
+ PQhookData 147
+ 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.c 31 Mar 2008 02:43:14 -0000 1.357
--- fe-connect.c 14 May 2008 17:47:59 -0000
***************
*** 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;
char sebuf[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 error_return;
}
/* We are open for business! */
conn->status = CONNECTION_OK;
+ pqInitObjectHooks(conn);
return PGRES_POLLING_OK;
default:
printfPQExpBuffer(&conn->errorMessage,
libpq_gettext(
"invalid connection state %c, "
***************
*** 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)
--- 1878,1889 ----
***************
*** 1970,1981 ****
--- 1972,2001 ----
* 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);
***************
*** 2151,2164 ****
PQreset(PGconn *conn)
{
if (conn)
{
closePGconn(conn);
! if (connectDBStart(conn))
! (void) connectDBComplete(conn);
}
}
/*
* PQresetStart:
--- 2171,2189 ----
PQreset(PGconn *conn)
{
if (conn)
{
closePGconn(conn);
! if (connectDBStart(conn) && connectDBComplete(conn))
! {
! int i;
! 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;
}
-
/*
* PQresetPoll:
* resets the connection to the backend
* closes the existing connection and makes a new one
*/
PostgresPollingStatusType
PQresetPoll(PGconn *conn)
{
if (conn)
! return PQconnectPoll(conn);
return PGRES_POLLING_FAILED;
}
/*
* PQcancelGet: get a PGcancel structure corresponding to a connection.
--- 2201,2234 ----
return connectDBStart(conn);
}
return 0;
}
/*
* PQresetPoll:
* resets the connection to the backend
* closes the existing connection and makes a new one
*/
PostgresPollingStatusType
PQresetPoll(PGconn *conn)
{
if (conn)
! {
! PostgresPollingStatusType status = PQconnectPoll(conn);
!
! if(status == PGRES_POLLING_OK)
! {
! int i;
! for(i=0; i < conn->objHooksCount; i++)
! if(conn->objHooks[i].connReset)
! conn->objHooks[i].connReset((const PGconn *)conn);
! }
!
! return status;
! }
return PGRES_POLLING_FAILED;
}
/*
* PQcancelGet: get a PGcancel structure corresponding to a connection.
***************
*** 3855,3860 ****
--- 3891,3897 ----
pg_g_threadlock = newhandler;
else
pg_g_threadlock = default_threadlock;
return prev;
}
+
Index: fe-exec.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-exec.c,v
retrieving revision 1.194
diff -C6 -r1.194 fe-exec.c
*** fe-exec.c 1 Jan 2008 19:46:00 -0000 1.194
--- fe-exec.c 14 May 2008 17:48:00 -0000
***************
*** 60,72 ****
int resultFormat);
static void parseInput(PGconn *conn);
static bool PQexecStart(PGconn *conn);
static PGresult *PQexecFinish(PGconn *conn);
static int PQsendDescribe(PGconn *conn, char desc_type,
const char *desc_target);
!
/* ----------------
* Space management for PGresult.
*
* Formerly, libpq did a separate malloc() for each field of each tuple
* returned by a query. This was remarkably expensive --- malloc/free
--- 60,72 ----
int resultFormat);
static void parseInput(PGconn *conn);
static bool PQexecStart(PGconn *conn);
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.
*
* Formerly, libpq did a separate malloc() for each field of each tuple
* returned by a query. This was remarkably expensive --- malloc/free
***************
*** 121,141 ****
--- 121,170 ----
#define PGRESULT_DATA_BLOCKSIZE 2048
#define PGRESULT_ALIGN_BOUNDARY MAXIMUM_ALIGNOF /* from configure */
#define PGRESULT_BLOCK_OVERHEAD Max(sizeof(PGresult_data), PGRESULT_ALIGN_BOUNDARY)
#define PGRESULT_SEP_ALLOC_THRESHOLD (PGRESULT_DATA_BLOCKSIZE / 2)
+ /* Does not duplicate the hook data, sets this to NULL */
+ static PGobjectHooks *
+ dupObjectHooks(PGobjectHooks *hooks, int count)
+ {
+ int i;
+ PGobjectHooks *newHooks;
+
+ if(!hooks || count <= 0)
+ return NULL;
+
+ newHooks = (PGobjectHooks *)malloc(count * sizeof(PGobjectHooks));
+ if(!newHooks)
+ return NULL;
+
+ memcpy(newHooks, hooks, count * sizeof(PGobjectHooks));
+
+ /* NULL out the hook data pointer */
+ for(i=0; i < count; i++)
+ {
+ newHooks[i].data = NULL;
+ newHooks[i].name = strdup(hooks[i].name);
+ }
+
+ return newHooks;
+ }
+
/*
* PQmakeEmptyPGresult
* returns a newly allocated, initialized PGresult with given status.
* If conn is not NULL and status indicates an error, the conn's
* errorMessage is copied.
*
* Note this is exported --- you wouldn't think an application would need
* to build its own PGresults, but this has proven useful in both libpgtcl
* and the Perl5 interface, so maybe it's not so unreasonable.
+ *
+ * Updated April 2008 - If conn is not NULL, ObjectHooks will be copied
+ * from the PGconn to the created PGresult.
*/
PGresult *
PQmakeEmptyPGresult(PGconn *conn, ExecStatusType status)
{
PGresult *result;
***************
*** 157,175 ****
--- 186,219 ----
result->errMsg = NULL;
result->errFields = NULL;
result->null_field[0] = '\0';
result->curBlock = NULL;
result->curOffset = 0;
result->spaceLeft = 0;
+ result->objHooksCount = 0;
+ result->objHooks = NULL;
if (conn)
{
/* copy connection data we might need for operations on PGresult */
result->noticeHooks = conn->noticeHooks;
result->client_encoding = conn->client_encoding;
+ /* copy object hooks from connection */
+ if(conn->objHooksCount > 0)
+ {
+ result->objHooks = dupObjectHooks(conn->objHooks, conn->objHooksCount);
+ if(!result->objHooks)
+ {
+ PQclear(result);
+ return NULL;
+ }
+
+ result->objHooksCount = conn->objHooksCount;
+ }
+
/* consider copying conn's errorMessage */
switch (status)
{
case PGRES_EMPTY_QUERY:
case PGRES_COMMAND_OK:
case PGRES_TUPLES_OK:
***************
*** 193,204 ****
--- 237,476 ----
}
return result;
}
/*
+ * PQcopyResult
+ * Returns a deep copy of the provided 'src' PGresult, which cannot be NULL.
+ * The 'options' argument controls which portions of the result will or will
+ * NOT be copied. If this value is 0, the entire result is deep copied.
+ * The created result is always put into the PGRES_TUPLES_OK status. The
+ * source result error message is not copied, although cmdStatus is.
+ *
+ * Options:
+ * PG_COPYRES_NO_TUPLES - Do not copy the tuples. This option is
+ * automatically enabled when PG_COPYRES_USE_ATTRS is set.
+ *
+ * PG_COPYRES_USE_ATTRS - Indicates that the 'numAttributes' and 'attDescs'
+ * arguments should be used as the fields for the created result, rather
+ * than copying them from the source result. When this option is set,
+ * the tuples will NOT be copied; behaving identically to setting the
+ * PG_COPYRES_NO_TUPLES option. One must use PQsetvalue to manually
+ * add tuples to the returned result. NOTE: numAttributes and attDescs
+ * arguments are ignored unless this option is set!
+ *
+ * PG_COPYRES_NO_OBJECTHOOKS - Indicates that the source result's
+ * ObjectHooks should NOT be copied to the created result.
+ *
+ * PG_COPYRES_NO_NOTICEHOOKS - Indicates that the source result's
+ * NoticeHooks should NOT be copied to the created result.
+ */
+
+ PGresult *
+ PQcopyResult(const PGresult *src, int numAttributes,
+ PGresAttDesc *attDescs, int options)
+ {
+ int i;
+ PGresult *dest;
+
+ if(!src)
+ return NULL;
+
+ /* Automatically turn on no_tuples since use_attrs is set. It makes
+ * no sense to copy tuples into an unknown set of columns.
+ */
+ if(options & PG_COPYRES_USE_ATTRS)
+ options |= PG_COPYRES_NO_TUPLES;
+
+ /* If use_attrs is set, verify attr arguments. */
+ if((options & PG_COPYRES_USE_ATTRS) && (numAttributes <= 0 || !attDescs))
+ return NULL;
+
+ dest = PQmakeEmptyPGresult((PGconn *)NULL, PGRES_TUPLES_OK);
+ if(!dest)
+ return NULL;
+
+ /* always copy these over. Is cmdStatus useful here? */
+ dest->client_encoding = src->client_encoding;
+ strcpy(dest->cmdStatus, src->cmdStatus);
+
+ /* Wants to copy notice hooks */
+ if(!(options & PG_COPYRES_NO_NOTICEHOOKS))
+ dest->noticeHooks = src->noticeHooks;
+
+ /* Copy from src result when not supplying attrs */
+ if(!(options & PG_COPYRES_USE_ATTRS) && src->numAttributes > 0)
+ {
+ numAttributes = src->numAttributes;
+ attDescs = src->attDescs;
+ }
+
+ /* copy attrs */
+ if(numAttributes > 0)
+ {
+ dest->attDescs = (PGresAttDesc *)PQresultAlloc(dest,
+ numAttributes * sizeof(PGresAttDesc));
+
+ if(!dest->attDescs)
+ {
+ PQclear(dest);
+ return NULL;
+ }
+
+ dest->numAttributes = numAttributes;
+ memcpy(dest->attDescs, attDescs, numAttributes * sizeof(PGresAttDesc));
+
+ /* resultalloc the attribute names. The above memcpy has the attr
+ * names pointing at the source result's private memory (or at the
+ * callers provided attDescs memory).
+ */
+ dest->binary = 1;
+ for(i=0; i < numAttributes; i++)
+ {
+ if(attDescs[i].name)
+ dest->attDescs[i].name = pqResultStrdup(dest, attDescs[i].name);
+ else
+ dest->attDescs[i].name = dest->null_field;
+
+ if(!dest->attDescs[i].name)
+ {
+ PQclear(dest);
+ return NULL;
+ }
+
+ /* Although deprecated, because results can have text+binary columns,
+ * its easy enough to deduce so set it for completeness.
+ */
+ if(dest->attDescs[i].format == 0)
+ dest->binary = 0;
+ }
+ }
+
+ /* Wants to copy result tuples: use PQsetvalue(). */
+ if(!(options & PG_COPYRES_NO_TUPLES) && src->ntups > 0)
+ {
+ int tup, field;
+ for(tup=0; tup < src->ntups; tup++)
+ for(field=0; field < src->numAttributes; field++)
+ PQsetvalue(dest, tup, field, src->tuples[tup][field].value,
+ src->tuples[tup][field].len);
+ }
+
+ /* Wants to copy objHooks. */
+ if(!(options & PG_COPYRES_NO_OBJECTHOOKS) && src->objHooksCount > 0)
+ {
+ dest->objHooks = dupObjectHooks(src->objHooks, src->objHooksCount);
+ if(!dest->objHooks)
+ {
+ PQclear(dest);
+ return NULL;
+ }
+
+ dest->objHooksCount = src->objHooksCount;
+ }
+
+ /* Invoke resultCopy for each Object Hook */
+ for(i=0; i < dest->objHooksCount; i++)
+ if(dest->objHooks[i].resultCopy)
+ dest->objHooks[i].data = dest->objHooks[i].resultCopy(dest, src);
+
+ return dest;
+ }
+
+ int
+ PQsetvalue(PGresult *res, int tup_num, int field_num,
+ char *value, int len)
+ {
+ PGresAttValue *attval;
+
+ if(!check_field_number(res, field_num))
+ return FALSE;
+
+ /* Invalid tup_num, must be <= ntups */
+ if(tup_num > res->ntups)
+ return FALSE;
+
+ /* need to grow the tuple table */
+ if(res->ntups >= res->tupArrSize)
+ {
+ int n = res->tupArrSize ? (res->tupArrSize*3)/2 : 64;
+ PGresAttValue **tups = (PGresAttValue **)
+ (res->tuples ? realloc(res->tuples, n*sizeof(PGresAttValue *)) :
+ malloc(n*sizeof(PGresAttValue *)));
+
+ if(!tups)
+ return FALSE;
+
+ memset(tups + res->tupArrSize, 0,
+ (n - res->tupArrSize) * sizeof(PGresAttValue *));
+ res->tuples = tups;
+ res->tupArrSize = n;
+ }
+
+ /* need to allocate a new tuple */
+ if(tup_num == res->ntups && !res->tuples[tup_num])
+ {
+ int i;
+ PGresAttValue *tup = (PGresAttValue *)PQresultAlloc(
+ res, res->numAttributes * sizeof(PGresAttValue));
+
+ if(!tup)
+ return FALSE;
+
+ /* initialize each column to NULL */
+ for(i=0; i < res->numAttributes; i++)
+ {
+ tup[i].len = NULL_LEN;
+ tup[i].value = res->null_field;
+ }
+
+ res->tuples[tup_num] = tup;
+ res->ntups++;
+ }
+
+ attval = &res->tuples[tup_num][field_num];
+
+ /* On top of NULL_LEN, treat a NULL value as a NULL field */
+ if(len == NULL_LEN || value == NULL)
+ {
+ attval->len = NULL_LEN;
+ attval->value = res->null_field;
+ }
+ else
+ {
+ if(len < 0)
+ len = 0;
+
+ if(len == 0)
+ {
+ attval->len = 0;
+ attval->value = res->null_field;
+ }
+ else
+ {
+ attval->value = (char *)PQresultAlloc(res, len + 1);
+ if(!attval->value)
+ return FALSE;
+
+ attval->len = len;
+ memcpy(attval->value, value, len);
+ attval->value[len] = '\0';
+ }
+ }
+
+ return TRUE;
+ }
+
+ void *
+ PQresultAlloc(PGresult *res, size_t nBytes)
+ {
+ return pqResultAlloc(res, nBytes, TRUE);
+ }
+
+ /*
* pqResultAlloc -
* Allocate subsidiary storage for a PGresult.
*
* nBytes is the amount of space needed for the object.
* If isBinary is true, we assume that we need to align the object on
* a machine allocation boundary.
***************
*** 349,365 ****
--- 621,654 ----
* PQclear -
* free's the memory associated with a PGresult
*/
void
PQclear(PGresult *res)
{
+ int i;
PGresult_data *block;
if (!res)
return;
+ for(i=0; i < res->objHooksCount; i++)
+ {
+ if(res->objHooks[i].resultDestroy)
+ {
+ res->objHooks[i].resultDestroy((const PGresult *)res);
+ free(res->objHooks[i].name);
+ }
+ }
+
+ if(res->objHooks)
+ {
+ free(res->objHooks);
+ res->objHooks = NULL;
+ res->objHooksCount = 0;
+ }
+
/* Free all the subsidiary blocks */
while ((block = res->curBlock) != NULL)
{
res->curBlock = block->next;
free(block);
}
***************
*** 1179,1202 ****
parseInput(conn);
/* PQgetResult will return immediately in all states except BUSY. */
return conn->asyncStatus == PGASYNC_BUSY;
}
-
/*
* PQgetResult
* Get the next PGresult produced by a query. Returns NULL if no
* query work remains or an error has occurred (e.g. out of
* memory).
*/
PGresult *
PQgetResult(PGconn *conn)
{
! PGresult *res;
if (!conn)
return NULL;
/* Parse any available data, if our state permits. */
parseInput(conn);
--- 1468,1490 ----
parseInput(conn);
/* PQgetResult will return immediately in all states except BUSY. */
return conn->asyncStatus == PGASYNC_BUSY;
}
/*
* PQgetResult
* Get the next PGresult produced by a query. Returns NULL if no
* query work remains or an error has occurred (e.g. out of
* memory).
*/
PGresult *
PQgetResult(PGconn *conn)
{
! PGresult *res=NULL;
if (!conn)
return NULL;
/* Parse any available data, if our state permits. */
parseInput(conn);
***************
*** 1265,1276 ****
--- 1553,1574 ----
libpq_gettext("unexpected asyncStatus: %d\n"),
(int) conn->asyncStatus);
res = PQmakeEmptyPGresult(conn, PGRES_FATAL_ERROR);
break;
}
+ if(res && (res->resultStatus == PGRES_COMMAND_OK ||
+ res->resultStatus == PGRES_TUPLES_OK))
+ {
+ int i;
+ for(i=0; i < res->objHooksCount; i++)
+ if(res->objHooks[i].resultCreate)
+ res->objHooks[i].data = res->objHooks[i].resultCreate(
+ (const PGconn *)conn, (const PGresult *)res);
+ }
+
return res;
}
/*
* PQexec
***************
*** 1291,1303 ****
if (!PQsendQuery(conn, query))
return NULL;
return PQexecFinish(conn);
}
/*
! * PQexecParams
* Like PQexec, but use protocol 3.0 so we can pass parameters
*/
PGresult *
PQexecParams(PGconn *conn,
const char *command,
int nParams,
--- 1589,1601 ----
if (!PQsendQuery(conn, query))
return NULL;
return PQexecFinish(conn);
}
/*
! *
* Like PQexec, but use protocol 3.0 so we can pass parameters
*/
PGresult *
PQexecParams(PGconn *conn,
const char *command,
int nParams,
Index: fe-misc.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-misc.c,v
retrieving revision 1.133
diff -C6 -r1.133 fe-misc.c
*** fe-misc.c 1 Jan 2008 19:46:00 -0000 1.133
--- fe-misc.c 14 May 2008 17:48:00 -0000
***************
*** 1152,1157 ****
--- 1152,1302 ----
}
return dgettext("libpq", msgid);
}
#endif /* ENABLE_NLS */
+
+
+ /* ---------------------
+ * ObjectHooks
+ */
+
+ /* global object hooks, see PQaddGlobalObjectHooks */
+ static int g_objHooksCount = 0;
+ static int g_objHooksSize = 0;
+ static PGobjectHooks *g_objHooks = NULL;
+
+ static int
+ hookExists(PGobjectHooks *objHooks, int objHooksCount, const char *name)
+ {
+ int i;
+ for(i=0; i < objHooksCount; i++)
+ if(strcmp(objHooks[i].name, name)==0)
+ return TRUE;
+ return FALSE;
+ }
+
+ static int
+ growHooks(PGobjectHooks **objHooks, int *objHooksSize,
+ int objHooksCount)
+ {
+ /* grow list */
+ if(objHooksCount == *objHooksSize)
+ {
+ PGobjectHooks *oh;
+ int n = *objHooksSize ? (*objHooksSize*3)/2 : 4;
+
+ if(*objHooks)
+ oh = (PGobjectHooks *)realloc(
+ *objHooks, n*sizeof(PGobjectHooks));
+ else
+ oh = (PGobjectHooks *)malloc(n*sizeof(PGobjectHooks));
+
+ if(!oh)
+ return FALSE;
+
+ *objHooks = oh;
+ *objHooksSize = n;
+ }
+
+ return TRUE;
+ }
+
+ static int
+ hookCopy(PGobjectHooks *dst, PGobjectHooks *src)
+ {
+ memcpy(dst, src, sizeof(PGobjectHooks));
+ dst->data = NULL;
+ dst->name = strdup(src->name);
+ return dst->name ? TRUE : FALSE;
+ }
+
+ int
+ PQaddGlobalObjectHooks(PGobjectHooks *hooks)
+ {
+ if(!hooks || !hooks->name || !*hooks->name)
+ return FALSE;
+
+ if(hookExists(g_objHooks, g_objHooksCount, hooks->name) ||
+ !growHooks(&g_objHooks, &g_objHooksSize, g_objHooksCount) ||
+ !hookCopy(g_objHooks + g_objHooksCount, hooks))
+ return FALSE;
+
+ g_objHooksCount++;
+ return TRUE;
+ }
+
+ /* inits the global hooks for a conn object. If the hooks were already
+ * installed, the request is ignored and a success value is returned.
+ * This will happen during a PQreset and PQresetPoll.
+ * Returns non-zero for success and zero on error.
+ */
+ int
+ pqInitObjectHooks(PGconn *conn)
+ {
+ int i;
+
+ /* Already installed, occurs on PQreset and PQresetPoll */
+ if(conn->objHooksCount > 0)
+ return TRUE;
+
+ for(i=0; i < g_objHooksCount; i++)
+ if(!PQaddObjectHooks(conn, &g_objHooks[i]))
+ return FALSE;
+
+ return TRUE;
+ }
+
+ int
+ PQaddObjectHooks(PGconn *conn, PGobjectHooks *hooks)
+ {
+ PGobjectHooks *oh;
+
+ if(!conn || !hooks || !hooks->name || !*hooks->name)
+ return FALSE;
+
+ /* names must be unique */
+ if(hookExists(conn->objHooks, conn->objHooksCount, hooks->name) ||
+ !growHooks(&conn->objHooks, &conn->objHooksSize, conn->objHooksCount) ||
+ !hookCopy(conn->objHooks + conn->objHooksCount, hooks))
+ return FALSE;
+
+ oh = &conn->objHooks[conn->objHooksCount++];
+ if(oh->initHookData)
+ oh->data = oh->initHookData((const PGconn *)conn);
+
+ return TRUE;
+ }
+
+ void *
+ PQhookData(const PGconn *conn, const char *hookName)
+ {
+ if(conn && hookName && *hookName)
+ {
+ int i;
+ PGobjectHooks *hooks = conn->objHooks;
+
+ for(i=0; i < conn->objHooksCount; i++)
+ if(strcmp(hooks[i].name, hookName)==0)
+ return hooks[i].data;
+ }
+
+ return NULL;
+ }
+
+ void *
+ PQresultHookData(const PGresult *res, const char *hookName)
+ {
+ if(res && hookName && *hookName)
+ {
+ int i;
+ PGobjectHooks *hooks = res->objHooks;
+
+ for(i=0; i < res->objHooksCount; i++)
+ if(strcmp(hooks[i].name, hookName)==0)
+ return hooks[i].data;
+ }
+
+ return NULL;
+ }
+
Index: libpq-fe.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/libpq-fe.h,v
retrieving revision 1.142
diff -C6 -r1.142 libpq-fe.h
*** libpq-fe.h 19 Mar 2008 00:39:33 -0000 1.142
--- libpq-fe.h 14 May 2008 17:48:00 -0000
***************
*** 25,36 ****
--- 25,45 ----
/*
* postgres_ext.h defines the backend's externally visible types,
* such as Oid.
*/
#include "postgres_ext.h"
+ /* -----------------------
+ * Options for PQcopyResult
+ */
+
+ #define PG_COPYRES_NO_TUPLES 0x01
+ #define PG_COPYRES_USE_ATTRS 0x02
+ #define PG_COPYRES_NO_OBJECTHOOKS 0x04
+ #define PG_COPYRES_NO_NOTICEHOOKS 0x08
+
/* Application-visible enum types */
typedef enum
{
/*
* Although it is okay to add to this list, values which become unused
***************
*** 190,201 ****
--- 199,260 ----
int *ptr; /* can't use void (dec compiler barfs) */
int integer;
} u;
} PQArgBlock;
/* ----------------
+ * PGresAttDesc -- Data about a single attribute (column) of a query result
+ * ----------------
+ */
+ typedef struct pgresAttDesc
+ {
+ char *name; /* column name */
+ Oid tableid; /* source table, if known */
+ int columnid; /* source column, if known */
+ int format; /* format code for value (text/binary) */
+ Oid typid; /* type id */
+ int typlen; /* type size */
+ int atttypmod; /* type-specific modifier info */
+ } PGresAttDesc;
+
+ /* ----------------
+ * PGobjectHooks -- structure for adding libpq object hooks
+ * Monitors the creation and deletion of objects.
+ * ----------------
+ */
+
+ typedef struct
+ {
+ char *name;
+
+ 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;
+
+ /* ----------------
* Exported functions of libpq
* ----------------
*/
/* === in fe-connect.c === */
***************
*** 434,445 ****
--- 493,523 ----
* Make an empty PGresult with given status (some apps find this
* useful). If conn is not NULL and status indicates an error, the
* conn's errorMessage is copied.
*/
extern PGresult *PQmakeEmptyPGresult(PGconn *conn, ExecStatusType status);
+ extern PGresult *
+ PQcopyResult(const PGresult *src, int numAttributes,
+ PGresAttDesc *attDescs, int options);
+
+ extern void *
+ PQresultAlloc(PGresult *res, size_t nBytes);
+
+ /*
+ * Sets the value for a tuple field. The tup_num must be less than or
+ * equal to PQntuples(res). This function will generate tuples as needed.
+ * A new tuple is generated when tup_num equals PQntuples(res) and there
+ * are no fields defined for that tuple.
+ *
+ * Returns a non-zero value for success and zero for failure.
+ */
+ extern int
+ PQsetvalue(PGresult *res, int tup_num, int field_num,
+ char *value, int len);
+
/* Quoting strings before inclusion in queries. */
extern size_t PQescapeStringConn(PGconn *conn,
char *to, const char *from, size_t length,
int *error);
extern unsigned char *PQescapeByteaConn(PGconn *conn,
***************
*** 506,517 ****
--- 584,617 ----
/* Determine display length of multibyte encoded char at *s */
extern int PQdsplen(const char *s, int encoding);
/* Get encoding id from environment variable PGCLIENTENCODING */
extern int PQenv2encoding(void);
+ /* Adds a set of global object hooks, which will be inherited by every
+ * PGconn. Object hooks added in this fashion do not require being
+ * added on a per-connection basis. Any number of object hooks can be
+ * be added.
+ *
+ * WARNING: This should only be called in the application's main thread
+ * before using any other libpq functions. This is not thread-safe and
+ * should be used prior to creating any application threads.
+ */
+ extern int
+ PQaddGlobalObjectHooks(PGobjectHooks *hooks);
+
+ /* Adds a set of object hooks to the given connection. */
+ extern int
+ PQaddObjectHooks(PGconn *conn, PGobjectHooks *hooks);
+
+ extern void *
+ PQhookData(const PGconn *conn, const char *hookName);
+
+ extern void *
+ PQresultHookData(const PGresult *res, const char *hookName);
+
/* === in fe-auth.c === */
extern char *PQencryptPassword(const char *passwd, const char *user);
/* === in encnames.c === */
Index: libpq-int.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/libpq-int.h,v
retrieving revision 1.129
diff -C6 -r1.129 libpq-int.h
*** libpq-int.h 1 Jan 2008 19:46:00 -0000 1.129
--- libpq-int.h 14 May 2008 17:48:00 -0000
***************
*** 97,121 ****
union pgresult_data
{
PGresult_data *next; /* link to next block, or NULL */
char space[1]; /* dummy for accessing block as bytes */
};
- /* Data about a single attribute (column) of a query result */
-
- typedef struct pgresAttDesc
- {
- char *name; /* column name */
- Oid tableid; /* source table, if known */
- int columnid; /* source column, if known */
- int format; /* format code for value (text/binary) */
- Oid typid; /* type id */
- int typlen; /* type size */
- int atttypmod; /* type-specific modifier info */
- } PGresAttDesc;
-
/* Data about a single parameter of a prepared statement */
typedef struct pgresParamDesc
{
Oid typid; /* type id */
} PGresParamDesc;
--- 97,108 ----
***************
*** 181,192 ****
--- 168,183 ----
* These fields are copied from the originating PGconn, so that operations
* on the PGresult don't have to reference the PGconn.
*/
PGNoticeHooks noticeHooks;
int client_encoding; /* encoding id */
+ /* Object Hooks */
+ int objHooksCount;
+ PGobjectHooks *objHooks;
+
/*
* Error information (all NULL if not an error result). errMsg is the
* "overall" error message returned by PQresultErrorMessage. If we have
* per-field info then it is stored in a linked list.
*/
char *errMsg; /* error message, or NULL if no error */
***************
*** 202,213 ****
--- 193,205 ----
*/
PGresult_data *curBlock; /* most recently allocated block */
int curOffset; /* start offset of free space in block */
int spaceLeft; /* number of free bytes remaining in block */
};
+
/* PGAsyncStatusType defines the state of the query-execution state machine */
typedef enum
{
PGASYNC_IDLE, /* nothing's happening, dude */
PGASYNC_BUSY, /* query in progress */
PGASYNC_READY, /* result ready for PQgetResult */
***************
*** 300,311 ****
--- 292,308 ----
/* Optional file to write trace info to */
FILE *Pfdebug;
/* Callback procedures for notice message processing */
PGNoticeHooks noticeHooks;
+ /* Object Hooks */
+ int objHooksCount;
+ int objHooksSize;
+ PGobjectHooks *objHooks;
+
/* Status indicators */
ConnStatusType status;
PGAsyncStatusType asyncStatus;
PGTransactionStatusType xactStatus; /* never changes to ACTIVE */
PGQueryClass queryclass;
char *last_query; /* last SQL command, or NULL if unknown */
***************
*** 516,531 ****
extern int pqPutInt(int value, size_t bytes, PGconn *conn);
extern int pqPutMsgStart(char msg_type, bool force_len, PGconn *conn);
extern int pqPutMsgEnd(PGconn *conn);
extern int pqReadData(PGconn *conn);
extern int pqFlush(PGconn *conn);
extern int pqWait(int forRead, int forWrite, PGconn *conn);
! extern int pqWaitTimed(int forRead, int forWrite, PGconn *conn,
time_t finish_time);
extern int pqReadReady(PGconn *conn);
extern int pqWriteReady(PGconn *conn);
/* === in fe-secure.c === */
extern int pqsecure_initialize(PGconn *);
extern void pqsecure_destroy(void);
extern PostgresPollingStatusType pqsecure_open_client(PGconn *);
--- 513,529 ----
extern int pqPutInt(int value, size_t bytes, PGconn *conn);
extern int pqPutMsgStart(char msg_type, bool force_len, PGconn *conn);
extern int pqPutMsgEnd(PGconn *conn);
extern int pqReadData(PGconn *conn);
extern int pqFlush(PGconn *conn);
extern int pqWait(int forRead, int forWrite, PGconn *conn);
! extern int pqWaitTimed(int forRead, int forWrite, PGconn *conn,
time_t finish_time);
extern int pqReadReady(PGconn *conn);
extern int pqWriteReady(PGconn *conn);
+ extern int pqInitObjectHooks(PGconn *conn);
/* === in fe-secure.c === */
extern int pqsecure_initialize(PGconn *);
extern void pqsecure_destroy(void);
extern PostgresPollingStatusType pqsecure_open_client(PGconn *);
On Wed, May 14, 2008 at 10:52:43AM -0400, Bruce Momjian wrote: > > One idea would be to add the libpq hooks but not document them. This > way, we can modify or remove the API as needed in the future. As > libpqtypes matures and we are sure what the API should be, we can > document it as stable and permanent. -1 Perhaps it is just me, but undocumented interface are evil. Simply document it with the changable bits labled as such. -dg -- David Gould daveg@sonic.net 510 536 1443 510 282 0869 If simplicity worked, the world would be overrun with insects.
On Wed, May 14, 2008 at 3:47 PM, daveg <daveg@sonic.net> wrote: > On Wed, May 14, 2008 at 10:52:43AM -0400, Bruce Momjian wrote: >> >> One idea would be to add the libpq hooks but not document them. This >> way, we can modify or remove the API as needed in the future. As >> libpqtypes matures and we are sure what the API should be, we can >> document it as stable and permanent. > > Perhaps it is just me, but undocumented interface are evil. Simply document > it with the changable bits labled as such. Well, in defense of Bruce, there is some precedent for that. Anything that queries binary currently falls under that umbrella (mostly undocumented and changeable). For functions exported by libpq though...it's probably better to nail things down as much as possible up front. This is a big advantage of the hooks strategy overall, it allows us to mature the library except for the interaction with libpq (ISTM Bruce was trying to give us a little leeway here as well, and we appreciate that). merlin
On Wed, May 14, 2008 at 10:44 AM, Tom Lane <tgl@sss.pgh.pa.us> 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. I'm trying to make this work as you suggest. It's pretty clear that all the callbacks should be modified to send a void* along with the other arguments. This would eliminate the need for hookname there (getting the state inside callback functions). 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. I'm a little bit stuck here. Is it possible to preserve the hookname outside of the context of the callback functions or is there something else I'm missing? (I'm sorry if I'm being obtuse) Eliminating the need for libpqtypes to need PQhookx functions, while possible, would make libpqtypes a lot more difficult to use and generally more awkward. merlin
"Merlin Moncure" <mmoncure@gmail.com> 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
Tom Lane wrote: > "Merlin Moncure" <mmoncure@gmail.com> 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. > > > Or else have the library return a unique handle when registering hooks, rather than supplying a hook name. cheers andrew
Tom Lane wrote: > "Merlin Moncure" <mmoncure@gmail.com> 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/
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/
Andrew Dunstan <andrew@dunslane.net> writes:
> 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.
Uh, how would that solve your problem? Seems like the difficulty
shifts from "how do I get the hook data" to "how do I get the key
with which to get the hook data".
regards, tom lane
Andrew Chernow <ac@esilo.com> 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
Tom Lane wrote:
> Andrew Chernow <ac@esilo.com> 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/
Andrew Chernow <ac@esilo.com> 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
Tom Lane wrote: > Andrew Chernow <ac@esilo.com> 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/
Tom Lane wrote:
> Andrew Chernow <ac@esilo.com> 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/
On Thu, May 15, 2008 at 8:38 PM, Andrew Chernow <ac@esilo.com> wrote: > 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). > 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. The most important part though is that it fits what we think is needed to maintain the data we associate with the libpq with the proper lifetime. I'm not sure that everyone was on the same page in terms of how this works...we may not have explained ourselves properly. In our defense, the interaction between libpq and a wrapping library like libpqtypes is a bit involved and the 'best possible' way to link things up did not necessarily suggest itself the first time out. We would like to wrap this up into some form the community would accept. The event proc style of doing this is better than our initial approach...faster and cleaner. In fact, we are pleased with all the changes that have come about due to community suggestions...there are many positive results. merlin
"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
On Fri, May 16, 2008 at 10:59 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > "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 *? 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
Merlin Moncure wrote: > >> 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. > > > All of this is getting quite a long way from what was in the commitfest queue. Do we still want to try to get this in this cycle, or should it be marked returned to author for more work? cheers andrew
"Merlin Moncure" <mmoncure@gmail.com> writes:
> Switch, plus struct (basically a union) will do the trick nicely. Can
> it be a formal union, or is it better as a void*?
I don't think a union buys much notational convenience or safety here,
although admittedly it's a close question. In one case you're trusting
to cast the pointer to the appropriate type, in the other you're
trusting to use the right union member. One advantage of separate
structs is that there's no reason not to make the struct type names
long enough to be clear, whereas there's a very strong notational
temptation to make union member names short, because you'll be typing
them a lot.
regards, tom lane
Andrew Dunstan <andrew@dunslane.net> writes:
> All of this is getting quite a long way from what was in the commitfest
> queue. Do we still want to try to get this in this cycle, or should it
> be marked returned to author for more work?
So far I think it still falls within the category of allowing the author
to revise his work. I don't want to hold commitfest open waiting on
revisions of this patch, but as long as there's still other stuff being
worked through I don't see why they can't keep trying.
Just for the record, I would really like to close this fest before
PGCon starts. We still have a couple more days to get it done.
regards, tom lane
Andrew Dunstan escribió: > All of this is getting quite a long way from what was in the commitfest > queue. Do we still want to try to get this in this cycle, or should it > be marked returned to author for more work? There are still patches open for which no discussion has even started, so I think this is OK for this commitfest. (Besides, it seems we're almost there on this patch.) -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
On Fri, May 16, 2008 at 11:21 AM, Andrew Dunstan <andrew@dunslane.net> wrote: > All of this is getting quite a long way from what was in the commitfest > queue. Do we still want to try to get this in this cycle, or should it be > marked returned to author for more work? That's your call...we can have a patch up within 24 hours or so. My suggestion would be to let us put up an updated version in the May queue, and if there are any further serious objections we can push it to the next queue giving some more time for things to percolate. merlin
Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: > >> All of this is getting quite a long way from what was in the commitfest >> queue. Do we still want to try to get this in this cycle, or should it >> be marked returned to author for more work? >> > > So far I think it still falls within the category of allowing the author > to revise his work. I don't want to hold commitfest open waiting on > revisions of this patch, but as long as there's still other stuff being > worked through I don't see why they can't keep trying. > > Just for the record, I would really like to close this fest before > PGCon starts. We still have a couple more days to get it done. > > > Apart from this one only two have not been claimed: . Map Forks . TRUNCATE TABLE with IDENTITY cheers andrew
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/
On Fri, May 16, 2008 at 2:34 PM, Andrew Chernow <ac@esilo.com> wrote:
> 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.
Right. I actually overlooked the 'passthrough' in
PQregisterEventProc. It turns out that we are still not quite on the
same page and this needs to be clarified before we move on. The
passthrough cannot exist...the correct prototypes (reasoning will
follow) are:
typedef void (*PGeventProc) (PGeventId eventId, const void *eventInfo);
int PQregisterEventProc(PGconn *conn, PGeventProc proc);
PQhookData(const PGconn* conn, PGeventProc proc);
...etc.
The purpose of the callbacks is to allow a hooking library to
establish private data that is associated with a PGconn or a PGresult.
Invoking PQregisterEventProc tells libpq 'I am interested in doing
this', for the supplied PGconn, future results created with that
connection, and (if PGconn is passed as null), all future connections.
Once that is established, libpq begins telling the hooking library
when and what needs to be allocated or deleted. This is done for both
connections and results at the appropriate times and the management
always occurs inside a callback function. Thus, while the hooking
library controls what is in the state data, _it does not control when
it is created or destroyed_. Passing a void* into PQregisterEventProc
suggests that is the wrapper's purvue to establish the private
information. It isn't, and it can't be..it's as simple as that.
For example, consider that 'hooked' PGconn then produces a result. If
a passthrough as Tom suggested were passed around, what would you do
to it when the connection was freed (which throws a connection closing
event)? What exactly is passthrough pointing to in a result copy
event? If passed around as a single poitner, they are not
defined.These are pointers to different structures who are created at
particular times that only libpq knows.
All of the callbacks (except for possibly conn reset) are basically
allocation and deletion events. For example, result create is libpq
telling the hooking library: 'a result is coming, please initialize
your result extensions now'. At that point, a very particular
structure is passed back to the hooking library:
{
const PGconn* conn; // the source conn the result
const void *conn_state; // the conn's private state (it might be
interesting, and we don't want to look it up later)
const PGresult* res; // the new result's poiter
void *out_res_state; // we are going to allocate something and set
this in the callback
}
As you can see, there is more going on here than a single passthrough
pointer can handle. We are in fact doing what a passthrough provides
in principle. I can certainly understand why our original 'lookup by
name' concept threw up a smokescreen (it did obfuscate the passing
requirements), but hooking can't be serviced by a single pointer set
at callback registration.
With the above prototypes and carefully designed structures,
everything feels natural. libpq is defining exactly what is supposed
to happen, and the structure parameters define what is coming in and
out. Things are being 'passed through'...but in a more discrete way.
merlin
On Fri, May 16, 2008 at 3:49 PM, Merlin Moncure <mmoncure@gmail.com> wrote: > On Fri, May 16, 2008 at 2:34 PM, Andrew Chernow <ac@esilo.com> wrote: >> 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. > > > Right. I actually overlooked the 'passthrough' in > PQregisterEventProc. It turns out that we are still not quite on the > same page and this needs to be clarified before we move on. The > passthrough cannot exist...the correct prototypes (reasoning will > follow) are: > > typedef void (*PGeventProc) (PGeventId eventId, const void *eventInfo); small typo: eventInfo obviously can't be const > int PQregisterEventProc(PGconn *conn, PGeventProc proc); > PQhookData(const PGconn* conn, PGeventProc proc); PQhookData is the old name...we are going with 'events' now....the proper names will come with the patch. merlin
"Merlin Moncure" <mmoncure@gmail.com> writes:
> Right. I actually overlooked the 'passthrough' in
> PQregisterEventProc. It turns out that we are still not quite on the
> same page and this needs to be clarified before we move on. The
> passthrough cannot exist...
Yes, it *will* exist. You are assuming that the goals you have for
these hooks are the only ones anyone will ever have. There are other
possible usage patterns and many of them will need some passthrough
state data. I'd venture to say that anytime I've ever used a callback
design that did not include a passthrough, I've had reason to curse
its designer sooner or later (qsort being a pretty typical example).
However, it's clear that we are not on the same page, because what
I thought you wanted the PQeventData function to return was the
passthrough pointer. Evidently that's not what you want it to do,
so you'd better back up a few steps and say what you do want it to do.
(I think that a function to get the passthrough value associated with
a given hook function might be useful too, but it's clear that what
you are after must be something else.)
> The purpose of the callbacks is to allow a hooking library to
> establish private data that is associated with a PGconn or a PGresult.
So you're imagining that on creation of a PGconn or PGresult, the
hook function would be allowed to create some storage and libpq would
then be responsible for tracking that storage? Okay, but that's a
separate feature from the sort of passthrough I had in mind. Where
exactly does the hook hand off the storage pointer to libpq? What
are you going to do if the hook fails to create the storage
(ie, out of memory during PGresult creation)?
> Invoking PQregisterEventProc tells libpq 'I am interested in doing
> this', for the supplied PGconn, future results created with that
> connection, and (if PGconn is passed as null), all future connections.
I am entirely serious about saying that I will not accept that last bit.
To do that we have to buy into having permanent modifiable storage
within libpq, protecting it with thread mutexes, etc etc. And I don't
like any of the possible usage scenarios for it. I think hooking into a
PGconn immediately after its creation is just as useful and a lot easier
to manage.
> Once that is established, libpq begins telling the hooking library
> when and what needs to be allocated or deleted.
Wait a minute --- you're trying to get between libpq and malloc?
Why? That's getting a *whole* lot more invasive than I thought
this patch intended to be, and I don't see a good reason for it.
regards, tom lane
On Fri, May 16, 2008 at 4:23 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > "Merlin Moncure" <mmoncure@gmail.com> writes: >> Right. I actually overlooked the 'passthrough' in >> PQregisterEventProc. It turns out that we are still not quite on the >> same page and this needs to be clarified before we move on. The >> passthrough cannot exist... > > Yes, it *will* exist. You are assuming that the goals you have for > these hooks are the only ones anyone will ever have. There are other > possible usage patterns and many of them will need some passthrough > state data. I'd venture to say that anytime I've ever used a callback > design that did not include a passthrough, I've had reason to curse > its designer sooner or later (qsort being a pretty typical example). right. It can exist, just not hold the event data (the extended properties of PGresult/conn). It has a meaning and scope that are not defined for libpq purposes. For the 'result' callbacks (see below), we will just use the passthrough passed in for the connection. libpqtypes will not use this, and we were getting nervous about people understanding what the different pointers were used for (we would call it a user pointer, not a pass through). > However, it's clear that we are not on the same page, because what > I thought you wanted the PQeventData function to return was the > passthrough pointer. Evidently that's not what you want it to do, > so you'd better back up a few steps and say what you do want it to do. > (I think that a function to get the passthrough value associated with > a given hook function might be useful too, but it's clear that what We will do this: void *PQeventData(PGconn*, PGeventProc, void **passthrough); void *PQresultEventData(PGresult*, PGeventProc, void **passthrough); > you are after must be something else.) The major challenge is that libpqtypes (and, presumably, other libraries) must essentially store its own data in both conn and result. We allocate our data when these structures are created and free it when they are destroyed. The idea is that libpq fires callbacks at appropriate times when conn/results are constructed/destructed. From these events we set up our private 'event' data and delete it. Each connection and result maintains a 'list' of private event states, one for each registering callback (libpqtypes only requires one). So, registering an event proc is analogous to adding one or fields to a conn or a result with a private meaning. So, libpq is defining 6 events: *) when a connection is created (put into OK status) *) when a connection is reset *) when a connection is destroyed *) when a result is created in non-error state *) when a result is copied (we are adding this to libpq w/PQcopyResult) *) when a result is destroyed In these events we set up or tear down the private state for the libpq objects. We need to sometimes look up the data from outside the library in public (non callback) code. This is what PQeventData, etc. accomplish...provide the private state for the object. The difference is that there can be more than one registered event proc on a conn/result at a time; thus the need for an additional "lookup" value when getting event data (what was hookName and is now the event proc address). >> The purpose of the callbacks is to allow a hooking library to >> establish private data that is associated with a PGconn or a PGresult. > > So you're imagining that on creation of a PGconn or PGresult, the > hook function would be allowed to create some storage and libpq would > then be responsible for tracking that storage? Okay, but that's a > separate feature from the sort of passthrough I had in mind. Where > exactly does the hook hand off the storage pointer to libpq? What > are you going to do if the hook fails to create the storage > (ie, out of memory during PGresult creation)? Right, this could happen for one of two likely reasons: OOM as you suggest or the callback fails for some reason only known to the hooking library. In either case, the callback would set the return pointer as defined by the structure to null, return FALSE, and libpq would not add the state to the array of 'event state datas' it maintains. Here is the event proc with passthrough added: int PGEventProc(PGEventId event, void *eventInfo, void *passThrough); // FALSE = failure >> Invoking PQregisterEventProc tells libpq 'I am interested in doing >> this', for the supplied PGconn, future results created with that >> connection, and (if PGconn is passed as null), all future connections. > > I am entirely serious about saying that I will not accept that last bit. > To do that we have to buy into having permanent modifiable storage > within libpq, protecting it with thread mutexes, etc etc. And I don't > like any of the possible usage scenarios for it. I think hooking into a > PGconn immediately after its creation is just as useful and a lot easier > to manage. Locking is not required so long as you follow certain conventions. Here is our warning that describes this in the header: + * WARNING: This should only be called in the application's main thread + * before using any other libpq functions. This is not thread-safe and + * should be used prior to creating any application threads. Obviously, the 'global' behavior is optional, but nice (it's kind of a 'gotcha' if you forget to re-register in a reconnect routine for example). It is like an _init() routine, and considered doing it that way. It does mean adding an array of 'global' event procs to libpq. So I would ask you to hold your judgment on this, and, once we get the new patch in, consider it and we will pull it if you still think it's too dangerous (meaning, the convention is not followed, by a api user). >> Once that is established, libpq begins telling the hooking library >> when and what needs to be allocated or deleted. > > Wait a minute --- you're trying to get between libpq and malloc? > Why? That's getting a *whole* lot more invasive than I thought > this patch intended to be, and I don't see a good reason for it. No, we don't get in between. Maybe my wording was a little misleading...libpq notifies the registered event proc implementations of when a conn or result is created/destroyed. libpq, strictly speaking, is not allocating the private states...just notifying other code it's time to do it. There is no change to the way libpq allocates things generally. When we send up the revised patch, you will see the invasion is minimal...the trickiest part was actually injecting the events in the proper places. merlin
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/
"Merlin Moncure" <mmoncure@gmail.com> writes:
> On Fri, May 16, 2008 at 4:23 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> So you're imagining that on creation of a PGconn or PGresult, the
>> hook function would be allowed to create some storage and libpq would
>> then be responsible for tracking that storage? Okay, but that's a
>> separate feature from the sort of passthrough I had in mind. Where
>> exactly does the hook hand off the storage pointer to libpq? What
>> are you going to do if the hook fails to create the storage
>> (ie, out of memory during PGresult creation)?
> Right, this could happen for one of two likely reasons: OOM as you
> suggest or the callback fails for some reason only known to the
> hooking library. In either case, the callback would set the return
> pointer as defined by the structure to null, return FALSE, and libpq
> would not add the state to the array of 'event state datas' it
> maintains.
So at that point we have an apparently-successful creation of a
PGresult, but some of the hook library's functionality is going to fail
to work with that PGresult. That doesn't seem very nice. 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.
regards, tom lane
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/
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 -0000 1.19 --- exports.txt 17 May 2008 12:06:10 -0000 *************** *** 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 + PQsetvalue 143 + 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.c 16 May 2008 18:30:53 -0000 1.358 --- fe-connect.c 17 May 2008 12:06:11 -0000 *************** *** 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; + } return PGRES_POLLING_OK; default: printfPQExpBuffer(&conn->errorMessage, libpq_gettext( "invalid connection state %c, " *************** *** 1970,1981 **** --- 1985,2016 ---- * 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 info; + + /* Let the event procs cleanup their state data */ + for(i=0; i < conn->evtStateCount; i++) + { + info.conn = conn; + info.data = conn->evtStates[i].data; + (void)conn->evtStates[i].proc(PGEVT_CONNDESTROY, + &info, conn->evtStates[i].arg); + } + + /* free the PGEventState array */ + if(conn->evtStates) + { + free(conn->evtStates); + conn->evtStates = NULL; + conn->evtStateCount = conn->evtStateSize = 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: --- 2186,2218 ---- PQreset(PGconn *conn) { if (conn) { closePGconn(conn); ! if (connectDBStart(conn) && connectDBComplete(conn)) ! { ! int i; ! PGEventConnReset info; ! ! for(i=0; i < conn->evtStateCount; i++) ! { ! info.conn = conn; ! info.data = conn->evtStates[i].data; ! ! if(!conn->evtStates[i].proc(PGEVT_CONNRESET, &info, ! conn->evtStates[i].arg)) ! { ! conn->status = CONNECTION_BAD; ! printfPQExpBuffer(&conn->errorMessage, ! libpq_gettext("PGEventProc %p failed during PGEVT_CONNRESET event\n"), ! conn->evtStates[i].proc); ! break; ! } ! } ! } } } /* * PQresetStart: *************** *** 2186,2198 **** * closes the existing connection and makes a new one */ PostgresPollingStatusType PQresetPoll(PGconn *conn) { if (conn) ! return PQconnectPoll(conn); return PGRES_POLLING_FAILED; } /* * PQcancelGet: get a PGcancel structure corresponding to a connection. --- 2240,2278 ---- * closes the existing connection and makes a new one */ PostgresPollingStatusType PQresetPoll(PGconn *conn) { if (conn) ! { ! PostgresPollingStatusType status = PQconnectPoll(conn); ! ! if(status == PGRES_POLLING_OK) ! { ! int i; ! PGEventConnReset info; ! ! for(i=0; i < conn->evtStateCount; i++) ! { ! info.conn = conn; ! info.data = conn->evtStates[i].data; ! ! if(!conn->evtStates[i].proc(PGEVT_CONNRESET, &info, ! conn->evtStates[i].arg)) ! { ! conn->status = CONNECTION_BAD; ! printfPQExpBuffer(&conn->errorMessage, ! libpq_gettext("PGEventProc %p failed during PGEVT_CONNRESET event\n"), ! conn->evtStates[i].proc); ! return PGRES_POLLING_FAILED; ! } ! } ! } ! ! return status; ! } return PGRES_POLLING_FAILED; } /* * PQcancelGet: get a PGcancel structure corresponding to a connection. Index: fe-exec.c =================================================================== RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-exec.c,v retrieving revision 1.194 diff -C6 -r1.194 fe-exec.c *** fe-exec.c 1 Jan 2008 19:46:00 -0000 1.194 --- fe-exec.c 17 May 2008 12:06:12 -0000 *************** *** 60,72 **** int resultFormat); static void parseInput(PGconn *conn); static bool PQexecStart(PGconn *conn); static PGresult *PQexecFinish(PGconn *conn); static int PQsendDescribe(PGconn *conn, char desc_type, const char *desc_target); ! /* ---------------- * Space management for PGresult. * * Formerly, libpq did a separate malloc() for each field of each tuple * returned by a query. This was remarkably expensive --- malloc/free --- 60,72 ---- int resultFormat); static void parseInput(PGconn *conn); static bool PQexecStart(PGconn *conn); 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. * * Formerly, libpq did a separate malloc() for each field of each tuple * returned by a query. This was remarkably expensive --- malloc/free *************** *** 121,141 **** --- 121,167 ---- #define PGRESULT_DATA_BLOCKSIZE 2048 #define PGRESULT_ALIGN_BOUNDARY MAXIMUM_ALIGNOF /* from configure */ #define PGRESULT_BLOCK_OVERHEAD Max(sizeof(PGresult_data), PGRESULT_ALIGN_BOUNDARY) #define PGRESULT_SEP_ALLOC_THRESHOLD (PGRESULT_DATA_BLOCKSIZE / 2) + /* Does not duplicate the private state data, sets this to NULL */ + static PGEventState * + dupEventStates(PGEventState *states, int count) + { + int i; + PGEventState *newStates; + + if(!states || count <= 0) + return NULL; + + newStates = (PGEventState *)malloc(count * sizeof(PGEventState)); + if(!newStates) + return NULL; + + memcpy(newStates, states, count * sizeof(PGEventState)); + + /* NULL out the data pointer */ + for(i=0; i < count; i++) + newStates[i].data = NULL; + + return newStates; + } + /* * PQmakeEmptyPGresult * returns a newly allocated, initialized PGresult with given status. * If conn is not NULL and status indicates an error, the conn's * errorMessage is copied. * * Note this is exported --- you wouldn't think an application would need * to build its own PGresults, but this has proven useful in both libpgtcl * and the Perl5 interface, so maybe it's not so unreasonable. + * + * Updated April 2008 - If conn is not NULL, event states will be copied + * from the PGconn to the created PGresult. */ PGresult * PQmakeEmptyPGresult(PGconn *conn, ExecStatusType status) { PGresult *result; *************** *** 157,175 **** --- 183,218 ---- result->errMsg = NULL; result->errFields = NULL; result->null_field[0] = '\0'; result->curBlock = NULL; result->curOffset = 0; result->spaceLeft = 0; + result->evtStateCount = 0; + result->evtStates = NULL; if (conn) { /* copy connection data we might need for operations on PGresult */ result->noticeHooks = conn->noticeHooks; result->client_encoding = conn->client_encoding; + /* copy event states from connection */ + if(conn->evtStateCount > 0) + { + result->evtStates = dupEventStates( + conn->evtStates, conn->evtStateCount); + + if(!result->evtStates) + { + PQclear(result); + return NULL; + } + + result->evtStateCount = conn->evtStateCount; + } + /* consider copying conn's errorMessage */ switch (status) { case PGRES_EMPTY_QUERY: case PGRES_COMMAND_OK: case PGRES_TUPLES_OK: *************** *** 193,204 **** --- 236,490 ---- } return result; } /* + * PQcopyResult + * Returns a deep copy of the provided 'src' PGresult, which cannot be NULL. + * The 'options' argument controls which portions of the result will or will + * NOT be copied. If this value is 0, the entire result is deep copied. + * The created result is always put into the PGRES_TUPLES_OK status. The + * source result error message is not copied, although cmdStatus is. + * + * Options: + * PG_COPYRES_NO_TUPLES - Do not copy the tuples. This option is + * automatically enabled when PG_COPYRES_USE_ATTRS is set. + * + * PG_COPYRES_USE_ATTRS - Indicates that the 'numAttributes' and 'attDescs' + * arguments should be used as the fields for the created result, rather + * than copying them from the source result. When this option is set, + * the tuples will NOT be copied; behaving identically to setting the + * PG_COPYRES_NO_TUPLES option. One must use PQsetvalue to manually + * add tuples to the returned result. NOTE: numAttributes and attDescs + * arguments are ignored unless this option is set! + * + * PG_COPYRES_NO_EVENTPROCS - Indicates that the source result's + * PGEventProcs should NOT be copied to the created result. + * + * PG_COPYRES_NO_NOTICEHOOKS - Indicates that the source result's + * NoticeHooks should NOT be copied to the created result. + */ + + PGresult * + PQcopyResult(const PGresult *src, int numAttributes, + PGresAttDesc *attDescs, int options) + { + int i; + PGresult *dest; + PGEventResultCopy info; + + if(!src) + return NULL; + + /* Automatically turn on no_tuples since use_attrs is set. It makes + * no sense to copy tuples into an unknown set of columns. + */ + if(options & PG_COPYRES_USE_ATTRS) + options |= PG_COPYRES_NO_TUPLES; + + /* If use_attrs is set, verify attr arguments. */ + if((options & PG_COPYRES_USE_ATTRS) && (numAttributes <= 0 || !attDescs)) + return NULL; + + dest = PQmakeEmptyPGresult((PGconn *)NULL, PGRES_TUPLES_OK); + if(!dest) + return NULL; + + /* always copy these over. Is cmdStatus useful here? */ + dest->client_encoding = src->client_encoding; + strcpy(dest->cmdStatus, src->cmdStatus); + + /* Wants to copy notice hooks */ + if(!(options & PG_COPYRES_NO_NOTICEHOOKS)) + dest->noticeHooks = src->noticeHooks; + + /* Copy from src result when not supplying attrs */ + if(!(options & PG_COPYRES_USE_ATTRS) && src->numAttributes > 0) + { + numAttributes = src->numAttributes; + attDescs = src->attDescs; + } + + /* copy attrs */ + if(numAttributes > 0) + { + dest->attDescs = (PGresAttDesc *)PQresultAlloc(dest, + numAttributes * sizeof(PGresAttDesc)); + + if(!dest->attDescs) + { + PQclear(dest); + return NULL; + } + + dest->numAttributes = numAttributes; + memcpy(dest->attDescs, attDescs, numAttributes * sizeof(PGresAttDesc)); + + /* resultalloc the attribute names. The above memcpy has the attr + * names pointing at the source result's private memory (or at the + * callers provided attDescs memory). + */ + dest->binary = 1; + for(i=0; i < numAttributes; i++) + { + if(attDescs[i].name) + dest->attDescs[i].name = pqResultStrdup(dest, attDescs[i].name); + else + dest->attDescs[i].name = dest->null_field; + + if(!dest->attDescs[i].name) + { + PQclear(dest); + return NULL; + } + + /* Although deprecated, because results can have text+binary columns, + * its easy enough to deduce so set it for completeness. + */ + if(dest->attDescs[i].format == 0) + dest->binary = 0; + } + } + + /* Wants to copy result tuples: use PQsetvalue(). */ + if(!(options & PG_COPYRES_NO_TUPLES) && src->ntups > 0) + { + int tup, field; + for(tup=0; tup < src->ntups; tup++) + for(field=0; field < src->numAttributes; field++) + PQsetvalue(dest, tup, field, src->tuples[tup][field].value, + src->tuples[tup][field].len); + } + + /* Wants to copy PGEventProcs. */ + if(!(options & PG_COPYRES_NO_EVENTPROCS) && src->evtStateCount > 0) + { + dest->evtStates = dupEventStates(src->evtStates, src->evtStateCount); + + if(!dest->evtStates) + { + PQclear(dest); + return NULL; + } + + dest->evtStateCount = src->evtStateCount; + } + + /* Trigger PGEVT_RESULTCOPY event for each event proc */ + for(i=0; i < dest->evtStateCount; i++) + { + info.src = src; + info.srcData = src->evtStates[i].data; + info.dest = dest; + info.destData = NULL; + + if(!src->evtStates[i].proc(PGEVT_RESULTCOPY, &info, + src->evtStates[i].arg)) + { + PQclear(dest); + return NULL; + } + + dest->evtStates[i].data = info.destData; + } + + return dest; + } + + int + PQsetvalue(PGresult *res, int tup_num, int field_num, + char *value, int len) + { + PGresAttValue *attval; + + if(!check_field_number(res, field_num)) + return FALSE; + + /* Invalid tup_num, must be <= ntups */ + if(tup_num > res->ntups) + return FALSE; + + /* need to grow the tuple table */ + if(res->ntups >= res->tupArrSize) + { + int n = res->tupArrSize ? (res->tupArrSize*3)/2 : 64; + PGresAttValue **tups = (PGresAttValue **) + (res->tuples ? realloc(res->tuples, n*sizeof(PGresAttValue *)) : + malloc(n*sizeof(PGresAttValue *))); + + if(!tups) + return FALSE; + + memset(tups + res->tupArrSize, 0, + (n - res->tupArrSize) * sizeof(PGresAttValue *)); + res->tuples = tups; + res->tupArrSize = n; + } + + /* need to allocate a new tuple */ + if(tup_num == res->ntups && !res->tuples[tup_num]) + { + int i; + PGresAttValue *tup = (PGresAttValue *)PQresultAlloc( + res, res->numAttributes * sizeof(PGresAttValue)); + + if(!tup) + return FALSE; + + /* initialize each column to NULL */ + for(i=0; i < res->numAttributes; i++) + { + tup[i].len = NULL_LEN; + tup[i].value = res->null_field; + } + + res->tuples[tup_num] = tup; + res->ntups++; + } + + attval = &res->tuples[tup_num][field_num]; + + /* On top of NULL_LEN, treat a NULL value as a NULL field */ + if(len == NULL_LEN || value == NULL) + { + attval->len = NULL_LEN; + attval->value = res->null_field; + } + else + { + if(len < 0) + len = 0; + + if(len == 0) + { + attval->len = 0; + attval->value = res->null_field; + } + else + { + attval->value = (char *)PQresultAlloc(res, len + 1); + if(!attval->value) + return FALSE; + + attval->len = len; + memcpy(attval->value, value, len); + attval->value[len] = '\0'; + } + } + + return TRUE; + } + + void * + PQresultAlloc(PGresult *res, size_t nBytes) + { + return pqResultAlloc(res, nBytes, TRUE); + } + + /* * pqResultAlloc - * Allocate subsidiary storage for a PGresult. * * nBytes is the amount of space needed for the object. * If isBinary is true, we assume that we need to align the object on * a machine allocation boundary. *************** *** 349,365 **** --- 635,669 ---- * PQclear - * free's the memory associated with a PGresult */ void PQclear(PGresult *res) { + int i; PGresult_data *block; + PGEventResultDestroy info; if (!res) return; + for(i=0; i < res->evtStateCount; i++) + { + info.result = res; + info.data = res->evtStates[i].data; + + (void)res->evtStates[i].proc(PGEVT_RESULTDESTROY, &info, + res->evtStates[i].arg); + } + + if(res->evtStates) + { + free(res->evtStates); + res->evtStates = NULL; + res->evtStateCount = 0; + } + /* Free all the subsidiary blocks */ while ((block = res->curBlock) != NULL) { res->curBlock = block->next; free(block); } *************** *** 1190,1202 **** * memory). */ PGresult * PQgetResult(PGconn *conn) { ! PGresult *res; if (!conn) return NULL; /* Parse any available data, if our state permits. */ parseInput(conn); --- 1494,1506 ---- * memory). */ PGresult * PQgetResult(PGconn *conn) { ! PGresult *res=NULL; if (!conn) return NULL; /* Parse any available data, if our state permits. */ parseInput(conn); *************** *** 1265,1276 **** --- 1569,1611 ---- libpq_gettext("unexpected asyncStatus: %d\n"), (int) conn->asyncStatus); res = PQmakeEmptyPGresult(conn, PGRES_FATAL_ERROR); break; } + if(res && (res->resultStatus == PGRES_COMMAND_OK || + res->resultStatus == PGRES_TUPLES_OK || + res->resultStatus == PGRES_EMPTY_QUERY)) + { + int i; + PGEventResultCreate info; + + info.conn = conn; + info.result = res; + + for(i=0; i < res->evtStateCount; i++) + { + info.connData = conn->evtStates[i].data; + info.resultData = NULL; + + if(!res->evtStates[i].proc(PGEVT_RESULTCREATE, + &info, res->evtStates[i].arg)) + { + char msg[256]; + sprintf(msg, + "PGEventProc %p failed during PGEVT_RESULTCREATE event", + res->evtStates[i].proc); + pqSetResultError(res, msg); + res->resultStatus = PGRES_FATAL_ERROR; + break; + } + + res->evtStates[i].data = info.resultData; + } + } + return res; } /* * PQexec Index: fe-misc.c =================================================================== RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-misc.c,v retrieving revision 1.133 diff -C6 -r1.133 fe-misc.c *** fe-misc.c 1 Jan 2008 19:46:00 -0000 1.133 --- fe-misc.c 17 May 2008 12:06:12 -0000 *************** *** 1152,1157 **** --- 1152,1301 ---- } return dgettext("libpq", msgid); } #endif /* ENABLE_NLS */ + + + /* --------------------- + * Events + */ + + static int g_evtStateCount = 0; + static int g_evtStateSize = 0; + static PGEventState *g_evtStates = NULL; + + int + PQregisterEventProc(PGconn *conn, PGEventProc proc, void *arg) + { + int i; + int *count; + int *size; + PGEventState **states; + + if(!proc) + return FALSE; + + /* global register */ + if(!conn) + { + count = &g_evtStateCount; + size = &g_evtStateSize; + states = &g_evtStates; + } + /* per-conn */ + else + { + count = &conn->evtStateCount; + size = &conn->evtStateSize; + states = &conn->evtStates; + } + + for(i=0; i < *count; i++) + if((*states)[i].proc == proc) + return FALSE; /* already registered */ + + if(*count >= *size) + { + PGEventState *e; + int newSize = *size ? (*size * 3) / 2 : 8; + + if(*states) + e = (PGEventState *)realloc(*states, newSize*sizeof(PGEventState)); + else + e = (PGEventState *)malloc(newSize*sizeof(PGEventState)); + + if(!e) + return FALSE; + + *size = newSize; + *states = e; + } + + (*states)[*count].arg = arg; + (*states)[*count].data = NULL; + (*states)[*count].proc = proc; + + /* per-conn register requires triggering an initstate event */ + if(conn) + { + PGEventInitState initState; + initState.data = NULL; + initState.conn = conn; + if(!proc(PGEVT_INITSTATE, &initState, arg)) + return FALSE; + (*states)[*count].data = initState.data; + } + + (*count)++; + return TRUE; + } + + void * + PQeventData(const PGconn *conn, PGEventProc proc, void **arg) + { + int i; + + if(arg) + *arg = NULL; + + if(!conn || !proc) + return NULL; + + for(i=0; i < conn->evtStateCount; i++) + { + if(conn->evtStates[i].proc == proc) + { + if(arg) + *arg = conn->evtStates[i].arg; + return conn->evtStates[i].data; + } + } + + return NULL; + } + + void * + PQresultEventData(const PGresult *result, PGEventProc proc, void **arg) + { + int i; + + if(arg) + *arg = NULL; + + if(!result || !proc) + return NULL; + + for(i=0; i < result->evtStateCount; i++) + { + if(result->evtStates[i].proc == proc) + { + if(arg) + *arg = result->evtStates[i].arg; + return result->evtStates[i].data; + } + } + + return NULL; + } + + /* Registers the global events procs for a conn object. If the procs + * were already registered, the request is ignored and a success value + * is returned. This will happen during a PQreset and PQresetPoll. + * Returns non-zero for success and zero on error. + */ + int + pqRegisterGlobalEvents(PGconn *conn) + { + int i; + + /* Already registered, occurs on PQreset and PQresetPoll */ + if(conn->evtStateCount > 0) + return TRUE; + + for(i=0; i < g_evtStateCount; i++) + if(!PQregisterEventProc(conn, g_evtStates[i].proc, g_evtStates[i].arg)) + return FALSE; + + return TRUE; + } Index: libpq-fe.h =================================================================== RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/libpq-fe.h,v retrieving revision 1.142 diff -C6 -r1.142 libpq-fe.h *** libpq-fe.h 19 Mar 2008 00:39:33 -0000 1.142 --- libpq-fe.h 17 May 2008 12:06:12 -0000 *************** *** 25,36 **** --- 25,45 ---- /* * postgres_ext.h defines the backend's externally visible types, * such as Oid. */ #include "postgres_ext.h" + /* ----------------------- + * Options for PQcopyResult + */ + + #define PG_COPYRES_NO_TUPLES 0x01 + #define PG_COPYRES_USE_ATTRS 0x02 + #define PG_COPYRES_NO_EVENTPROCS 0x04 + #define PG_COPYRES_NO_NOTICEHOOKS 0x08 + /* Application-visible enum types */ typedef enum { /* * Although it is okay to add to this list, values which become unused *************** *** 190,201 **** --- 199,283 ---- int *ptr; /* can't use void (dec compiler barfs) */ int integer; } u; } PQArgBlock; /* ---------------- + * PGresAttDesc -- Data about a single attribute (column) of a query result + * ---------------- + */ + typedef struct pgresAttDesc + { + char *name; /* column name */ + Oid tableid; /* source table, if known */ + int columnid; /* source column, if known */ + int format; /* format code for value (text/binary) */ + Oid typid; /* type id */ + int typlen; /* type size */ + int atttypmod; /* type-specific modifier info */ + } PGresAttDesc; + + /* ---------------- + * Events - Allows receiving events from libpq. + * ---------------- + */ + + /* Event Ids */ + typedef enum + { + PGEVT_INITSTATE, + PGEVT_CONNRESET, + PGEVT_CONNDESTROY, + PGEVT_RESULTCREATE, + PGEVT_RESULTCOPY, + PGEVT_RESULTDESTROY + } PGEventId; + + typedef struct + { + void *data; /* create/assign during this event if needed */ + const PGconn *conn; + } PGEventInitState; + + typedef struct + { + void *data; + const PGconn *conn; + } PGEventConnReset; + + typedef struct + { + void *data; + const PGconn *conn; + } PGEventConnDestroy; + + typedef struct + { + void *connData; /* conn's event data */ + const PGconn *conn; + void *resultData; /* create/assign during this event if needed */ + const PGresult *result; + } PGEventResultCreate; + + typedef struct + { + void *srcData; /* src result's event data */ + const PGresult *src; + void *destData; /* create/assign during this event if needed */ + PGresult *dest; + } PGEventResultCopy; + + typedef struct + { + void *data; + const PGresult *result; + } PGEventResultDestroy; + + typedef int (*PGEventProc)(PGEventId evtId, void *evtInfo, void *arg); + + /* ---------------- * Exported functions of libpq * ---------------- */ /* === in fe-connect.c === */ *************** *** 434,445 **** --- 516,546 ---- * Make an empty PGresult with given status (some apps find this * useful). If conn is not NULL and status indicates an error, the * conn's errorMessage is copied. */ extern PGresult *PQmakeEmptyPGresult(PGconn *conn, ExecStatusType status); + extern PGresult * + PQcopyResult(const PGresult *src, int numAttributes, + PGresAttDesc *attDescs, int options); + + extern void * + PQresultAlloc(PGresult *res, size_t nBytes); + + /* + * Sets the value for a tuple field. The tup_num must be less than or + * equal to PQntuples(res). This function will generate tuples as needed. + * A new tuple is generated when tup_num equals PQntuples(res) and there + * are no fields defined for that tuple. + * + * Returns a non-zero value for success and zero for failure. + */ + extern int + PQsetvalue(PGresult *res, int tup_num, int field_num, + char *value, int len); + /* Quoting strings before inclusion in queries. */ extern size_t PQescapeStringConn(PGconn *conn, char *to, const char *from, size_t length, int *error); extern unsigned char *PQescapeByteaConn(PGconn *conn, *************** *** 506,517 **** --- 607,661 ---- /* Determine display length of multibyte encoded char at *s */ extern int PQdsplen(const char *s, int encoding); /* Get encoding id from environment variable PGCLIENTENCODING */ extern int PQenv2encoding(void); + /* + * Registers an event proc with the given PGconn. The arg parameter + * is an application specific pointer and can be set to NULL if not + * required. The arg parameter will be passed to the event proc and + * is publically retrieveable via PQeventData or PQresultEventData. + * + * The proc parameter cannot be NULL. + * + * If the conn is NULL, this will register a global event proc. + * Global event procs are automatically registered on every PGconn + * created, removing the need for per-conn registration. + * + * WARNING: Global registrations should only be called from the application's + * main thread, prior to using any libpq functions and creating + * any application threads. Global registration should be treated as an + * init function. + * + * The function returns a non-zero if successful. If the function + * fails, zero is returned. + */ + extern int + PQregisterEventProc(PGconn *conn, PGEventProc proc, void *arg); + + /* + * Gets the event data created by the provided event proc for the given + * conn. The event data is returned, which may be NULL if the event proc + * does not create any event data. The arg parameter can be used to + * get the application specific pointer provided during event proc + * registration. If arg is not needed, it can be set to NULL. + */ + extern void * + PQeventData(const PGconn *conn, PGEventProc proc, void **arg); + + /* + * Gets the event data created by the provided event proc for the given + * result. The event data is returned, which may be NULL if the event proc + * does not create any event data. The arg parameter can be used to + * get the application specific pointer provided during event proc + * registration. If arg is not needed, it can be set to NULL. + */ + extern void * + PQresultEventData(const PGresult *res, PGEventProc proc, void **arg); + /* === in fe-auth.c === */ extern char *PQencryptPassword(const char *passwd, const char *user); /* === in encnames.c === */ Index: libpq-int.h =================================================================== RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/libpq-int.h,v retrieving revision 1.130 diff -C6 -r1.130 libpq-int.h *** libpq-int.h 16 May 2008 18:30:53 -0000 1.130 --- libpq-int.h 17 May 2008 12:06:12 -0000 *************** *** 97,121 **** union pgresult_data { PGresult_data *next; /* link to next block, or NULL */ char space[1]; /* dummy for accessing block as bytes */ }; - /* Data about a single attribute (column) of a query result */ - - typedef struct pgresAttDesc - { - char *name; /* column name */ - Oid tableid; /* source table, if known */ - int columnid; /* source column, if known */ - int format; /* format code for value (text/binary) */ - Oid typid; /* type id */ - int typlen; /* type size */ - int atttypmod; /* type-specific modifier info */ - } PGresAttDesc; - /* Data about a single parameter of a prepared statement */ typedef struct pgresParamDesc { Oid typid; /* type id */ } PGresParamDesc; --- 97,108 ---- *************** *** 159,170 **** --- 146,166 ---- PQnoticeReceiver noticeRec; /* notice message receiver */ void *noticeRecArg; PQnoticeProcessor noticeProc; /* notice message processor */ void *noticeProcArg; } PGNoticeHooks; + typedef struct + { + /* pointer supplied by user */ + void *arg; + /* state data, optionally generated by event proc */ + void *data; + PGEventProc proc; + } PGEventState; + struct pg_result { int ntups; int numAttributes; PGresAttDesc *attDescs; PGresAttValue **tuples; /* each PGresTuple is an array of *************** *** 181,192 **** --- 177,192 ---- * These fields are copied from the originating PGconn, so that operations * on the PGresult don't have to reference the PGconn. */ PGNoticeHooks noticeHooks; int client_encoding; /* encoding id */ + /* Event states */ + int evtStateCount; + PGEventState *evtStates; + /* * Error information (all NULL if not an error result). errMsg is the * "overall" error message returned by PQresultErrorMessage. If we have * per-field info then it is stored in a linked list. */ char *errMsg; /* error message, or NULL if no error */ *************** *** 300,311 **** --- 300,316 ---- /* Optional file to write trace info to */ FILE *Pfdebug; /* Callback procedures for notice message processing */ PGNoticeHooks noticeHooks; + /* Event states */ + int evtStateCount; + int evtStateSize; + PGEventState *evtStates; + /* Status indicators */ ConnStatusType status; PGAsyncStatusType asyncStatus; PGTransactionStatusType xactStatus; /* never changes to ACTIVE */ PGQueryClass queryclass; char *last_query; /* last SQL command, or NULL if unknown */ *************** *** 527,538 **** --- 532,544 ---- extern int pqFlush(PGconn *conn); extern int pqWait(int forRead, int forWrite, PGconn *conn); extern int pqWaitTimed(int forRead, int forWrite, PGconn *conn, time_t finish_time); extern int pqReadReady(PGconn *conn); extern int pqWriteReady(PGconn *conn); + extern int pqRegisterGlobalEvents(PGconn *conn); /* === in fe-secure.c === */ extern int pqsecure_initialize(PGconn *); extern void pqsecure_destroy(void); extern PostgresPollingStatusType pqsecure_open_client(PGconn *);
On Sat, May 17, 2008 at 8:28 AM, Andrew Chernow <ac@esilo.com> 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
Merlin Moncure wrote: > On Sat, May 17, 2008 at 8:28 AM, Andrew Chernow <ac@esilo.com> 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/
Andrew Chernow <ac@esilo.com> writes:
> 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.
This is starting to get there, though I am still desperately unhappy
with the notion of "global" registrations. As I've said repeatedly,
I don't want that concept in any shape or form: there are no uses for
it that I regard as good ideas. (And taking out the mutex protection
for the global state isn't making it better, since that makes the
plausible use cases even narrower.)
> 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.
Hmm. "Event" for a callback seems a bit weird. The reasons for calling
the callback are events, and by extension the information structs you
pass to it could legitimately be called events, but the callback isn't
an event. "Event hook" or "event proc" seem possibly reasonable
nomenclature, though I'd still rather just go with "PGCallback".
Likewise I don't care for "event data", as the data inside that struct
is even less of an event than the proc is. "Per-callback instance data"
is the best description I can think of but it seems a bit awkward.
I've gone with "instance data" and "passthrough data" in the comments
below.
Some other comments in no particular order:
* I'm inclined to split out the hook-related stuff into a separate
header instead of cluttering libpq-fe.h with it, on the theory that
it has a separate audience; average applications will not be interested
in it.
* The proposed API passes the instance data pointer via the event struct
and the passthrough data pointer as a separate parameter. This seems
just weird. Shouldn't we do both the same way? This ties into my
original thought that the event data struct should be const, on the
grounds that you shouldn't have to initialize it more than once for all
the callbacks. But I'm not quite sure how you allocate a new instance
data value if that's the case. 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);
This would cost a couple extra cycles compared to what you have,
but surely not much in the normal case where there are very few
hooks registered. Also, meseems you need such a callback anyway:
what if the hook library desires to realloc its instance data
larger?
* Likewise it's weird to provide PQeventData and PQresultEventData
returning one pointer as function result and one as an output
argument. I'd suggest four functions returning results only,
perhaps
void *PQinstanceData(const PGconn *conn, PGEventProc proc)
void *PQpassthroughData(const PGconn *conn, PGEventProc proc)
void *PQresultInstanceData(const PGresult *res, PGEventProc proc)
void *PQresultPassthroughData(const PGresult *res, PGEventProc proc)
The way you have it might save a few cycles in the case where a client
needs both pointers, but I have a feeling that most callers will only
care about one or the other.
regards, tom lane
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/
Andrew Chernow <ac@esilo.com> 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?
> 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.
> 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.
regards, tom lane
On Mon, May 19, 2008 at 10:37 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Andrew Chernow <ac@esilo.com> 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? > >> 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 think 'events' best describes what is going on. Your other suggestions are improvements (we maybe tried a little too hard to minimize exports). Losing the global hooks is no problem (it was basically syntax sugar). merlin
Tom Lane wrote: > Andrew Chernow <ac@esilo.com> 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/
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/ 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 -0000 1.166 --- src/interfaces/libpq/Makefile 20 May 2008 04:18:07 -0000 *************** *** 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 -0000 1.19 --- src/interfaces/libpq/exports.txt 20 May 2008 04:18:07 -0000 *************** *** 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 + PQsetvalue 143 + PQresultAlloc 144 + PQregisterEventProc 145 + PQinstanceData 146 + 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 -0000 1.358 --- src/interfaces/libpq/fe-connect.c 20 May 2008 04:18:08 -0000 *************** *** 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(conn) && connectDBComplete(conn)) ! { ! int i; ! PGEventConnReset evt; ! ! for(i=0; i < conn->nEvents; i++) ! { ! evt.conn = conn; ! ! if(!conn->events[i].proc(PGEVT_CONNRESET, &evt)) ! { ! conn->status = CONNECTION_BAD; ! printfPQExpBuffer(&conn->errorMessage, ! libpq_gettext("PGEventProc %p failed during PGEVT_CONNRESET event\n"), ! conn->events[i].proc); ! break; ! } ! } ! } } } /* * PQresetStart: *************** *** 2186,2198 **** * closes the existing connection and makes a new one */ PostgresPollingStatusType PQresetPoll(PGconn *conn) { if (conn) ! return PQconnectPoll(conn); return PGRES_POLLING_FAILED; } /* * PQcancelGet: get a PGcancel structure corresponding to a connection. --- 2221,2257 ---- * closes the existing connection and makes a new one */ PostgresPollingStatusType PQresetPoll(PGconn *conn) { if (conn) ! { ! PostgresPollingStatusType status = PQconnectPoll(conn); ! ! if(status == PGRES_POLLING_OK) ! { ! int i; ! PGEventConnReset evt; ! ! for(i=0; i < conn->nEvents; i++) ! { ! evt.conn = conn; ! ! if(!conn->events[i].proc(PGEVT_CONNRESET, &evt)) ! { ! conn->status = CONNECTION_BAD; ! printfPQExpBuffer(&conn->errorMessage, ! libpq_gettext("PGEventProc %p failed during PGEVT_CONNRESET event\n"), ! conn->events[i].proc); ! return PGRES_POLLING_FAILED; ! } ! } ! } ! ! return status; ! } return PGRES_POLLING_FAILED; } /* * PQcancelGet: get a PGcancel structure corresponding to a connection. Index: src/interfaces/libpq/fe-exec.c =================================================================== RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-exec.c,v retrieving revision 1.194 diff -C6 -r1.194 fe-exec.c *** src/interfaces/libpq/fe-exec.c 1 Jan 2008 19:46:00 -0000 1.194 --- src/interfaces/libpq/fe-exec.c 20 May 2008 04:18:08 -0000 *************** *** 60,72 **** int resultFormat); static void parseInput(PGconn *conn); static bool PQexecStart(PGconn *conn); static PGresult *PQexecFinish(PGconn *conn); static int PQsendDescribe(PGconn *conn, char desc_type, const char *desc_target); ! /* ---------------- * Space management for PGresult. * * Formerly, libpq did a separate malloc() for each field of each tuple * returned by a query. This was remarkably expensive --- malloc/free --- 60,72 ---- int resultFormat); static void parseInput(PGconn *conn); static bool PQexecStart(PGconn *conn); 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. * * Formerly, libpq did a separate malloc() for each field of each tuple * returned by a query. This was remarkably expensive --- malloc/free *************** *** 121,141 **** --- 121,167 ---- #define PGRESULT_DATA_BLOCKSIZE 2048 #define PGRESULT_ALIGN_BOUNDARY MAXIMUM_ALIGNOF /* from configure */ #define PGRESULT_BLOCK_OVERHEAD Max(sizeof(PGresult_data), PGRESULT_ALIGN_BOUNDARY) #define PGRESULT_SEP_ALLOC_THRESHOLD (PGRESULT_DATA_BLOCKSIZE / 2) + /* Does not duplicate the event instance data, sets this to NULL */ + static PGEvent * + dupEvents(PGEvent *events, int count) + { + int i; + PGEvent *newEvents; + + if(!events || count <= 0) + return NULL; + + newEvents = (PGEvent *)malloc(count * sizeof(PGEvent)); + if(!newEvents) + return NULL; + + memcpy(newEvents, events, count * sizeof(PGEvent)); + + /* NULL out the data pointer */ + for(i=0; i < count; i++) + newEvents[i].data = NULL; + + return newEvents; + } + /* * PQmakeEmptyPGresult * returns a newly allocated, initialized PGresult with given status. * If conn is not NULL and status indicates an error, the conn's * errorMessage is copied. * * Note this is exported --- you wouldn't think an application would need * to build its own PGresults, but this has proven useful in both libpgtcl * and the Perl5 interface, so maybe it's not so unreasonable. + * + * Updated April 2008 - If conn is not NULL, event states will be copied + * from the PGconn to the created PGresult. */ PGresult * PQmakeEmptyPGresult(PGconn *conn, ExecStatusType status) { PGresult *result; *************** *** 157,175 **** --- 183,216 ---- result->errMsg = NULL; result->errFields = NULL; result->null_field[0] = '\0'; result->curBlock = NULL; result->curOffset = 0; result->spaceLeft = 0; + result->nEvents = 0; + result->events = NULL; if (conn) { /* copy connection data we might need for operations on PGresult */ result->noticeHooks = conn->noticeHooks; result->client_encoding = conn->client_encoding; + /* copy events from connection */ + if(conn->nEvents > 0) + { + result->events = dupEvents(conn->events, conn->nEvents); + if(!result->events) + { + PQclear(result); + return NULL; + } + + result->nEvents = conn->nEvents; + } + /* consider copying conn's errorMessage */ switch (status) { case PGRES_EMPTY_QUERY: case PGRES_COMMAND_OK: case PGRES_TUPLES_OK: *************** *** 193,204 **** --- 234,481 ---- } return result; } /* + * PQcopyResult + * Returns a deep copy of the provided 'src' PGresult, which cannot be NULL. + * The 'options' argument controls which portions of the result will or will + * NOT be copied. If this value is 0, the entire result is deep copied. + * The created result is always put into the PGRES_TUPLES_OK status. The + * source result error message is not copied, although cmdStatus is. + * + * Options: + * PG_COPYRES_NO_TUPLES - Do not copy the tuples. This option is + * automatically enabled when PG_COPYRES_USE_ATTRS is set. + * + * PG_COPYRES_USE_ATTRS - Indicates that the 'numAttributes' and 'attDescs' + * arguments should be used as the fields for the created result, rather + * than copying them from the source result. When this option is set, + * the tuples will NOT be copied; behaving identically to setting the + * PG_COPYRES_NO_TUPLES option. One must use PQsetvalue to manually + * add tuples to the returned result. NOTE: numAttributes and attDescs + * arguments are ignored unless this option is set! + * + * PG_COPYRES_NO_EVENTS - Indicates that the source result's + * events should NOT be copied to the created result. + * + * PG_COPYRES_NO_NOTICEHOOKS - Indicates that the source result's + * NoticeHooks should NOT be copied to the created result. + */ + + PGresult * + PQcopyResult(const PGresult *src, int numAttributes, + PGresAttDesc *attDescs, int options) + { + int i; + PGresult *dest; + PGEventResultCopy evt; + + if(!src) + return NULL; + + /* Automatically turn on no_tuples since use_attrs is set. It makes + * no sense to copy tuples into an unknown set of columns. + */ + if(options & PG_COPYRES_USE_ATTRS) + options |= PG_COPYRES_NO_TUPLES; + + /* If use_attrs is set, verify attr arguments. */ + if((options & PG_COPYRES_USE_ATTRS) && (numAttributes <= 0 || !attDescs)) + return NULL; + + dest = PQmakeEmptyPGresult((PGconn *)NULL, PGRES_TUPLES_OK); + if(!dest) + return NULL; + + /* always copy these over. Is cmdStatus useful here? */ + dest->client_encoding = src->client_encoding; + strcpy(dest->cmdStatus, src->cmdStatus); + + /* Wants to copy notice hooks */ + if(!(options & PG_COPYRES_NO_NOTICEHOOKS)) + dest->noticeHooks = src->noticeHooks; + + /* Copy from src result when not supplying attrs */ + if(!(options & PG_COPYRES_USE_ATTRS) && src->numAttributes > 0) + { + numAttributes = src->numAttributes; + attDescs = src->attDescs; + } + + /* copy attrs */ + if(numAttributes > 0) + { + dest->attDescs = (PGresAttDesc *)PQresultAlloc(dest, + numAttributes * sizeof(PGresAttDesc)); + + if(!dest->attDescs) + { + PQclear(dest); + return NULL; + } + + dest->numAttributes = numAttributes; + memcpy(dest->attDescs, attDescs, numAttributes * sizeof(PGresAttDesc)); + + /* resultalloc the attribute names. The above memcpy has the attr + * names pointing at the source result's private memory (or at the + * callers provided attDescs memory). + */ + dest->binary = 1; + for(i=0; i < numAttributes; i++) + { + if(attDescs[i].name) + dest->attDescs[i].name = pqResultStrdup(dest, attDescs[i].name); + else + dest->attDescs[i].name = dest->null_field; + + if(!dest->attDescs[i].name) + { + PQclear(dest); + return NULL; + } + + /* Although deprecated, because results can have text+binary columns, + * its easy enough to deduce so set it for completeness. + */ + if(dest->attDescs[i].format == 0) + dest->binary = 0; + } + } + + /* Wants to copy result tuples: use PQsetvalue(). */ + if(!(options & PG_COPYRES_NO_TUPLES) && src->ntups > 0) + { + int tup, field; + for(tup=0; tup < src->ntups; tup++) + for(field=0; field < src->numAttributes; field++) + PQsetvalue(dest, tup, field, src->tuples[tup][field].value, + src->tuples[tup][field].len); + } + + /* Wants to copy PGEvents. */ + if(!(options & PG_COPYRES_NO_EVENTS) && src->nEvents > 0) + { + dest->events = dupEvents(src->events, src->nEvents); + if(!dest->events) + { + PQclear(dest); + return NULL; + } + + dest->nEvents = src->nEvents; + } + + /* Trigger PGEVT_RESULTCOPY event */ + for(i=0; i < dest->nEvents; i++) + { + evt.src = src; + evt.dest = dest; + if(!dest->events[i].proc(PGEVT_RESULTCOPY, &evt)) + { + PQclear(dest); + return NULL; + } + } + + return dest; + } + + int + PQsetvalue(PGresult *res, int tup_num, int field_num, + char *value, int len) + { + PGresAttValue *attval; + + if(!check_field_number(res, field_num)) + return FALSE; + + /* Invalid tup_num, must be <= ntups */ + if(tup_num > res->ntups) + return FALSE; + + /* need to grow the tuple table */ + if(res->ntups >= res->tupArrSize) + { + int n = res->tupArrSize ? (res->tupArrSize*3)/2 : 64; + PGresAttValue **tups = (PGresAttValue **) + (res->tuples ? realloc(res->tuples, n*sizeof(PGresAttValue *)) : + malloc(n*sizeof(PGresAttValue *))); + + if(!tups) + return FALSE; + + memset(tups + res->tupArrSize, 0, + (n - res->tupArrSize) * sizeof(PGresAttValue *)); + res->tuples = tups; + res->tupArrSize = n; + } + + /* need to allocate a new tuple */ + if(tup_num == res->ntups && !res->tuples[tup_num]) + { + int i; + PGresAttValue *tup = (PGresAttValue *)PQresultAlloc( + res, res->numAttributes * sizeof(PGresAttValue)); + + if(!tup) + return FALSE; + + /* initialize each column to NULL */ + for(i=0; i < res->numAttributes; i++) + { + tup[i].len = NULL_LEN; + tup[i].value = res->null_field; + } + + res->tuples[tup_num] = tup; + res->ntups++; + } + + attval = &res->tuples[tup_num][field_num]; + + /* On top of NULL_LEN, treat a NULL value as a NULL field */ + if(len == NULL_LEN || value == NULL) + { + attval->len = NULL_LEN; + attval->value = res->null_field; + } + else + { + if(len < 0) + len = 0; + + if(len == 0) + { + attval->len = 0; + attval->value = res->null_field; + } + else + { + attval->value = (char *)PQresultAlloc(res, len + 1); + if(!attval->value) + return FALSE; + + attval->len = len; + memcpy(attval->value, value, len); + attval->value[len] = '\0'; + } + } + + return TRUE; + } + + void * + PQresultAlloc(PGresult *res, size_t nBytes) + { + return pqResultAlloc(res, nBytes, TRUE); + } + + /* * pqResultAlloc - * Allocate subsidiary storage for a PGresult. * * nBytes is the amount of space needed for the object. * If isBinary is true, we assume that we need to align the object on * a machine allocation boundary. *************** *** 349,365 **** --- 626,657 ---- * PQclear - * free's the memory associated with a PGresult */ void PQclear(PGresult *res) { + int i; PGresult_data *block; + PGEventResultDestroy evt; if (!res) return; + for(i=0; i < res->nEvents; i++) + { + evt.result = res; + (void)res->events[i].proc(PGEVT_RESULTDESTROY, &evt); + } + + if(res->events) + { + free(res->events); + res->events = NULL; + res->nEvents = 0; + } + /* Free all the subsidiary blocks */ while ((block = res->curBlock) != NULL) { res->curBlock = block->next; free(block); } *************** *** 1190,1202 **** * memory). */ PGresult * PQgetResult(PGconn *conn) { ! PGresult *res; if (!conn) return NULL; /* Parse any available data, if our state permits. */ parseInput(conn); --- 1482,1494 ---- * memory). */ PGresult * PQgetResult(PGconn *conn) { ! PGresult *res=NULL; if (!conn) return NULL; /* Parse any available data, if our state permits. */ parseInput(conn); *************** *** 1265,1276 **** --- 1557,1593 ---- libpq_gettext("unexpected asyncStatus: %d\n"), (int) conn->asyncStatus); res = PQmakeEmptyPGresult(conn, PGRES_FATAL_ERROR); break; } + if(res && (res->resultStatus == PGRES_COMMAND_OK || + res->resultStatus == PGRES_TUPLES_OK || + res->resultStatus == PGRES_EMPTY_QUERY)) + { + int i; + PGEventResultCreate evt; + + for(i=0; i < res->nEvents; i++) + { + evt.conn = conn; + evt.result = res; + + if(!res->events[i].proc(PGEVT_RESULTCREATE, &evt)) + { + char msg[256]; + sprintf(msg, + "PGEventProc %p failed during PGEVT_RESULTCREATE event", + res->events[i].proc); + pqSetResultError(res, msg); + res->resultStatus = PGRES_FATAL_ERROR; + break; + } + } + } + return res; } /* * PQexec Index: src/interfaces/libpq/fe-misc.c =================================================================== RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-misc.c,v retrieving revision 1.133 diff -C6 -r1.133 fe-misc.c *** src/interfaces/libpq/fe-misc.c 1 Jan 2008 19:46:00 -0000 1.133 --- src/interfaces/libpq/fe-misc.c 20 May 2008 04:18:09 -0000 *************** *** 1152,1157 **** --- 1152,1158 ---- } return dgettext("libpq", msgid); } #endif /* ENABLE_NLS */ + Index: src/interfaces/libpq/libpq-fe.h =================================================================== RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/libpq-fe.h,v retrieving revision 1.142 diff -C6 -r1.142 libpq-fe.h *** src/interfaces/libpq/libpq-fe.h 19 Mar 2008 00:39:33 -0000 1.142 --- src/interfaces/libpq/libpq-fe.h 20 May 2008 04:18:09 -0000 *************** *** 25,36 **** --- 25,45 ---- /* * postgres_ext.h defines the backend's externally visible types, * such as Oid. */ #include "postgres_ext.h" + /* ----------------------- + * Options for PQcopyResult + */ + + #define PG_COPYRES_NO_TUPLES 0x01 + #define PG_COPYRES_USE_ATTRS 0x02 + #define PG_COPYRES_NO_EVENTS 0x04 + #define PG_COPYRES_NO_NOTICEHOOKS 0x08 + /* Application-visible enum types */ typedef enum { /* * Although it is okay to add to this list, values which become unused *************** *** 190,201 **** --- 199,225 ---- int *ptr; /* can't use void (dec compiler barfs) */ int integer; } u; } PQArgBlock; /* ---------------- + * PGresAttDesc -- Data about a single attribute (column) of a query result + * ---------------- + */ + typedef struct pgresAttDesc + { + char *name; /* column name */ + Oid tableid; /* source table, if known */ + int columnid; /* source column, if known */ + int format; /* format code for value (text/binary) */ + Oid typid; /* type id */ + int typlen; /* type size */ + int atttypmod; /* type-specific modifier info */ + } PGresAttDesc; + + /* ---------------- * Exported functions of libpq * ---------------- */ /* === in fe-connect.c === */ *************** *** 434,445 **** --- 458,488 ---- * Make an empty PGresult with given status (some apps find this * useful). If conn is not NULL and status indicates an error, the * conn's errorMessage is copied. */ extern PGresult *PQmakeEmptyPGresult(PGconn *conn, ExecStatusType status); + extern PGresult * + PQcopyResult(const PGresult *src, int numAttributes, + PGresAttDesc *attDescs, int options); + + extern void * + PQresultAlloc(PGresult *res, size_t nBytes); + + /* + * Sets the value for a tuple field. The tup_num must be less than or + * equal to PQntuples(res). This function will generate tuples as needed. + * A new tuple is generated when tup_num equals PQntuples(res) and there + * are no fields defined for that tuple. + * + * Returns a non-zero value for success and zero for failure. + */ + extern int + PQsetvalue(PGresult *res, int tup_num, int field_num, + char *value, int len); + /* Quoting strings before inclusion in queries. */ extern size_t PQescapeStringConn(PGconn *conn, char *to, const char *from, size_t length, int *error); extern unsigned char *PQescapeByteaConn(PGconn *conn, Index: src/interfaces/libpq/libpq-int.h =================================================================== RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/libpq-int.h,v retrieving revision 1.130 diff -C6 -r1.130 libpq-int.h *** src/interfaces/libpq/libpq-int.h 16 May 2008 18:30:53 -0000 1.130 --- src/interfaces/libpq/libpq-int.h 20 May 2008 04:18:09 -0000 *************** *** 19,30 **** --- 19,31 ---- #ifndef LIBPQ_INT_H #define LIBPQ_INT_H /* We assume libpq-fe.h has already been included. */ #include "postgres_fe.h" + #include "libpq-events.h" #include <time.h> #include <sys/types.h> #ifndef WIN32 #include <sys/time.h> #endif *************** *** 97,121 **** union pgresult_data { PGresult_data *next; /* link to next block, or NULL */ char space[1]; /* dummy for accessing block as bytes */ }; - /* Data about a single attribute (column) of a query result */ - - typedef struct pgresAttDesc - { - char *name; /* column name */ - Oid tableid; /* source table, if known */ - int columnid; /* source column, if known */ - int format; /* format code for value (text/binary) */ - Oid typid; /* type id */ - int typlen; /* type size */ - int atttypmod; /* type-specific modifier info */ - } PGresAttDesc; - /* Data about a single parameter of a prepared statement */ typedef struct pgresParamDesc { Oid typid; /* type id */ } PGresParamDesc; --- 98,109 ---- *************** *** 159,170 **** --- 147,167 ---- PQnoticeReceiver noticeRec; /* notice message receiver */ void *noticeRecArg; PQnoticeProcessor noticeProc; /* notice message processor */ void *noticeProcArg; } PGNoticeHooks; + typedef struct + { + /* pointer supplied by user */ + void *passThrough; + /* state data, optionally generated by event proc */ + void *data; + PGEventProc proc; + } PGEvent; + struct pg_result { int ntups; int numAttributes; PGresAttDesc *attDescs; PGresAttValue **tuples; /* each PGresTuple is an array of *************** *** 181,192 **** --- 178,193 ---- * These fields are copied from the originating PGconn, so that operations * on the PGresult don't have to reference the PGconn. */ PGNoticeHooks noticeHooks; int client_encoding; /* encoding id */ + /* registered events, copied from conn */ + int nEvents; + PGEvent *events; + /* * Error information (all NULL if not an error result). errMsg is the * "overall" error message returned by PQresultErrorMessage. If we have * per-field info then it is stored in a linked list. */ char *errMsg; /* error message, or NULL if no error */ *************** *** 300,311 **** --- 301,317 ---- /* Optional file to write trace info to */ FILE *Pfdebug; /* Callback procedures for notice message processing */ PGNoticeHooks noticeHooks; + /* registered events via PQregisterEventProc */ + int nEvents; + int eventArrSize; + PGEvent *events; + /* Status indicators */ ConnStatusType status; PGAsyncStatusType asyncStatus; PGTransactionStatusType xactStatus; /* never changes to ACTIVE */ PGQueryClass queryclass; char *last_query; /* last SQL command, or NULL if unknown */
Attachment
On Mon, May 19, 2008 at 8:22 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Andrew Chernow <ac@esilo.com> writes: >> 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. > > This is starting to get there, Interesting you haven't commented on two areas, the actual placement of the event callers in the libpq code (i'm assuming these are ok), and the PQcopyResult/PQsetvalue functions. The latter area introduces some new behavior into standard libpq (imo) so it deserves some scrutiny. merlin
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 -0000 1.166 --- src/interfaces/libpq/Makefile 3 Sep 2008 16:06:49 -0000 *************** *** 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 -0000 1.19 --- src/interfaces/libpq/exports.txt 3 Sep 2008 16:06:49 -0000 *************** *** 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 + PQsetvalue 144 + PQresultAlloc 145 + PQregisterEventProc 146 + PQinstanceData 147 + 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 -0000 1.359 --- src/interfaces/libpq/fe-connect.c 3 Sep 2008 16:06:49 -0000 *************** *** 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); if (conn->pgport) free(conn->pgport); *************** *** 2152,2165 **** PQreset(PGconn *conn) { if (conn) { closePGconn(conn); ! if (connectDBStart(conn)) ! (void) connectDBComplete(conn); } } /* * PQresetStart: --- 2170,2200 ---- PQreset(PGconn *conn) { if (conn) { closePGconn(conn); ! if (connectDBStart(conn) && connectDBComplete(conn)) ! { ! int i; ! PGEventConnReset evt; ! ! for(i=0; i < conn->nEvents; i++) ! { ! evt.conn = conn; ! ! if(!conn->events[i].proc(PGEVT_CONNRESET, &evt)) ! { ! conn->status = CONNECTION_BAD; ! printfPQExpBuffer(&conn->errorMessage, ! libpq_gettext("PGEventProc %p failed during PGEVT_CONNRESET event\n"), ! conn->events[i].proc); ! break; ! } ! } ! } } } /* * PQresetStart: *************** *** 2187,2199 **** * closes the existing connection and makes a new one */ PostgresPollingStatusType PQresetPoll(PGconn *conn) { if (conn) ! return PQconnectPoll(conn); return PGRES_POLLING_FAILED; } /* * PQcancelGet: get a PGcancel structure corresponding to a connection. --- 2222,2258 ---- * closes the existing connection and makes a new one */ PostgresPollingStatusType PQresetPoll(PGconn *conn) { if (conn) ! { ! PostgresPollingStatusType status = PQconnectPoll(conn); ! ! if(status == PGRES_POLLING_OK) ! { ! int i; ! PGEventConnReset evt; ! ! for(i=0; i < conn->nEvents; i++) ! { ! evt.conn = conn; ! ! if(!conn->events[i].proc(PGEVT_CONNRESET, &evt)) ! { ! conn->status = CONNECTION_BAD; ! printfPQExpBuffer(&conn->errorMessage, ! libpq_gettext("PGEventProc %p failed during PGEVT_CONNRESET event\n"), ! conn->events[i].proc); ! return PGRES_POLLING_FAILED; ! } ! } ! } ! ! return status; ! } return PGRES_POLLING_FAILED; } /* * PQcancelGet: get a PGcancel structure corresponding to a connection. Index: src/interfaces/libpq/fe-exec.c =================================================================== RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-exec.c,v retrieving revision 1.196 diff -C6 -r1.196 fe-exec.c *** src/interfaces/libpq/fe-exec.c 23 Jun 2008 21:10:49 -0000 1.196 --- src/interfaces/libpq/fe-exec.c 3 Sep 2008 16:06:50 -0000 *************** *** 60,72 **** int resultFormat); static void parseInput(PGconn *conn); static bool PQexecStart(PGconn *conn); static PGresult *PQexecFinish(PGconn *conn); static int PQsendDescribe(PGconn *conn, char desc_type, const char *desc_target); ! /* ---------------- * Space management for PGresult. * * Formerly, libpq did a separate malloc() for each field of each tuple * returned by a query. This was remarkably expensive --- malloc/free --- 60,72 ---- int resultFormat); static void parseInput(PGconn *conn); static bool PQexecStart(PGconn *conn); 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. * * Formerly, libpq did a separate malloc() for each field of each tuple * returned by a query. This was remarkably expensive --- malloc/free *************** *** 121,141 **** --- 121,167 ---- #define PGRESULT_DATA_BLOCKSIZE 2048 #define PGRESULT_ALIGN_BOUNDARY MAXIMUM_ALIGNOF /* from configure */ #define PGRESULT_BLOCK_OVERHEAD Max(sizeof(PGresult_data), PGRESULT_ALIGN_BOUNDARY) #define PGRESULT_SEP_ALLOC_THRESHOLD (PGRESULT_DATA_BLOCKSIZE / 2) + /* Does not duplicate the event instance data, sets this to NULL */ + static PGEvent * + dupEvents(PGEvent *events, int count) + { + int i; + PGEvent *newEvents; + + if(!events || count <= 0) + return NULL; + + newEvents = (PGEvent *)malloc(count * sizeof(PGEvent)); + if(!newEvents) + return NULL; + + memcpy(newEvents, events, count * sizeof(PGEvent)); + + /* NULL out the data pointer */ + for(i=0; i < count; i++) + newEvents[i].data = NULL; + + return newEvents; + } + /* * PQmakeEmptyPGresult * returns a newly allocated, initialized PGresult with given status. * If conn is not NULL and status indicates an error, the conn's * errorMessage is copied. * * Note this is exported --- you wouldn't think an application would need * to build its own PGresults, but this has proven useful in both libpgtcl * and the Perl5 interface, so maybe it's not so unreasonable. + * + * Updated April 2008 - If conn is not NULL, event states will be copied + * from the PGconn to the created PGresult. */ PGresult * PQmakeEmptyPGresult(PGconn *conn, ExecStatusType status) { PGresult *result; *************** *** 157,175 **** --- 183,216 ---- result->errMsg = NULL; result->errFields = NULL; result->null_field[0] = '\0'; result->curBlock = NULL; result->curOffset = 0; result->spaceLeft = 0; + result->nEvents = 0; + result->events = NULL; if (conn) { /* copy connection data we might need for operations on PGresult */ result->noticeHooks = conn->noticeHooks; result->client_encoding = conn->client_encoding; + /* copy events from connection */ + if(conn->nEvents > 0) + { + result->events = dupEvents(conn->events, conn->nEvents); + if(!result->events) + { + PQclear(result); + return NULL; + } + + result->nEvents = conn->nEvents; + } + /* consider copying conn's errorMessage */ switch (status) { case PGRES_EMPTY_QUERY: case PGRES_COMMAND_OK: case PGRES_TUPLES_OK: *************** *** 192,203 **** --- 233,487 ---- result->client_encoding = PG_SQL_ASCII; } return result; } + /* PQsetResultAttrs + * Set the attributes for a given result. This function fails if there are + * already attributes contained in the provided result. The call is + * ignored if numAttributes is is zero or attDescs is NULL. If the + * function fails, it returns zero. If the function succeeds, it + * returns a non-zero value. + */ + int + PQsetResultAttrs(PGresult *res, int numAttributes, PGresAttDesc *attDescs) + { + int i; + + /* If attrs already exist, they cannot be overwritten. */ + if(!res || res->numAttributes > 0) + return FALSE; + + /* ignore request */ + if(numAttributes <= 0 || !attDescs) + return TRUE; + + res->attDescs = (PGresAttDesc *)PQresultAlloc(res, + numAttributes * sizeof(PGresAttDesc)); + + if(!res->attDescs) + return FALSE; + + res->numAttributes = numAttributes; + memcpy(res->attDescs, attDescs, + numAttributes * sizeof(PGresAttDesc)); + + /* resultalloc the attribute names. The above memcpy has the attr + * names pointing at the source result's private memory (or at the + * callers provided attDescs memory). + */ + res->binary = 1; + for(i=0; i < res->numAttributes; i++) + { + if(res->attDescs[i].name) + res->attDescs[i].name = pqResultStrdup(res, res->attDescs[i].name); + else + res->attDescs[i].name = res->null_field; + + if(!res->attDescs[i].name) + return FALSE; + + /* Although deprecated, because results can have text+binary columns, + * its easy enough to deduce so set it for completeness. + */ + if(res->attDescs[i].format == 0) + res->binary = 0; + } + + return TRUE; + } + + /* + * PQcopyResult + * Returns a deep copy of the provided 'src' PGresult, which cannot be NULL. + * The 'options' argument controls which portions of the result will or will + * NOT be copied. The created result is always put into the + * PGRES_TUPLES_OK status. The source result error message is not copied, + * although cmdStatus is. + * + * To set custom attributes, see PQsetResultAttrs. That function requires + * that there are no attrs contained in the result, so to use that + * function you cannot use the PG_COPYRES_ATTRS or PG_COPYRES_TUPLES + * options with this function. + * + * Options: + * PG_COPYRES_ATTRS - Copy the source result's attributes + * + * PG_COPYRES_TUPLES - Copy the source result's tuples. This implies + * copying the attrs, being how the attrs are needed by the tuples. + * + * PG_COPYRES_EVENTS - Copy the source result's events. + * + * PG_COPYRES_NOTICEHOOKS - Copy the source result's notice hooks. + */ + + PGresult * + PQcopyResult(const PGresult *src, int options) + { + int i; + PGresult *dest; + PGEventResultCopy evt; + + if(!src) + return NULL; + + /* Automatically turn on attrs options because you can't copy tuples + * without copying the attrs. _TUPLES implies _ATTRS. + */ + if(options & PG_COPYRES_TUPLES) + options |= PG_COPYRES_ATTRS; + + dest = PQmakeEmptyPGresult((PGconn *)NULL, PGRES_TUPLES_OK); + if(!dest) + return NULL; + + /* always copy these over. Is cmdStatus useful here? */ + dest->client_encoding = src->client_encoding; + strcpy(dest->cmdStatus, src->cmdStatus); + + /* Wants to copy notice hooks */ + if(options & PG_COPYRES_NOTICEHOOKS) + dest->noticeHooks = src->noticeHooks; + + /* Wants attrs */ + if((options & PG_COPYRES_ATTRS) && + !PQsetResultAttrs(dest, src->numAttributes, src->attDescs)) + { + PQclear(dest); + return NULL; + } + + /* Wants to copy result tuples: use PQsetvalue(). */ + if((options & PG_COPYRES_TUPLES) && src->ntups > 0) + { + int tup, field; + for(tup=0; tup < src->ntups; tup++) + for(field=0; field < src->numAttributes; field++) + PQsetvalue(dest, tup, field, src->tuples[tup][field].value, + src->tuples[tup][field].len); + } + + /* Wants to copy PGEvents. */ + if((options & PG_COPYRES_EVENTS) && src->nEvents > 0) + { + dest->events = dupEvents(src->events, src->nEvents); + if(!dest->events) + { + PQclear(dest); + return NULL; + } + + dest->nEvents = src->nEvents; + } + + /* Trigger PGEVT_RESULTCOPY event */ + for(i=0; i < dest->nEvents; i++) + { + evt.src = src; + evt.dest = dest; + if(!dest->events[i].proc(PGEVT_RESULTCOPY, &evt)) + { + PQclear(dest); + return NULL; + } + } + + return dest; + } + + int + PQsetvalue(PGresult *res, int tup_num, int field_num, + char *value, int len) + { + PGresAttValue *attval; + + if(!check_field_number(res, field_num)) + return FALSE; + + /* Invalid tup_num, must be <= ntups */ + if(tup_num > res->ntups) + return FALSE; + + /* need to grow the tuple table */ + if(res->ntups >= res->tupArrSize) + { + int n = res->tupArrSize ? (res->tupArrSize*3)/2 : 64; + PGresAttValue **tups = (PGresAttValue **) + (res->tuples ? realloc(res->tuples, n*sizeof(PGresAttValue *)) : + malloc(n*sizeof(PGresAttValue *))); + + if(!tups) + return FALSE; + + memset(tups + res->tupArrSize, 0, + (n - res->tupArrSize) * sizeof(PGresAttValue *)); + res->tuples = tups; + res->tupArrSize = n; + } + + /* need to allocate a new tuple */ + if(tup_num == res->ntups && !res->tuples[tup_num]) + { + int i; + PGresAttValue *tup = (PGresAttValue *)PQresultAlloc( + res, res->numAttributes * sizeof(PGresAttValue)); + + if(!tup) + return FALSE; + + /* initialize each column to NULL */ + for(i=0; i < res->numAttributes; i++) + { + tup[i].len = NULL_LEN; + tup[i].value = res->null_field; + } + + res->tuples[tup_num] = tup; + res->ntups++; + } + + attval = &res->tuples[tup_num][field_num]; + + /* On top of NULL_LEN, treat a NULL value as a NULL field */ + if(len == NULL_LEN || value == NULL) + { + attval->len = NULL_LEN; + attval->value = res->null_field; + } + else + { + if(len < 0) + len = 0; + + if(len == 0) + { + attval->len = 0; + attval->value = res->null_field; + } + else + { + attval->value = (char *)PQresultAlloc(res, len + 1); + if(!attval->value) + return FALSE; + + attval->len = len; + memcpy(attval->value, value, len); + attval->value[len] = '\0'; + } + } + + return TRUE; + } + + void * + PQresultAlloc(PGresult *res, size_t nBytes) + { + return pqResultAlloc(res, nBytes, TRUE); + } + /* * pqResultAlloc - * Allocate subsidiary storage for a PGresult. * * nBytes is the amount of space needed for the object. * If isBinary is true, we assume that we need to align the object on *************** *** 349,365 **** --- 633,664 ---- * PQclear - * free's the memory associated with a PGresult */ void PQclear(PGresult *res) { + int i; PGresult_data *block; + PGEventResultDestroy evt; if (!res) return; + for(i=0; i < res->nEvents; i++) + { + evt.result = res; + (void)res->events[i].proc(PGEVT_RESULTDESTROY, &evt); + } + + if(res->events) + { + free(res->events); + res->events = NULL; + res->nEvents = 0; + } + /* Free all the subsidiary blocks */ while ((block = res->curBlock) != NULL) { res->curBlock = block->next; free(block); } *************** *** 1192,1204 **** * memory). */ PGresult * PQgetResult(PGconn *conn) { ! PGresult *res; if (!conn) return NULL; /* Parse any available data, if our state permits. */ parseInput(conn); --- 1491,1503 ---- * memory). */ PGresult * PQgetResult(PGconn *conn) { ! PGresult *res=NULL; if (!conn) return NULL; /* Parse any available data, if our state permits. */ parseInput(conn); *************** *** 1267,1278 **** --- 1566,1602 ---- libpq_gettext("unexpected asyncStatus: %d\n"), (int) conn->asyncStatus); res = PQmakeEmptyPGresult(conn, PGRES_FATAL_ERROR); break; } + if(res && (res->resultStatus == PGRES_COMMAND_OK || + res->resultStatus == PGRES_TUPLES_OK || + res->resultStatus == PGRES_EMPTY_QUERY)) + { + int i; + PGEventResultCreate evt; + + for(i=0; i < res->nEvents; i++) + { + evt.conn = conn; + evt.result = res; + + if(!res->events[i].proc(PGEVT_RESULTCREATE, &evt)) + { + char msg[256]; + sprintf(msg, + "PGEventProc %p failed during PGEVT_RESULTCREATE event", + res->events[i].proc); + pqSetResultError(res, msg); + res->resultStatus = PGRES_FATAL_ERROR; + break; + } + } + } + return res; } /* * PQexec Index: src/interfaces/libpq/libpq-fe.h =================================================================== RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/libpq-fe.h,v retrieving revision 1.142 diff -C6 -r1.142 libpq-fe.h *** src/interfaces/libpq/libpq-fe.h 19 Mar 2008 00:39:33 -0000 1.142 --- src/interfaces/libpq/libpq-fe.h 3 Sep 2008 16:06:50 -0000 *************** *** 25,36 **** --- 25,45 ---- /* * postgres_ext.h defines the backend's externally visible types, * such as Oid. */ #include "postgres_ext.h" + /* ----------------------- + * Options for PQcopyResult + */ + + #define PG_COPYRES_ATTRS 0x01 + #define PG_COPYRES_TUPLES 0x02 /* Implies PG_COPYRES_ATTRS */ + #define PG_COPYRES_EVENTS 0x04 + #define PG_COPYRES_NOTICEHOOKS 0x08 + /* Application-visible enum types */ typedef enum { /* * Although it is okay to add to this list, values which become unused *************** *** 190,201 **** --- 199,225 ---- int *ptr; /* can't use void (dec compiler barfs) */ int integer; } u; } PQArgBlock; /* ---------------- + * PGresAttDesc -- Data about a single attribute (column) of a query result + * ---------------- + */ + typedef struct pgresAttDesc + { + char *name; /* column name */ + Oid tableid; /* source table, if known */ + int columnid; /* source column, if known */ + int format; /* format code for value (text/binary) */ + Oid typid; /* type id */ + int typlen; /* type size */ + int atttypmod; /* type-specific modifier info */ + } PGresAttDesc; + + /* ---------------- * Exported functions of libpq * ---------------- */ /* === in fe-connect.c === */ *************** *** 434,445 **** --- 458,490 ---- * Make an empty PGresult with given status (some apps find this * useful). If conn is not NULL and status indicates an error, the * conn's errorMessage is copied. */ extern PGresult *PQmakeEmptyPGresult(PGconn *conn, ExecStatusType status); + extern PGresult * + PQcopyResult(const PGresult *src, int options); + + extern int + PQsetResultAttrs(PGresult *res, int numAttributes, PGresAttDesc *attDescs); + + extern void * + PQresultAlloc(PGresult *res, size_t nBytes); + + /* + * Sets the value for a tuple field. The tup_num must be less than or + * equal to PQntuples(res). This function will generate tuples as needed. + * A new tuple is generated when tup_num equals PQntuples(res) and there + * are no fields defined for that tuple. + * + * Returns a non-zero value for success and zero for failure. + */ + extern int + PQsetvalue(PGresult *res, int tup_num, int field_num, + char *value, int len); + /* Quoting strings before inclusion in queries. */ extern size_t PQescapeStringConn(PGconn *conn, char *to, const char *from, size_t length, int *error); extern unsigned char *PQescapeByteaConn(PGconn *conn, Index: src/interfaces/libpq/libpq-int.h =================================================================== RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/libpq-int.h,v retrieving revision 1.131 diff -C6 -r1.131 libpq-int.h *** src/interfaces/libpq/libpq-int.h 29 May 2008 22:02:44 -0000 1.131 --- src/interfaces/libpq/libpq-int.h 3 Sep 2008 16:06:50 -0000 *************** *** 19,30 **** --- 19,31 ---- #ifndef LIBPQ_INT_H #define LIBPQ_INT_H /* We assume libpq-fe.h has already been included. */ #include "postgres_fe.h" + #include "libpq-events.h" #include <time.h> #include <sys/types.h> #ifndef WIN32 #include <sys/time.h> #endif *************** *** 97,121 **** union pgresult_data { PGresult_data *next; /* link to next block, or NULL */ char space[1]; /* dummy for accessing block as bytes */ }; - /* Data about a single attribute (column) of a query result */ - - typedef struct pgresAttDesc - { - char *name; /* column name */ - Oid tableid; /* source table, if known */ - int columnid; /* source column, if known */ - int format; /* format code for value (text/binary) */ - Oid typid; /* type id */ - int typlen; /* type size */ - int atttypmod; /* type-specific modifier info */ - } PGresAttDesc; - /* Data about a single parameter of a prepared statement */ typedef struct pgresParamDesc { Oid typid; /* type id */ } PGresParamDesc; --- 98,109 ---- *************** *** 159,170 **** --- 147,167 ---- PQnoticeReceiver noticeRec; /* notice message receiver */ void *noticeRecArg; PQnoticeProcessor noticeProc; /* notice message processor */ void *noticeProcArg; } PGNoticeHooks; + typedef struct + { + /* pointer supplied by user */ + void *passThrough; + /* state (instance) data, optionally generated by event proc */ + void *data; + PGEventProc proc; + } PGEvent; + struct pg_result { int ntups; int numAttributes; PGresAttDesc *attDescs; PGresAttValue **tuples; /* each PGresTuple is an array of *************** *** 181,192 **** --- 178,193 ---- * These fields are copied from the originating PGconn, so that operations * on the PGresult don't have to reference the PGconn. */ PGNoticeHooks noticeHooks; int client_encoding; /* encoding id */ + /* registered events, copied from conn */ + int nEvents; + PGEvent *events; + /* * Error information (all NULL if not an error result). errMsg is the * "overall" error message returned by PQresultErrorMessage. If we have * per-field info then it is stored in a linked list. */ char *errMsg; /* error message, or NULL if no error */ *************** *** 300,311 **** --- 301,317 ---- /* Optional file to write trace info to */ FILE *Pfdebug; /* Callback procedures for notice message processing */ PGNoticeHooks noticeHooks; + /* registered events via PQregisterEventProc */ + int nEvents; + int eventArrSize; + PGEvent *events; + /* Status indicators */ ConnStatusType status; PGAsyncStatusType asyncStatus; PGTransactionStatusType xactStatus; /* never changes to ACTIVE */ PGQueryClass queryclass; char *last_query; /* last SQL command, or NULL if unknown */ Index: src/interfaces/libpq/pthread-win32.c =================================================================== RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/pthread-win32.c,v retrieving revision 1.17 diff -C6 -r1.17 pthread-win32.c *** src/interfaces/libpq/pthread-win32.c 21 May 2008 14:20:48 -0000 1.17 --- src/interfaces/libpq/pthread-win32.c 3 Sep 2008 16:06:50 -0000 *************** *** 2,14 **** * * pthread-win32.c * partial pthread implementation for win32 * * Copyright (c) 2004-2008, PostgreSQL Global Development Group * IDENTIFICATION ! * $PostgreSQL: pgsql/src/interfaces/libpq/pthread-win32.c,v 1.17 2008/05/21 14:20:48 mha Exp $ * *------------------------------------------------------------------------- */ #include "postgres_fe.h" --- 2,14 ---- * * pthread-win32.c * partial pthread implementation for win32 * * Copyright (c) 2004-2008, PostgreSQL Global Development Group * IDENTIFICATION ! * $PostgreSQL: pgsql/src/interfaces/libpq/pthread-win32.c,v 1.16 2008/05/16 18:30:53 mha Exp $ * *------------------------------------------------------------------------- */ #include "postgres_fe.h"
Attachment
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. You didn't merge the other changes I did to your patch. Please use that one as starting point, or merge them into your version. They were minor, sure, but I went some lengths to make them, which means the code kinda has my signoff that way. Please don't waste that work. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
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 -0000 1.166 --- src/interfaces/libpq/Makefile 3 Sep 2008 21:55:06 -0000 *************** *** 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.cstrlcpy.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.cstrlcpy.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 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 -0000 1.19 --- src/interfaces/libpq/exports.txt 3 Sep 2008 21:55:06 -0000 *************** *** 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 + PQsetvalue 144 + PQresultAlloc 145 + PQregisterEventProc 146 + PQinstanceData 147 + 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 -0000 1.359 --- src/interfaces/libpq/fe-connect.c 3 Sep 2008 21:55:06 -0000 *************** *** 1971,1982 **** --- 1971,2002 ---- * 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); + if (conn->events[i].name) + free(conn->events[i].name); + } + + /* 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); *************** *** 2152,2165 **** PQreset(PGconn *conn) { if (conn) { closePGconn(conn); ! if (connectDBStart(conn)) ! (void) connectDBComplete(conn); } } /* * PQresetStart: --- 2172,2207 ---- PQreset(PGconn *conn) { if (conn) { closePGconn(conn); ! if (connectDBStart(conn) && connectDBComplete(conn)) ! { ! int i; ! PGEventConnReset evt; ! ! for (i = 0; i < conn->nEvents; i++) ! { ! evt.conn = conn; ! ! if (!conn->events[i].proc(PGEVT_CONNRESET, &evt)) ! { ! conn->status = CONNECTION_BAD; ! if (conn->events[i].name) ! printfPQExpBuffer(&conn->errorMessage, ! libpq_gettext("PGEventProc \"%s\" failed during PGEVT_CONNRESET event\n"), ! conn->events[i].name); ! else ! printfPQExpBuffer(&conn->errorMessage, ! libpq_gettext("PGEventProc \"addr:%p\" failed during PGEVT_CONNRESET event\n"), ! conn->events[i].proc); ! break; ! } ! } ! } } } /* * PQresetStart: *************** *** 2187,2199 **** * closes the existing connection and makes a new one */ PostgresPollingStatusType PQresetPoll(PGconn *conn) { if (conn) ! return PQconnectPoll(conn); return PGRES_POLLING_FAILED; } /* * PQcancelGet: get a PGcancel structure corresponding to a connection. --- 2229,2270 ---- * closes the existing connection and makes a new one */ PostgresPollingStatusType PQresetPoll(PGconn *conn) { if (conn) ! { ! PostgresPollingStatusType status = PQconnectPoll(conn); ! ! if (status == PGRES_POLLING_OK) ! { ! int i; ! PGEventConnReset evt; ! ! for (i = 0; i < conn->nEvents; i++) ! { ! evt.conn = conn; ! ! if (!conn->events[i].proc(PGEVT_CONNRESET, &evt)) ! { ! conn->status = CONNECTION_BAD; ! if (conn->events[i].name) ! printfPQExpBuffer(&conn->errorMessage, ! libpq_gettext("PGEventProc \"%s\" failed during PGEVT_CONNRESET event\n"), ! conn->events[i].name); ! else ! printfPQExpBuffer(&conn->errorMessage, ! libpq_gettext("PGEventProc \"addr:%p\" failed during PGEVT_CONNRESET event\n"), ! conn->events[i].proc); ! return PGRES_POLLING_FAILED; ! } ! } ! } ! ! return status; ! } return PGRES_POLLING_FAILED; } /* * PQcancelGet: get a PGcancel structure corresponding to a connection. Index: src/interfaces/libpq/fe-exec.c =================================================================== RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-exec.c,v retrieving revision 1.196 diff -C6 -r1.196 fe-exec.c *** src/interfaces/libpq/fe-exec.c 23 Jun 2008 21:10:49 -0000 1.196 --- src/interfaces/libpq/fe-exec.c 3 Sep 2008 21:55:07 -0000 *************** *** 60,72 **** int resultFormat); static void parseInput(PGconn *conn); static bool PQexecStart(PGconn *conn); static PGresult *PQexecFinish(PGconn *conn); static int PQsendDescribe(PGconn *conn, char desc_type, const char *desc_target); ! /* ---------------- * Space management for PGresult. * * Formerly, libpq did a separate malloc() for each field of each tuple * returned by a query. This was remarkably expensive --- malloc/free --- 60,72 ---- int resultFormat); static void parseInput(PGconn *conn); static bool PQexecStart(PGconn *conn); 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. * * Formerly, libpq did a separate malloc() for each field of each tuple * returned by a query. This was remarkably expensive --- malloc/free *************** *** 121,141 **** --- 121,171 ---- #define PGRESULT_DATA_BLOCKSIZE 2048 #define PGRESULT_ALIGN_BOUNDARY MAXIMUM_ALIGNOF /* from configure */ #define PGRESULT_BLOCK_OVERHEAD Max(sizeof(PGresult_data), PGRESULT_ALIGN_BOUNDARY) #define PGRESULT_SEP_ALLOC_THRESHOLD (PGRESULT_DATA_BLOCKSIZE / 2) + /* Does not duplicate the event instance data, sets this to NULL */ + static PGEvent * + dupEvents(PGEvent *events, int count) + { + int i; + PGEvent *newEvents; + + if (!events || count <= 0) + return NULL; + + newEvents = (PGEvent *) malloc(count * sizeof(PGEvent)); + if (!newEvents) + return NULL; + + memcpy(newEvents, events, count * sizeof(PGEvent)); + + /* NULL out the data pointer and deep copy name */ + for (i = 0; i < count; i++) + { + if (newEvents[i].name) + newEvents[i].name = strdup(newEvents[i].name); + newEvents[i].data = NULL; + } + + return newEvents; + } + /* * PQmakeEmptyPGresult * returns a newly allocated, initialized PGresult with given status. * If conn is not NULL and status indicates an error, the conn's * errorMessage is copied. * * Note this is exported --- you wouldn't think an application would need * to build its own PGresults, but this has proven useful in both libpgtcl * and the Perl5 interface, so maybe it's not so unreasonable. + * + * Updated April 2008 - If conn is not NULL, event states will be copied + * from the PGconn to the created PGresult. */ PGresult * PQmakeEmptyPGresult(PGconn *conn, ExecStatusType status) { PGresult *result; *************** *** 157,175 **** --- 187,220 ---- result->errMsg = NULL; result->errFields = NULL; result->null_field[0] = '\0'; result->curBlock = NULL; result->curOffset = 0; result->spaceLeft = 0; + result->nEvents = 0; + result->events = NULL; if (conn) { /* copy connection data we might need for operations on PGresult */ result->noticeHooks = conn->noticeHooks; result->client_encoding = conn->client_encoding; + /* copy events from connection */ + if (conn->nEvents > 0) + { + result->events = dupEvents(conn->events, conn->nEvents); + if (!result->events) + { + PQclear(result); + return NULL; + } + + result->nEvents = conn->nEvents; + } + /* consider copying conn's errorMessage */ switch (status) { case PGRES_EMPTY_QUERY: case PGRES_COMMAND_OK: case PGRES_TUPLES_OK: *************** *** 192,203 **** --- 237,496 ---- result->client_encoding = PG_SQL_ASCII; } return result; } + /* PQsetResultAttrs + * Set the attributes for a given result. This function fails if there are + * already attributes contained in the provided result. The call is + * ignored if numAttributes is is zero or attDescs is NULL. If the + * function fails, it returns zero. If the function succeeds, it + * returns a non-zero value. + */ + int + PQsetResultAttrs(PGresult *res, int numAttributes, PGresAttDesc *attDescs) + { + int i; + + /* If attrs already exist, they cannot be overwritten. */ + if (!res || res->numAttributes > 0) + return FALSE; + + /* ignore request */ + if (numAttributes <= 0 || !attDescs) + return TRUE; + + res->attDescs = (PGresAttDesc *) PQresultAlloc(res, + numAttributes * sizeof(PGresAttDesc)); + + if (!res->attDescs) + return FALSE; + + res->numAttributes = numAttributes; + memcpy(res->attDescs, attDescs, + numAttributes * sizeof(PGresAttDesc)); + + /* resultalloc the attribute names. The above memcpy has the attr + * names pointing at the source result's private memory (or at the + * callers provided attDescs memory). + */ + res->binary = 1; + for (i = 0; i < res->numAttributes; i++) + { + if (res->attDescs[i].name) + res->attDescs[i].name = pqResultStrdup(res, res->attDescs[i].name); + else + res->attDescs[i].name = res->null_field; + + if (!res->attDescs[i].name) + return FALSE; + + /* Although deprecated, because results can have text+binary columns, + * its easy enough to deduce so set it for completeness. + */ + if (res->attDescs[i].format == 0) + res->binary = 0; + } + + return TRUE; + } + + /* + * PQcopyResult + * Returns a deep copy of the provided 'src' PGresult, which cannot be NULL. + * The 'options' argument controls which portions of the result will or will + * NOT be copied. The created result is always put into the + * PGRES_TUPLES_OK status. The source result error message is not copied, + * although cmdStatus is. + * + * To set custom attributes, see PQsetResultAttrs. That function requires + * that there are no attrs contained in the result, so to use that + * function you cannot use the PG_COPYRES_ATTRS or PG_COPYRES_TUPLES + * options with this function. + * + * Options: + * PG_COPYRES_ATTRS - Copy the source result's attributes + * + * PG_COPYRES_TUPLES - Copy the source result's tuples. This implies + * copying the attrs, being how the attrs are needed by the tuples. + * + * PG_COPYRES_EVENTS - Copy the source result's events. + * + * PG_COPYRES_NOTICEHOOKS - Copy the source result's notice hooks. + */ + + PGresult * + PQcopyResult(const PGresult *src, int options) + { + int i; + PGresult *dest; + PGEventResultCopy evt; + + if (!src) + return NULL; + + /* Automatically turn on attrs options because you can't copy tuples + * without copying the attrs. _TUPLES implies _ATTRS. + */ + if (options & PG_COPYRES_TUPLES) + options |= PG_COPYRES_ATTRS; + + dest = PQmakeEmptyPGresult((PGconn *)NULL, PGRES_TUPLES_OK); + if (!dest) + return NULL; + + /* always copy these over. Is cmdStatus useful here? */ + dest->client_encoding = src->client_encoding; + strcpy(dest->cmdStatus, src->cmdStatus); + + /* Wants to copy notice hooks */ + if (options & PG_COPYRES_NOTICEHOOKS) + dest->noticeHooks = src->noticeHooks; + + /* Wants attrs */ + if ((options & PG_COPYRES_ATTRS) && + !PQsetResultAttrs(dest, src->numAttributes, src->attDescs)) + { + PQclear(dest); + return NULL; + } + + /* Wants to copy result tuples: use PQsetvalue(). */ + if ((options & PG_COPYRES_TUPLES) && src->ntups > 0) + { + int tup, field; + for (tup = 0; tup < src->ntups; tup++) + for (field = 0; field < src->numAttributes; field++) + PQsetvalue(dest, tup, field, src->tuples[tup][field].value, + src->tuples[tup][field].len); + } + + /* Wants to copy PGEvents. */ + if ((options & PG_COPYRES_EVENTS) && src->nEvents > 0) + { + dest->events = dupEvents(src->events, src->nEvents); + if (!dest->events) + { + PQclear(dest); + return NULL; + } + + dest->nEvents = src->nEvents; + } + + /* Trigger PGEVT_RESULTCOPY event */ + for (i = 0; i < dest->nEvents; i++) + { + evt.src = src; + evt.dest = dest; + if (!dest->events[i].proc(PGEVT_RESULTCOPY, &evt)) + { + PQclear(dest); + return NULL; + } + } + + return dest; + } + + int + PQsetvalue(PGresult *res, int tup_num, int field_num, + char *value, int len) + { + PGresAttValue *attval; + + if (!check_field_number(res, field_num)) + return FALSE; + + /* Invalid tup_num, must be <= ntups */ + if (tup_num > res->ntups) + return FALSE; + + /* need to grow the tuple table */ + if (res->ntups >= res->tupArrSize) + { + int n = res->tupArrSize ? res->tupArrSize * 2 : 128; + PGresAttValue **tups; + + if (res->tuples) + tups = (PGresAttValue **) realloc(res->tuples, n * sizeof(PGresAttValue *)); + else + tups = (PGresAttValue **) malloc(n * sizeof(PGresAttValue *)); + + if (!tups) + return FALSE; + + memset(tups + res->tupArrSize, 0, + (n - res->tupArrSize) * sizeof(PGresAttValue *)); + res->tuples = tups; + res->tupArrSize = n; + } + + /* need to allocate a new tuple */ + if (tup_num == res->ntups && !res->tuples[tup_num]) + { + int i; + PGresAttValue *tup; + + tup = (PGresAttValue *) pqResultAlloc(res, + res->numAttributes * sizeof(PGresAttValue), TRUE); + + if (!tup) + return FALSE; + + /* initialize each column to NULL */ + for (i = 0; i < res->numAttributes; i++) + { + tup[i].len = NULL_LEN; + tup[i].value = res->null_field; + } + + res->tuples[tup_num] = tup; + res->ntups++; + } + + attval = &res->tuples[tup_num][field_num]; + + /* On top of NULL_LEN, treat a NULL value as a NULL field */ + if (len == NULL_LEN || value == NULL) + { + attval->len = NULL_LEN; + attval->value = res->null_field; + } + else + { + if (len < 0) + len = 0; + + if (len == 0) + { + attval->len = 0; + attval->value = res->null_field; + } + else + { + attval->value = (char *) PQresultAlloc(res, len + 1); + if (!attval->value) + return FALSE; + + attval->len = len; + memcpy(attval->value, value, len); + attval->value[len] = '\0'; + } + } + + return TRUE; + } + + void * + PQresultAlloc(PGresult *res, size_t nBytes) + { + return pqResultAlloc(res, nBytes, TRUE); + } + /* * pqResultAlloc - * Allocate subsidiary storage for a PGresult. * * nBytes is the amount of space needed for the object. * If isBinary is true, we assume that we need to align the object on *************** *** 349,365 **** --- 642,675 ---- * PQclear - * free's the memory associated with a PGresult */ void PQclear(PGresult *res) { + int i; PGresult_data *block; + PGEventResultDestroy evt; if (!res) return; + for (i = 0; i < res->nEvents; i++) + { + evt.result = res; + (void)res->events[i].proc(PGEVT_RESULTDESTROY, &evt); + if (res->events[i].name) + free(res->events[i].name); + } + + if (res->events) + { + free(res->events); + res->events = NULL; + res->nEvents = 0; + } + /* Free all the subsidiary blocks */ while ((block = res->curBlock) != NULL) { res->curBlock = block->next; free(block); } *************** *** 1192,1204 **** * memory). */ PGresult * PQgetResult(PGconn *conn) { ! PGresult *res; if (!conn) return NULL; /* Parse any available data, if our state permits. */ parseInput(conn); --- 1502,1514 ---- * memory). */ PGresult * PQgetResult(PGconn *conn) { ! PGresult *res=NULL; if (!conn) return NULL; /* Parse any available data, if our state permits. */ parseInput(conn); *************** *** 1267,1278 **** --- 1577,1621 ---- libpq_gettext("unexpected asyncStatus: %d\n"), (int) conn->asyncStatus); res = PQmakeEmptyPGresult(conn, PGRES_FATAL_ERROR); break; } + if (res && res->nEvents > 0 && + (res->resultStatus == PGRES_COMMAND_OK || + res->resultStatus == PGRES_TUPLES_OK || + res->resultStatus == PGRES_EMPTY_QUERY)) + { + int i; + PGEventResultCreate evt; + + for (i = 0; i < res->nEvents; i++) + { + evt.conn = conn; + evt.result = res; + + if (!res->events[i].proc(PGEVT_RESULTCREATE, &evt)) + { + char msg[256]; + + if (res->events[i].name) + sprintf(msg, + "PGEventProc \"%s\" failed during PGEVT_RESULTCREATE event", + res->events[i].name); + else + sprintf(msg, + "PGEventProc \"addr:%p\" failed during PGEVT_RESULTCREATE event", + res->events[i].proc); + + pqSetResultError(res, msg); + res->resultStatus = PGRES_FATAL_ERROR; + break; + } + } + } + return res; } /* * PQexec Index: src/interfaces/libpq/libpq-fe.h =================================================================== RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/libpq-fe.h,v retrieving revision 1.142 diff -C6 -r1.142 libpq-fe.h *** src/interfaces/libpq/libpq-fe.h 19 Mar 2008 00:39:33 -0000 1.142 --- src/interfaces/libpq/libpq-fe.h 3 Sep 2008 21:55:07 -0000 *************** *** 25,36 **** --- 25,45 ---- /* * postgres_ext.h defines the backend's externally visible types, * such as Oid. */ #include "postgres_ext.h" + /* ----------------------- + * Options for PQcopyResult + */ + + #define PG_COPYRES_ATTRS 0x01 + #define PG_COPYRES_TUPLES 0x02 /* Implies PG_COPYRES_ATTRS */ + #define PG_COPYRES_EVENTS 0x04 + #define PG_COPYRES_NOTICEHOOKS 0x08 + /* Application-visible enum types */ typedef enum { /* * Although it is okay to add to this list, values which become unused *************** *** 190,201 **** --- 199,225 ---- int *ptr; /* can't use void (dec compiler barfs) */ int integer; } u; } PQArgBlock; /* ---------------- + * PGresAttDesc -- Data about a single attribute (column) of a query result + * ---------------- + */ + typedef struct pgresAttDesc + { + char *name; /* column name */ + Oid tableid; /* source table, if known */ + int columnid; /* source column, if known */ + int format; /* format code for value (text/binary) */ + Oid typid; /* type id */ + int typlen; /* type size */ + int atttypmod; /* type-specific modifier info */ + } PGresAttDesc; + + /* ---------------- * Exported functions of libpq * ---------------- */ /* === in fe-connect.c === */ *************** *** 434,445 **** --- 458,489 ---- * Make an empty PGresult with given status (some apps find this * useful). If conn is not NULL and status indicates an error, the * conn's errorMessage is copied. */ extern PGresult *PQmakeEmptyPGresult(PGconn *conn, ExecStatusType status); + extern PGresult * + PQcopyResult(const PGresult *src, int options); + + extern int + PQsetResultAttrs(PGresult *res, int numAttributes, PGresAttDesc *attDescs); + + extern void * + PQresultAlloc(PGresult *res, size_t nBytes); + + /* + * Sets the value for a tuple field. The tup_num must be less than or + * equal to PQntuples(res). This function will generate tuples as needed. + * A new tuple is generated when tup_num equals PQntuples(res) and there + * are no fields defined for that tuple. + * + * Returns a non-zero value for success and zero for failure. + */ + extern int + PQsetvalue(PGresult *res, int tup_num, int field_num, char *value, int len); + /* Quoting strings before inclusion in queries. */ extern size_t PQescapeStringConn(PGconn *conn, char *to, const char *from, size_t length, int *error); extern unsigned char *PQescapeByteaConn(PGconn *conn, Index: src/interfaces/libpq/libpq-int.h =================================================================== RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/libpq-int.h,v retrieving revision 1.131 diff -C6 -r1.131 libpq-int.h *** src/interfaces/libpq/libpq-int.h 29 May 2008 22:02:44 -0000 1.131 --- src/interfaces/libpq/libpq-int.h 3 Sep 2008 21:55:07 -0000 *************** *** 19,30 **** --- 19,31 ---- #ifndef LIBPQ_INT_H #define LIBPQ_INT_H /* We assume libpq-fe.h has already been included. */ #include "postgres_fe.h" + #include "libpq-events.h" #include <time.h> #include <sys/types.h> #ifndef WIN32 #include <sys/time.h> #endif *************** *** 97,121 **** union pgresult_data { PGresult_data *next; /* link to next block, or NULL */ char space[1]; /* dummy for accessing block as bytes */ }; - /* Data about a single attribute (column) of a query result */ - - typedef struct pgresAttDesc - { - char *name; /* column name */ - Oid tableid; /* source table, if known */ - int columnid; /* source column, if known */ - int format; /* format code for value (text/binary) */ - Oid typid; /* type id */ - int typlen; /* type size */ - int atttypmod; /* type-specific modifier info */ - } PGresAttDesc; - /* Data about a single parameter of a prepared statement */ typedef struct pgresParamDesc { Oid typid; /* type id */ } PGresParamDesc; --- 98,109 ---- *************** *** 159,170 **** --- 147,166 ---- PQnoticeReceiver noticeRec; /* notice message receiver */ void *noticeRecArg; PQnoticeProcessor noticeProc; /* notice message processor */ void *noticeProcArg; } PGNoticeHooks; + typedef struct + { + char *name; /* for error messages */ + void *passThrough; /* pointer supplied by user */ + void *data; /* state (instance) data, optionally generated by event proc */ + PGEventProc proc; /* the function to call on events */ + } PGEvent; + struct pg_result { int ntups; int numAttributes; PGresAttDesc *attDescs; PGresAttValue **tuples; /* each PGresTuple is an array of *************** *** 181,192 **** --- 177,192 ---- * These fields are copied from the originating PGconn, so that operations * on the PGresult don't have to reference the PGconn. */ PGNoticeHooks noticeHooks; int client_encoding; /* encoding id */ + /* registered events, copied from conn */ + int nEvents; + PGEvent *events; + /* * Error information (all NULL if not an error result). errMsg is the * "overall" error message returned by PQresultErrorMessage. If we have * per-field info then it is stored in a linked list. */ char *errMsg; /* error message, or NULL if no error */ *************** *** 300,311 **** --- 300,316 ---- /* Optional file to write trace info to */ FILE *Pfdebug; /* Callback procedures for notice message processing */ PGNoticeHooks noticeHooks; + /* registered events via PQregisterEventProc */ + int nEvents; + int eventArrSize; + PGEvent *events; + /* Status indicators */ ConnStatusType status; PGAsyncStatusType asyncStatus; PGTransactionStatusType xactStatus; /* never changes to ACTIVE */ PGQueryClass queryclass; char *last_query; /* last SQL command, or NULL if unknown */ Index: src/interfaces/libpq/pthread-win32.c =================================================================== RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/pthread-win32.c,v retrieving revision 1.17 diff -C6 -r1.17 pthread-win32.c *** src/interfaces/libpq/pthread-win32.c 21 May 2008 14:20:48 -0000 1.17 --- src/interfaces/libpq/pthread-win32.c 3 Sep 2008 21:55:07 -0000 *************** *** 2,14 **** * * pthread-win32.c * partial pthread implementation for win32 * * Copyright (c) 2004-2008, PostgreSQL Global Development Group * IDENTIFICATION ! * $PostgreSQL: pgsql/src/interfaces/libpq/pthread-win32.c,v 1.17 2008/05/21 14:20:48 mha Exp $ * *------------------------------------------------------------------------- */ #include "postgres_fe.h" --- 2,14 ---- * * pthread-win32.c * partial pthread implementation for win32 * * Copyright (c) 2004-2008, PostgreSQL Global Development Group * IDENTIFICATION ! * $PostgreSQL: pgsql/src/interfaces/libpq/pthread-win32.c,v 1.16 2008/05/16 18:30:53 mha Exp $ * *------------------------------------------------------------------------- */ #include "postgres_fe.h"