Re: [PATCHES] libpq events patch (with sgml docs) - Mailing list pgsql-hackers

Andrew Chernow wrote:
> New patch following our discussion with updated docs.
>
>>> few logical errors).  I don't think it makes sense to do it
>>> otherwise, it would be like calling free after a malloc failure.
>>
>> I can live with that definition, but please document it.
>>
>>
>
> To build on this analogy, PGEVT_CONNRESET is like a realloc.  Meaning,
> the initial malloc "PGEVT_REGISTER" worked by the realloc
> "PGEVT_CONNRESET" didn't ... you still have free "PGEVT_CONNDESTROY" the
> initial.  Its documented that way.  Basically if a register succeeds, a
> destroy will always be sent regardless of what happens with a reset.
>
>

I attached the wrong patch.  I'm sorry.

--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
Index: doc/src/sgml/libpq.sgml
===================================================================
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/libpq.sgml,v
retrieving revision 1.261
diff -C6 -r1.261 libpq.sgml
*** doc/src/sgml/libpq.sgml    17 Sep 2008 04:31:08 -0000    1.261
--- doc/src/sgml/libpq.sgml    17 Sep 2008 14:19:29 -0000
***************
*** 4911,4923 ****
         When a <literal>PGEVT_REGISTER</literal> event is received, the
         <parameter>evtInfo</parameter> pointer should be cast to a
         <structname>PGEventRegister *</structname>.  This structure contains a
         <structname>PGconn</structname> that should be in the
         <literal>CONNECTION_OK</literal> status; guaranteed if one calls
         <function>PQregisterEventProc</function> right after obtaining a good
!        <structname>PGconn</structname>.
        </para>
       </listitem>
      </varlistentry>

      <varlistentry>
       <term><literal>PGEVT_CONNRESET</literal></term>
--- 4911,4925 ----
         When a <literal>PGEVT_REGISTER</literal> event is received, the
         <parameter>evtInfo</parameter> pointer should be cast to a
         <structname>PGEventRegister *</structname>.  This structure contains a
         <structname>PGconn</structname> that should be in the
         <literal>CONNECTION_OK</literal> status; guaranteed if one calls
         <function>PQregisterEventProc</function> right after obtaining a good
!        <structname>PGconn</structname>.  When returning a failure code, all
!        cleanup must be performed as no <literal>PGEVT_CONNDESTROY</literal>
!        event will be sent.
        </para>
       </listitem>
      </varlistentry>

      <varlistentry>
       <term><literal>PGEVT_CONNRESET</literal></term>
***************
*** 4941,4953 ****

         When a <literal>PGEVT_CONNRESET</literal> event is received, the
         <parameter>evtInfo</parameter> pointer should be cast to a
         <structname>PGEventConnReset *</structname>.  Although the contained
         <structname>PGconn</structname> was just reset, all event data remains
         unchanged.  This event should be used to reset/reload/requery any
!        associated <literal>instanceData</literal>.
        </para>
       </listitem>
      </varlistentry>

      <varlistentry>
       <term><literal>PGEVT_CONNDESTROY</literal></term>
--- 4943,4956 ----

         When a <literal>PGEVT_CONNRESET</literal> event is received, the
         <parameter>evtInfo</parameter> pointer should be cast to a
         <structname>PGEventConnReset *</structname>.  Although the contained
         <structname>PGconn</structname> was just reset, all event data remains
         unchanged.  This event should be used to reset/reload/requery any
!        associated <literal>instanceData</literal>.  A PGEVT_CONNDESTROY event
!        is always sent, regardless of the event procedure's return value.
        </para>
       </listitem>
      </varlistentry>

      <varlistentry>
       <term><literal>PGEVT_CONNDESTROY</literal></term>
***************
*** 5000,5023 ****
         <structname>PGEventResultCreate *</structname>.  The
         <parameter>conn</parameter> is the connection used to generate the
         result.  This is the ideal place to initialize any
         <literal>instanceData</literal> that needs to be associated with the
         result.  If the event procedure fails, the result will be cleared and
         the failure will be propagated.  The event procedure must not try to
!        <function>PQclear</> the result object for itself.
        </para>
       </listitem>
      </varlistentry>

      <varlistentry>
       <term><literal>PGEVT_RESULTCOPY</literal></term>
       <listitem>
        <para>
         The result copy event is fired in response to
         <function>PQcopyResult</function>.  This event will only be fired after
!        the copy is complete.

        <synopsis>
  typedef struct
  {
      const PGresult *src;
      PGresult *dest;
--- 5003,5030 ----
         <structname>PGEventResultCreate *</structname>.  The
         <parameter>conn</parameter> is the connection used to generate the
         result.  This is the ideal place to initialize any
         <literal>instanceData</literal> that needs to be associated with the
         result.  If the event procedure fails, the result will be cleared and
         the failure will be propagated.  The event procedure must not try to
!        <function>PQclear</> the result object for itself.  When returning a
!        failure code, all cleanup must be performed as no
!        <literal>PGEVT_RESULTDESTROY</literal> event will be sent.
        </para>
       </listitem>
      </varlistentry>

      <varlistentry>
       <term><literal>PGEVT_RESULTCOPY</literal></term>
       <listitem>
        <para>
         The result copy event is fired in response to
         <function>PQcopyResult</function>.  This event will only be fired after
!        the copy is complete.  Only source result event procedures that have
!        successfully handled the <literal>PGEVT_RESULTCREATE</literal> event,
!        will be copied to the destination result.

        <synopsis>
  typedef struct
  {
      const PGresult *src;
      PGresult *dest;
***************
*** 5029,5041 ****
         <structname>PGEventResultCopy *</structname>.  The
         <parameter>src</parameter> result is what was copied while the
         <parameter>dest</parameter> result is the copy destination.  This event
         can be used to provide a deep copy of <literal>instanceData</literal>,
         since <literal>PQcopyResult</literal> cannot do that.  If the event
         procedure fails, the entire copy operation will fail and the
!        <parameter>dest</parameter> result will be cleared.
        </para>
       </listitem>
      </varlistentry>

      <varlistentry>
       <term><literal>PGEVT_RESULTDESTROY</literal></term>
--- 5036,5050 ----
         <structname>PGEventResultCopy *</structname>.  The
         <parameter>src</parameter> result is what was copied while the
         <parameter>dest</parameter> result is the copy destination.  This event
         can be used to provide a deep copy of <literal>instanceData</literal>,
         since <literal>PQcopyResult</literal> cannot do that.  If the event
         procedure fails, the entire copy operation will fail and the
!        <parameter>dest</parameter> result will be cleared.   When returning a
!        failure code, all cleanup must be performed as no
!        <literal>PGEVT_RESULTDESTROY</literal> event will be sent.
        </para>
       </listitem>
      </varlistentry>

      <varlistentry>
       <term><literal>PGEVT_RESULTDESTROY</literal></term>
Index: src/interfaces/libpq/fe-exec.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-exec.c,v
retrieving revision 1.198
diff -C6 -r1.198 fe-exec.c
*** src/interfaces/libpq/fe-exec.c    17 Sep 2008 04:31:08 -0000    1.198
--- src/interfaces/libpq/fe-exec.c    17 Sep 2008 14:19:29 -0000
***************
*** 346,366 ****
          dest->nEvents = src->nEvents;
      }

      /* Okay, trigger PGEVT_RESULTCOPY event */
      for (i = 0; i < dest->nEvents; i++)
      {
!         PGEventResultCopy evt;
!
!         evt.src = src;
!         evt.dest = dest;
!         if (!dest->events[i].proc(PGEVT_RESULTCOPY, &evt,
!                                   dest->events[i].passThrough))
          {
!             PQclear(dest);
!             return NULL;
          }
      }

      return dest;
  }

--- 346,371 ----
          dest->nEvents = src->nEvents;
      }

      /* Okay, trigger PGEVT_RESULTCOPY event */
      for (i = 0; i < dest->nEvents; i++)
      {
!         if(src->events[i].resultInitialized)
          {
!             PGEventResultCopy evt;
!
!             evt.src = src;
!             evt.dest = dest;
!             if (!dest->events[i].proc(PGEVT_RESULTCOPY, &evt,
!                                       dest->events[i].passThrough))
!             {
!                 PQclear(dest);
!                 return NULL;
!             }
!
!             dest->events[i].resultInitialized = TRUE;
          }
      }

      return dest;
  }

***************
*** 378,396 ****
          return NULL;

      newEvents = (PGEvent *) malloc(count * sizeof(PGEvent));
      if (!newEvents)
          return NULL;

-     memcpy(newEvents, events, count * sizeof(PGEvent));
-
-     /* NULL out the data pointers and deep copy names */
      for (i = 0; i < count; i++)
      {
          newEvents[i].data = NULL;
!         newEvents[i].name = strdup(newEvents[i].name);
          if (!newEvents[i].name)
          {
              while (--i >= 0)
                  free(newEvents[i].name);
              free(newEvents);
              return NULL;
--- 383,401 ----
          return NULL;

      newEvents = (PGEvent *) malloc(count * sizeof(PGEvent));
      if (!newEvents)
          return NULL;

      for (i = 0; i < count; i++)
      {
+         newEvents[i].proc = events[i].proc;
+         newEvents[i].passThrough = events[i].passThrough;
+         newEvents[i].resultInitialized = FALSE;
          newEvents[i].data = NULL;
!         newEvents[i].name = strdup(events[i].name);
          if (!newEvents[i].name)
          {
              while (--i >= 0)
                  free(newEvents[i].name);
              free(newEvents);
              return NULL;
***************
*** 663,679 ****

      if (!res)
          return;

      for (i = 0; i < res->nEvents; i++)
      {
!         PGEventResultDestroy evt;

-         evt.result = res;
-         (void) res->events[i].proc(PGEVT_RESULTDESTROY, &evt,
-                                    res->events[i].passThrough);
          free(res->events[i].name);
      }

      if (res->events)
          free(res->events);

--- 668,688 ----

      if (!res)
          return;

      for (i = 0; i < res->nEvents; i++)
      {
!         if(res->events[i].resultInitialized)
!         {
!             PGEventResultDestroy evt;
!
!             evt.result = res;
!             (void) res->events[i].proc(PGEVT_RESULTDESTROY, &evt,
!                                        res->events[i].passThrough);
!         }

          free(res->events[i].name);
      }

      if (res->events)
          free(res->events);

***************
*** 1609,1620 ****
--- 1618,1631 ----
                                    libpq_gettext("PGEventProc \"%s\" failed during PGEVT_RESULTCREATE event\n"),
                                    res->events[i].name);
                  pqSetResultError(res, conn->errorMessage.data);
                  res->resultStatus = PGRES_FATAL_ERROR;
                  break;
              }
+
+             res->events[i].resultInitialized = TRUE;
          }
      }

      return res;
  }

Index: src/interfaces/libpq/libpq-events.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/libpq-events.c,v
retrieving revision 1.1
diff -C6 -r1.1 libpq-events.c
*** src/interfaces/libpq/libpq-events.c    17 Sep 2008 04:31:08 -0000    1.1
--- src/interfaces/libpq/libpq-events.c    17 Sep 2008 14:19:29 -0000
***************
*** 73,84 ****
--- 73,85 ----
      conn->events[conn->nEvents].proc = proc;
      conn->events[conn->nEvents].name = strdup(name);
      if (!conn->events[conn->nEvents].name)
          return FALSE;
      conn->events[conn->nEvents].passThrough = passThrough;
      conn->events[conn->nEvents].data = NULL;
+     conn->events[conn->nEvents].resultInitialized = FALSE;
      conn->nEvents++;

      regevt.conn = conn;
      if (!proc(PGEVT_REGISTER, ®evt, passThrough))
      {
          conn->nEvents--;
Index: src/interfaces/libpq/libpq-int.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/libpq-int.h,v
retrieving revision 1.132
diff -C6 -r1.132 libpq-int.h
*** src/interfaces/libpq/libpq-int.h    17 Sep 2008 04:31:08 -0000    1.132
--- src/interfaces/libpq/libpq-int.h    17 Sep 2008 14:19:29 -0000
***************
*** 153,164 ****
--- 153,165 ----
  typedef struct PGEvent
  {
      PGEventProc    proc;            /* the function to call on events */
      char       *name;            /* used only for error messages */
      void       *passThrough;    /* pointer supplied at registration time */
      void       *data;            /* optional state (instance) data */
+     int           resultInitialized; /* indicates a PGEVT_RESULTCREATE succeeded. */
  } PGEvent;

  struct pg_result
  {
      int            ntups;
      int            numAttributes;