Re: Problem with dblink - Mailing list pgsql-patches

>>Tom Lane wrote:
>>>(More generally, I wonder if AtEOXact_SPI oughtn't be fixed to emit
>>>a WARNING if the SPI stack isn't empty, except in the error case.
>>>Neglecting SPI_finish is analogous to a resource leak, and we have
>>>code in place to warn about other sorts of leaks.)

Is the attached what you had in mind? The original problem function
would now look like:

SELECT dblink_build_sql_insert('foo','1 2',2,'{"0", "a"}','{"99", "xyz"}');
WARNING:  freeing non-empty SPI stack
HINT:  Check for missing "SPI_finish" calls
                   dblink_build_sql_insert
-----------------------------------------------------------
  INSERT INTO foo(f1,f2,f3) VALUES('99','xyz','{a0,b0,c0}')
(1 row)

Joe

Index: src/backend/access/transam/xact.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/access/transam/xact.c,v
retrieving revision 1.157
diff -c -r1.157 xact.c
*** src/backend/access/transam/xact.c    29 Nov 2003 19:51:40 -0000    1.157
--- src/backend/access/transam/xact.c    29 Nov 2003 23:57:09 -0000
***************
*** 977,983 ****

      CallEOXactCallbacks(true);
      AtEOXact_GUC(true);
!     AtEOXact_SPI();
      AtEOXact_gist();
      AtEOXact_hash();
      AtEOXact_nbtree();
--- 977,983 ----

      CallEOXactCallbacks(true);
      AtEOXact_GUC(true);
!     AtEOXact_SPI(false);
      AtEOXact_gist();
      AtEOXact_hash();
      AtEOXact_nbtree();
***************
*** 1087,1093 ****

      CallEOXactCallbacks(false);
      AtEOXact_GUC(false);
!     AtEOXact_SPI();
      AtEOXact_gist();
      AtEOXact_hash();
      AtEOXact_nbtree();
--- 1087,1093 ----

      CallEOXactCallbacks(false);
      AtEOXact_GUC(false);
!     AtEOXact_SPI(true);
      AtEOXact_gist();
      AtEOXact_hash();
      AtEOXact_nbtree();
Index: src/backend/executor/spi.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/executor/spi.c,v
retrieving revision 1.108
diff -c -r1.108 spi.c
*** src/backend/executor/spi.c    29 Nov 2003 19:51:48 -0000    1.108
--- src/backend/executor/spi.c    29 Nov 2003 23:57:09 -0000
***************
*** 177,191 ****
   * Clean up SPI state at transaction commit or abort (we don't care which).
   */
  void
! AtEOXact_SPI(void)
  {
      /*
       * Note that memory contexts belonging to SPI stack entries will be
       * freed automatically, so we can ignore them here.  We just need to
       * restore our static variables to initial state.
       */
!     if (_SPI_stack != NULL)        /* there was abort */
          free(_SPI_stack);
      _SPI_current = _SPI_stack = NULL;
      _SPI_connected = _SPI_curid = -1;
      SPI_processed = 0;
--- 177,199 ----
   * Clean up SPI state at transaction commit or abort (we don't care which).
   */
  void
! AtEOXact_SPI(bool isAbort)
  {
      /*
       * Note that memory contexts belonging to SPI stack entries will be
       * freed automatically, so we can ignore them here.  We just need to
       * restore our static variables to initial state.
       */
!     if (_SPI_stack != NULL)
!     {
          free(_SPI_stack);
+         if (!isAbort)
+             ereport(WARNING,
+                     (errcode(ERRCODE_WARNING),
+                      errmsg("freeing non-empty SPI stack"),
+                      errhint("Check for missing \"SPI_finish\" calls")));
+     }
+
      _SPI_current = _SPI_stack = NULL;
      _SPI_connected = _SPI_curid = -1;
      SPI_processed = 0;
Index: src/include/executor/spi.h
===================================================================
RCS file: /cvsroot/pgsql-server/src/include/executor/spi.h,v
retrieving revision 1.40
diff -c -r1.40 spi.h
*** src/include/executor/spi.h    29 Nov 2003 22:41:01 -0000    1.40
--- src/include/executor/spi.h    29 Nov 2003 23:57:09 -0000
***************
*** 116,121 ****
  extern void SPI_cursor_move(Portal portal, bool forward, int count);
  extern void SPI_cursor_close(Portal portal);

! extern void AtEOXact_SPI(void);

  #endif   /* SPI_H */
--- 116,121 ----
  extern void SPI_cursor_move(Portal portal, bool forward, int count);
  extern void SPI_cursor_close(Portal portal);

! extern void AtEOXact_SPI(bool isAbort);

  #endif   /* SPI_H */