libpq object hooks

[Merlin Moncure-2]

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

[objhooks.patch]

---

Index: exports.txt
===================================================================
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/exports.txt,v
retrieving revision 1.19
diff -C6 -r1.19 exports.txt
*** exports.txt 19 Mar 2008 00:39:33 -0000 1.19
--- exports.txt 30 Apr 2008 20:05:25 -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 30 Apr 2008 20:05:25 -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 30 Apr 2008 20:05:26 -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 30 Apr 2008 20:05:26 -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(pg_strcasecmp(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(pg_strcasecmp(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(pg_strcasecmp(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 30 Apr 2008 20:05:26 -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 30 Apr 2008 20:05:26 -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 *);

---

--
Sent via pgsql-patches mailing list (pgsql-patches@...)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

[Andrew Dunstan]

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

--
Sent via pgsql-patches mailing list (pgsql-patches@...)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

[Bruce Momjian-5]

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@...>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

--
Sent via pgsql-patches mailing list (pgsql-patches@...)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

[Alvaro Herrera-7]

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.

--
Sent via pgsql-patches mailing list (pgsql-patches@...)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

[Andrew Dunstan]

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

--
Sent via pgsql-patches mailing list (pgsql-patches@...)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

[Merlin Moncure-2]

On Wed, May 14, 2008 at 8:18 AM, Andrew Dunstan <andrew@...> 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

--
Sent via pgsql-patches mailing list (pgsql-patches@...)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

[Tom Lane-2]

Andrew Dunstan <andrew@...> 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

--
Sent via pgsql-patches mailing list (pgsql-patches@...)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

[Merlin Moncure-2]

On Tue, May 13, 2008 at 11:52 PM, Bruce Momjian <bruce@...> 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

--
Sent via pgsql-patches mailing list (pgsql-patches@...)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

[Bruce Momjian-5]

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@...>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

--
Sent via pgsql-patches mailing list (pgsql-patches@...)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

[Merlin Moncure-2]

On Wed, May 14, 2008 at 10:44 AM, Tom Lane <tgl@...> 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

--
Sent via pgsql-patches mailing list (pgsql-patches@...)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

[Merlin Moncure-2]

On Wed, May 14, 2008 at 10:52 AM, Bruce Momjian <bruce@...> 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

--
Sent via pgsql-patches mailing list (pgsql-patches@...)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

[slakr]

>> I'm wondering why the hooks need names at all.  AFAICS all that
>> libpq needs to know about a hook is a callback function address
>> and a void * passthrough pointer.
>
>
> In question is:
> + void *
> + PQhookData(const PGconn *conn, const char *hookName)
>
> Basically, libpqtypes has various functions that take a PGconn that
> need the private data that is stored in libpq with the connection.
> PQhookData just does simple linear search and returns the data.
>
> [thinks]
> are you suggesting something like
> + void *
> + PQhookData(const PGconn *conn, const void *hookHandle)
> ?
>
> I would have to take a quick look at the code with Andrew C (he'll be
> in in a bit)...but this might be doable.
>

The hook callback functions allow the hook implementor to receive
created/destroyed events about a PGconn and PGresult (PQreset as well).  The
hook implementor has the option of associating some memory with either.  But,
that memory pointer is worthless unless there is a way of referencing it at a
later time.

HookName would not be needed if the libpq hook API only supported a single
Object Hook to be registered per conn (or library-wide).  It was requested of us
to allow a list of hooks per conn.  This requries a way of referencing the item.

Functions outside the hook callback functions:
- PQparamCreate needs libpq-hook-func void *PQhookData(conn, hookName)
- PQgetf needs libpq-hook-func void *PQresultHookData(res, hookName)
- Also, the "void *resultCreate(...)" hook callback implementation inside
libpqtypes must use PQhookData on the provided conn so it can copy some
conn.hookData properties to the result.hookData.  The created result.hookData is
returned (one can return NULL if no hookData is needed).

I have no issue with merlin's idea of a void *handle, but that doesn't really
change the concept at all ... just allows someone registering hooks with libpq
to use something other than a string.  The hookName string idea feels a little
more natural and simple.

--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/

--
Sent via pgsql-patches mailing list (pgsql-patches@...)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

[Merlin Moncure-2]

On Wed, May 14, 2008 at 10:44 AM, Tom Lane <tgl@...> 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

--
Sent via pgsql-patches mailing list (pgsql-patches@...)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

[slakr]

Merlin Moncure wrote:

> On Wed, May 14, 2008 at 10:44 AM, Tom Lane <tgl@...> wrote:
>> I'm wondering why the hooks need names at all.  AFAICS all that
>> libpq needs to know about a hook is a callback function address
>> and a void * passthrough pointer.
>
> For reference...here is what libpqtypes has to do to bind via the
> hooking interface:
> http://libpqtypes.esilo.com/browse_source.html?file=hooks.c
>
> Are you proposing something substantially different (not my handle
> suggestion)? How would it work exactly?
>
> merlin
>
>

It is important to see how "NON-hook-callback" functions in libpqtypes
make use of the hook data.

PQparamCreate must get a pointer to the conn hook data
http://libpqtypes.esilo.com/browse_source.html?file=param.c#line24

PQgetf must get a pointer to the result hook data
http://libpqtypes.esilo.com/browse_source.html?file=exec.c#line65

These are NOT hook callbacks.  The hook data is NOT isolated to callback
functions.  It is memory that is publically accessible, outside hook
implementations.

--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/

--
Sent via pgsql-patches mailing list (pgsql-patches@...)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

[slakr]

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 *);


--
Sent via pgsql-patches mailing list (pgsql-patches@...)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

[DaveG-4]

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@...      510 536 1443    510 282 0869
If simplicity worked, the world would be overrun with insects.

--
Sent via pgsql-patches mailing list (pgsql-patches@...)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

[Merlin Moncure-2]

On Wed, May 14, 2008 at 3:47 PM, daveg <daveg@...> 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

--
Sent via pgsql-patches mailing list (pgsql-patches@...)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

[Merlin Moncure-2]

On Wed, May 14, 2008 at 10:44 AM, Tom Lane <tgl@...> 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

--
Sent via pgsql-patches mailing list (pgsql-patches@...)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

[Tom Lane-2]

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

--
Sent via pgsql-patches mailing list (pgsql-patches@...)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

[Andrew Dunstan]

Tom Lane wrote:

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

--
Sent via pgsql-patches mailing list (pgsql-patches@...)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches