> Now, it's questionable how tense we need to be about that as long as
> event proc failure aborts calling of later event procs.  That means
> that procs have to be robust against getting DESTROY with no CREATE
> calls in any case.  Should we try to make that less uncertain?
>
>

I attached a patch that adds a 'needDestroy' member to PGEvent It is set when resultcreate or resultcopy succeeds and checked during a PQclear. That *should* resolve the issue of "no resultcreate but gets a resultdestroy".

The general question of symmetry between RESULTCREATE and RESULTDESTROY
callbacks is still bothering me.  As committed, PQmakeEmptyPGresult
will copy events into its result, but not fire RESULTCREATE for them
... but they'll get RESULTDESTROY when it's deleted.  Is that what we
want?

PQmakeEmptyPGResult was given thought. The problem is every internal function that generates a result calls PQmakeEmptyPGResult, but those cases should not fire a resultcreate. resultcreate should be called when the result is fully constructed (tuples and all) so the eventproc gets a useful PGresult.

One solution is to do something like the below:

PGresult *
PQmakeEmptyPGresult(PGconn *conn, ExecStatusType status)
{
  return pqMakeEmptyPGresult(conn, status, TRUE);
}

PGresult *
pqMakeEmptyPGresult(PGconn *conn, ExecStatusType status, int fireEvents)
{
  // existing function, only change is handling fireEvents
}

I am willing to create a patch for this.  Is this an acceptable idea?

> I don't have a lot of faith that PQgetResult is the only place
> inside libpq that needs to fire RESULTCREATE, either.
>

I looked again and I didn't see anything. Is there something your thinking of? ISTM that PQgetResult is called every where a result is created (outside of PQmakeEmptyPGresult).

--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
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 10:40:41 -0000
***************
*** 356,367 ****
--- 356,369 ----
  		if (!dest->events[i].proc(PGEVT_RESULTCOPY, &evt,
  								  dest->events[i].passThrough))
  		{
  			PQclear(dest);
  			return NULL;
  		}
+ 
+ 		dest->events[i].needDestroy = TRUE;
  	}
  
  	return dest;
  }
  
  /*
***************
*** 378,394 ****
  		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);
--- 380,396 ----
  		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].needDestroy = FALSE;
  		newEvents[i].data = NULL;
  		newEvents[i].name = strdup(newEvents[i].name);
  		if (!newEvents[i].name)
  		{
  			while (--i >= 0)
  				free(newEvents[i].name);
***************
*** 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);
  
--- 665,685 ----
  
  	if (!res)
  		return;
  
  	for (i = 0; i < res->nEvents; i++)
  	{
! 		if(res->events[i].needDestroy)
! 		{
! 			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 ****
--- 1615,1628 ----
  								  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].needDestroy = TRUE;
  		}
  	}
  
  	return res;
  }
  
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 10:40:41 -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		   needDestroy; 	/* indicates a PGEVT_RESULTDESTROY is needed. */
  } PGEvent;
  
  struct pg_result
  {
  	int			ntups;
  	int			numAttributes;
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to