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