Do not execute fastpath function calls if in transaction ABORT state.

Just like queries, doing nothing is better than possibly getting weird
error messages.  Also, improve comments.

diff --git a/src/backend/tcop/fastpath.c b/src/backend/tcop/fastpath.c
index fd30bd58ac555ce04a75719a9be659041b1df76a..05640b5be8f003793c8b554add064f7d348934d9 100644 (file)
--- a/src/backend/tcop/fastpath.c
+++ b/src/backend/tcop/fastpath.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/tcop/fastpath.c,v 1.43 2000/07/03 23:09:46 wieck Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/tcop/fastpath.c,v 1.44 2000/10/24 20:59:35 tgl Exp $
  *
  * NOTES
  *   This cruft is the server side of PQfn.
@@ -271,9 +271,14 @@ update_fp_info(Oid func_id, struct fp_info * fip)
  * Note: All ordinary errors result in elog(ERROR,...).  However,
  * if we lose the frontend connection there is no one to elog to,
  * and no use in proceeding...
+ *
+ * Note: palloc()s done here and in the called function do not need to be
+ * cleaned up explicitly.  We are called from PostgresMain() in the
+ * QueryContext memory context, which will be automatically reset when
+ * control returns to PostgresMain.
  */
 int
-HandleFunctionRequest()
+HandleFunctionRequest(void)
 {
    Oid         fid;
    int         argsize;
@@ -285,6 +290,32 @@ HandleFunctionRequest()
    char       *p;
    struct fp_info *fip;
 
+   /*
+    * XXX FIXME: This protocol is misdesigned.
+    *
+    * We really do not want to elog() before having swallowed all of the
+    * frontend's fastpath message; otherwise we will lose sync with the input
+    * datastream.  What should happen is we absorb all of the input message
+    * per protocol syntax, and *then* do error checking (including lookup of
+    * the given function ID) and elog if appropriate.  Unfortunately, because
+    * we cannot even read the message properly without knowing whether the
+    * data types are pass-by-ref or pass-by-value, it's not all that easy to
+    * do :-(.  The protocol should require the client to supply what it
+    * thinks is the typbyval and typlen value for each arg, so that we can
+    * read the data without having to do any lookups.  Then after we've read
+    * the message, we should do the lookups, verify agreement of the actual
+    * function arg types with what we received, and finally call the function.
+    *
+    * As things stand, not only will we lose sync for an invalid message
+    * (such as requested function OID doesn't exist), but we may lose sync
+    * for a perfectly valid message if we are in transaction-aborted state!
+    * This can happen because our database lookup attempts may fail entirely
+    * in abort state.
+    *
+    * Unfortunately I see no way to fix this without breaking a lot of
+    * existing clients.  Maybe do it as part of next protocol version change.
+    */
+
    if (pq_getint(&tmp, 4))     /* function oid */
        return EOF;
    fid = (Oid) tmp;
@@ -293,23 +324,13 @@ HandleFunctionRequest()
 
    /*
     * This is where the one-back caching is done. If you want to save
-    * more state, make this a loop around an array.
+    * more state, make this a loop around an array.  Given the relatively
+    * short lifespan of the cache, not clear that there's any win possible.
     */
    fip = &last_fp;
    if (!valid_fp_info(fid, fip))
        update_fp_info(fid, fip);
 
-   /*
-    * XXX FIXME: elog() here means we lose sync with the frontend, since
-    * we have not swallowed all of its input message.  What should happen
-    * is we absorb all of the input message per protocol syntax, and
-    * *then* do error checking (including lookup of the given function ID)
-    * and elog if appropriate.  Unfortunately, because we cannot even read
-    * the message properly without knowing whether the data types are
-    * pass-by-ref or pass-by-value, it's not all that easy to fix :-(.
-    * This protocol is misdesigned.
-    */
-
    if (fip->flinfo.fn_nargs != nargs || nargs > FUNC_MAX_ARGS)
    {
        elog(ERROR, "HandleFunctionRequest: actual arguments (%d) != registered arguments (%d)",
@@ -366,6 +387,17 @@ HandleFunctionRequest()
        }
    }
 
+   /*
+    * Now that we've eaten the input message, check to see if we actually
+    * want to do the function call or not.
+    *
+    * Currently, we report an error if in ABORT state, or return a dummy
+    * NULL response if fastpath support has been compiled out.
+    */
+   if (IsAbortedTransactionBlockState())
+       elog(ERROR, "current transaction is aborted, "
+            "queries ignored until end of transaction block");
+
 #ifdef NO_FASTPATH
    /* force a NULL return */
    retval = (Datum) 0;