Re: libpq patch for pqtypes hook api and PGresult creation

> 
> Should the hook only be called when the conn or result was 
 > successfull or should the hooks be called for failed
> connections/commands as well?
> 

ISTM that the hooks should be called on success and error (doesn't 
include cases where a NULL conn or result is returned).  I think this 
makes things more useful.  If the hooker (pun intended) isn't interested 
in error results or conns, it can easily ignore them.

connCreate: The best solution I found was to hook into PQconnectPoll. 
This required making the current PQconnectPoll a static named 
connectPoll and making PQconnectPoll a wrapper to it.  The wrapper just 
calls connectPoll and checks the status for hook points.

resultCreate: PQgetResult seemed like the best place.  I only notify the 
hooks if the resultStatus is NOT copyin or copyout.

I diff'd fe-connect.c and fe-exec.c against cvs which shows these changes.

-- 
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
Index: fe-connect.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.357
diff -C6 -r1.357 fe-connect.c
*** fe-connect.c	31 Mar 2008 02:43:14 -0000	1.357
--- fe-connect.c	12 Apr 2008 13:22:30 -0000
***************
*** 240,252 ****
  static int parseServiceInfo(PQconninfoOption *options,
  				 PQExpBuffer errorMessage);
  static char *pwdfMatchesString(char *buf, char *token);
  static char *PasswordFromFile(char *hostname, char *port, char *dbname,
  				 char *username);
  static void default_threadlock(int acquire);
! 
  
  /* global variable because fe-auth.c needs to access it */
  pgthreadlock_t pg_g_threadlock = default_threadlock;
  
  
  /*
--- 240,252 ----
  static int parseServiceInfo(PQconninfoOption *options,
  				 PQExpBuffer errorMessage);
  static char *pwdfMatchesString(char *buf, char *token);
  static char *PasswordFromFile(char *hostname, char *port, char *dbname,
  				 char *username);
  static void default_threadlock(int acquire);
! static void notifyConnCreateHooks(PGconn *conn);
  
  /* global variable because fe-auth.c needs to access it */
  pgthreadlock_t pg_g_threadlock = default_threadlock;
  
  
  /*
***************
*** 891,903 ****
--- 891,907 ----
  connectDBComplete(PGconn *conn)
  {
  	PostgresPollingStatusType flag = PGRES_POLLING_WRITING;
  	time_t		finish_time = ((time_t) -1);
  
  	if (conn == NULL || conn->status == CONNECTION_BAD)
+ 	{
+ 		if(conn)
+ 			notifyConnCreateHooks(conn);
  		return 0;
+ 	}
  
  	/*
  	 * Set up a time limit, if connect_timeout isn't zero.
  	 */
  	if (conn->connect_timeout != NULL)
  	{
***************
*** 979,992 ****
   *	 o	If your backend wants to use Kerberos authentication then you must
   *		supply both a host name and a host address, otherwise this function
   *		may block on gethostname.
   *
   * ----------------
   */
! PostgresPollingStatusType
! PQconnectPoll(PGconn *conn)
  {
  	PGresult   *res;
  	char		sebuf[256];
  
  	if (conn == NULL)
  		return PGRES_POLLING_FAILED;
--- 983,997 ----
   *	 o	If your backend wants to use Kerberos authentication then you must
   *		supply both a host name and a host address, otherwise this function
   *		may block on gethostname.
   *
   * ----------------
   */
! 
! static PostgresPollingStatusType
! connectPoll(PGconn *conn)
  {
  	PGresult   *res;
  	char		sebuf[256];
  
  	if (conn == NULL)
  		return PGRES_POLLING_FAILED;
***************
*** 1875,1886 ****
--- 1880,1908 ----
  	 * the connection structure must be freed).
  	 */
  	conn->status = CONNECTION_BAD;
  	return PGRES_POLLING_FAILED;
  }
  
+ /* See connectPoll.  All this does is wrap the real PQconnectPoll so
+  * we can trap PGRES_POLLING_OK or PGRES_POLLING_FAILED statuses.  This
+  * could be done in the real connectPoll, but that requires littering the
+  * function with notifyConnCreateHooks calls because there are many
+  * places the function returns a status.  This solution is much less
+  * obtrusive and avoids messing with the black magic of connectPoll.
+  */
+ PostgresPollingStatusType
+ PQconnectPoll(PGconn *conn)
+ {
+ 	PostgresPollingStatusType status = connectPoll(conn);
+ 
+ 	if(status == PGRES_POLLING_OK || status == PGRES_POLLING_FAILED)
+ 		notifyConnCreateHooks(conn);
+ 
+ 	return status;
+ }
  
  /*
   * makeEmptyPGconn
   *	 - create a PGconn data structure with (as yet) no interesting data
   */
  static PGconn *
***************
*** 1970,1981 ****
--- 1992,2015 ----
   * release data that is to be held for the life of the PGconn structure.
   * If a value ought to be cleared/freed during PQreset(), do it there not here.
   */
  static void
  freePGconn(PGconn *conn)
  {
+ 	int objHooksCount;
+ 	const PGobjectHooks *objHooks;
+ 
+ 	if((objHooksCount = pqGetObjectHooks(&objHooks)) > 0)
+ 	{
+ 		int i;
+ 
+ 		for(i=0; i < objHooksCount; i++)
+ 			if(objHooks[i].connDestroy)
+ 				objHooks[i].connDestroy(objHooks[i].name, (const PGconn *)conn);
+ 	}
+ 	pqReleaseObjectHooks();
  	if (conn->pghost)
  		free(conn->pghost);
  	if (conn->pghostaddr)
  		free(conn->pghostaddr);
  	if (conn->pgport)
  		free(conn->pgport);
***************
*** 3843,3854 ****
--- 3877,3904 ----
  		pthread_mutex_lock(&singlethread_lock);
  	else
  		pthread_mutex_unlock(&singlethread_lock);
  #endif
  }
  
+ /* Calls the connCreate hook. */
+ static void notifyConnCreateHooks(PGconn *conn)
+ {
+ 	int objHooksCount;
+ 	const PGobjectHooks *objHooks;
+ 
+ 	if((objHooksCount = pqGetObjectHooks(&objHooks)) > 0)
+ 	{
+ 		int i;
+ 		for(i=0; i < objHooksCount; i++)
+ 			if(objHooks[i].connCreate)
+ 				objHooks[i].connCreate(objHooks[i].name, (const PGconn *)conn);
+ 	}
+ 
+ 	pqReleaseObjectHooks();
+ }
  pgthreadlock_t
  PQregisterThreadLock(pgthreadlock_t newhandler)
  {
  	pgthreadlock_t prev = pg_g_threadlock;
  
  	if (newhandler)
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	12 Apr 2008 13:22:31 -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
***************
*** 192,203 ****
--- 192,331 ----
  		result->client_encoding = PG_SQL_ASCII;
  	}
  
  	return result;
  }
  
+ PGresult *PQmakeResult(
+   PGconn *conn,
+   int numAttributes,
+   PGresAttDesc *attDescs)
+ {
+ 	int i;
+ 	PGresult *res;
+ 
+ 	if(numAttributes <= 0 || !attDescs)
+ 		return NULL;
+ 
+ 	res = PQmakeEmptyPGresult(conn, PGRES_TUPLES_OK);
+ 	if(!res)
+ 		return NULL;
+ 
+ 	res->attDescs = (PGresAttDesc *)
+ 		pqResultAlloc(res, numAttributes * sizeof(PGresAttDesc), TRUE);
+ 
+ 	if(!res->attDescs)
+ 	{
+ 		PQclear(res);
+ 		return NULL;
+ 	}
+ 
+ 	res->numAttributes = numAttributes;
+ 	memcpy(res->attDescs, attDescs, numAttributes * sizeof(PGresAttDesc));
+ 
+ 	/* resultalloc the attribute names. */
+ 	res->binary = 1;
+ 	for(i=0; i < numAttributes; i++)
+ 	{
+ 		if(attDescs[i].name)
+ 			res->attDescs[i].name = pqResultStrdup(res, attDescs[i].name);
+ 		else
+ 			res->attDescs[i].name = res->null_field;
+ 
+ 		if(!res->attDescs[i].name)
+ 		{
+ 			PQclear(res);
+ 			return NULL;
+ 		}
+ 
+ 		/* 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 res;
+ }
+ 
+ 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;
+ 
+ 		res->tuples = tups;
+ 		res->tupArrSize = n;
+ 	}
+ 
+ 	/* new 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), 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;
+ 
+ 		attval->value = (char *)pqResultAlloc(res, len + 1, TRUE);
+ 		if(!attval->value)
+ 			return FALSE;
+ 
+ 		attval->len = len;
+ 		memcpy(attval->value, value, len);
+ 		attval->value[len] = '\0';
+ 	}
+ 
+ 	return 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
***************
*** 350,365 ****
--- 478,504 ----
   *	  free's the memory associated with a PGresult
   */
  void
  PQclear(PGresult *res)
  {
  	PGresult_data *block;
+ 	int objHooksCount;
+ 	const PGobjectHooks *objHooks;
  
  	if (!res)
  		return;
  
+ 	if((objHooksCount = pqGetObjectHooks(&objHooks)) > 0)
+ 	{
+ 		int i;
+ 
+ 		for(i=0; i < objHooksCount; i++)
+ 			if(objHooks[i].resultDestroy)
+ 				objHooks[i].resultDestroy(objHooks[i].name, (const PGresult *)res);
+ 	}
+ 	pqReleaseObjectHooks();
  	/* Free all the subsidiary blocks */
  	while ((block = res->curBlock) != NULL)
  	{
  		res->curBlock = block->next;
  		free(block);
  	}
***************
*** 1179,1190 ****
--- 1318,1346 ----
  	parseInput(conn);
  
  	/* PQgetResult will return immediately in all states except BUSY. */
  	return conn->asyncStatus == PGASYNC_BUSY;
  }
  
+ /* Used by PQgetResult */
+ static void notifyResultCreateHooks(PGconn *conn, PGresult *res)
+ {
+ 	int objHooksCount;
+ 	const PGobjectHooks *objHooks;
+ 
+ 	if((objHooksCount = pqGetObjectHooks(&objHooks)) > 0)
+ 	{
+ 		int i;
+ 		for(i=0; i < objHooksCount; i++)
+ 			if(objHooks[i].resultCreate)
+ 				objHooks[i].resultCreate(objHooks[i].name, (const PGconn *)conn,
+ 					(const PGresult *)res);
+ 	}
+ 
+ 	pqReleaseObjectHooks();
+ }
  
  /*
   * 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).
***************
*** 1227,1239 ****
  			/*
  			 * conn->errorMessage has been set by pqWait or pqReadData. We
  			 * want to append it to any already-received error message.
  			 */
  			pqSaveErrorResult(conn);
  			conn->asyncStatus = PGASYNC_IDLE;
! 			return pqPrepareAsyncResult(conn);
  		}
  
  		/* Parse it. */
  		parseInput(conn);
  	}
  
--- 1383,1403 ----
  			/*
  			 * conn->errorMessage has been set by pqWait or pqReadData. We
  			 * want to append it to any already-received error message.
  			 */
  			pqSaveErrorResult(conn);
  			conn->asyncStatus = PGASYNC_IDLE;
! 			res = pqPrepareAsyncResult(conn);
! 
! 			if(res && res->resultStatus != PGRES_COPY_IN &&
! 				 res->resultStatus != PGRES_COPY_OUT)
! 			{
! 				notifyResultCreateHooks(conn, res);
! 			}
! 
! 			return res;
  		}
  
  		/* Parse it. */
  		parseInput(conn);
  	}
  
***************
*** 1265,1276 ****
--- 1429,1446 ----
  							  libpq_gettext("unexpected asyncStatus: %d\n"),
  							  (int) conn->asyncStatus);
  			res = PQmakeEmptyPGresult(conn, PGRES_FATAL_ERROR);
  			break;
  	}
  
+ 	if(res && res->resultStatus != PGRES_COPY_IN &&
+ 		 res->resultStatus != PGRES_COPY_OUT)
+ 	{
+ 		notifyResultCreateHooks(conn, res);
+ 	}
+ 
  	return res;
  }
  
  
  /*
   * PQexec