Fix an oversight in my patch of a couple weeks ago that ensured a snapshot

is available during datatype input in Bind message processing.  I put the
PopActiveSnapshot() or equivalent just before PortalDefineQuery, which is
an unsafe spot for it (in 8.3 and later) because we are carrying a plancache
refcount that hasn't yet been assigned to the portal.  Any error thrown there
would result in leaking the refcount.  It's not exactly likely that
PopActiveSnapshot would throw an elog, perhaps, but it could happen.
Reorder the code and add another comment warning not to do that.

diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 85a266172f867ce1253f89e28bdde0001d5e9d95..5bfeae97d67bc623a5c4eb54fc2c2249e5bd66ef 100644 (file)
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.542.2.3 2008/12/13 02:00:29 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.542.2.4 2009/01/01 17:12:24 tgl Exp $
  *
  * NOTES
  *       this is the "main" module of the postgres backend and
@@ -1728,13 +1728,11 @@ exec_bind_message(StringInfo input_message)
                cplan = NULL;
        }
 
-       /* Done with the snapshot used for parameter I/O and parsing/planning */
-       ActiveSnapshot = NULL;
-       if (mySnapshot)
-               FreeSnapshot(mySnapshot);
-
        /*
-        * Define portal and start execution.
+        * Now we can define the portal.
+        *
+        * DO NOT put any code that could possibly throw an error between the
+        * above "RevalidateCachedPlan(psrc, false)" call and here.
         */
        PortalDefineQuery(portal,
                                          saved_stmt_name,
@@ -1743,6 +1741,14 @@ exec_bind_message(StringInfo input_message)
                                          plan_list,
                                          cplan);
 
+       /* Done with the snapshot used for parameter I/O and parsing/planning */
+       ActiveSnapshot = NULL;
+       if (mySnapshot)
+               FreeSnapshot(mySnapshot);
+
+       /*
+        * And we're ready to start portal execution.
+        */
        PortalStart(portal, params, InvalidSnapshot);
 
        /*