Invent pre-commit/pre-prepare/pre-subcommit events for xact callbacks.

Currently it's only possible for loadable modules to get control during
post-commit cleanup of a transaction.  That doesn't work too well if they
want to do something that could throw an error; for example, an FDW might
need to issue a remote commit, which could well fail.  To improve matters,
extend the existing APIs for XactCallback and SubXactCallback functions
to provide new pre-commit events for this purpose.

The release notes will need to mention that existing callback functions
should be checked to make sure they don't do something unwanted when one
of the new event types occurs.  In the examples within our source tree,
contrib/sepgsql was fine but plpgsql had been a bit too cute.

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 81d268749564687ee2c525698e8ab32c5e446079..9a9724574bec2874710316fa74643ae24d2d305e 100644 (file)
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -1825,6 +1825,8 @@ CommitTransaction(void)
            break;
    }
 
+   CallXactCallbacks(XACT_EVENT_PRE_COMMIT);
+
    /*
     * The remaining actions cannot call any user-defined code, so it's safe
     * to start shutting down within-transaction services.  But note that most
@@ -2028,6 +2030,8 @@ PrepareTransaction(void)
            break;
    }
 
+   CallXactCallbacks(XACT_EVENT_PRE_PREPARE);
+
    /*
     * The remaining actions cannot call any user-defined code, so it's safe
     * to start shutting down within-transaction services.  But note that most
@@ -4058,8 +4062,12 @@ CommitSubTransaction(void)
        elog(WARNING, "CommitSubTransaction while in %s state",
             TransStateAsString(s->state));
 
-   /* Pre-commit processing goes here -- nothing to do at the moment */
+   /* Pre-commit processing goes here */
+
+   CallSubXactCallbacks(SUBXACT_EVENT_PRE_COMMIT_SUB, s->subTransactionId,
+                        s->parent->subTransactionId);
 
+   /* Do the actual "commit", such as it is */
    s->state = TRANS_COMMIT;
 
    /* Must CCI to ensure commands of subtransaction are seen as done */

diff --git a/src/include/access/xact.h b/src/include/access/xact.h
index fcdff932721d060ce957cca798886c22001f3ac2..8cadc549716479a807f713e7f76291d5c7ab7789 100644 (file)
--- a/src/include/access/xact.h
+++ b/src/include/access/xact.h
@@ -76,7 +76,9 @@ typedef enum
 {
    XACT_EVENT_COMMIT,
    XACT_EVENT_ABORT,
-   XACT_EVENT_PREPARE
+   XACT_EVENT_PREPARE,
+   XACT_EVENT_PRE_COMMIT,
+   XACT_EVENT_PRE_PREPARE
 } XactEvent;
 
 typedef void (*XactCallback) (XactEvent event, void *arg);
@@ -85,7 +87,8 @@ typedef enum
 {
    SUBXACT_EVENT_START_SUB,
    SUBXACT_EVENT_COMMIT_SUB,
-   SUBXACT_EVENT_ABORT_SUB
+   SUBXACT_EVENT_ABORT_SUB,
+   SUBXACT_EVENT_PRE_COMMIT_SUB
 } SubXactEvent;
 
 typedef void (*SubXactCallback) (SubXactEvent event, SubTransactionId mySubid,

diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 591a432f542c5347d24628b9b0718e0c0f0abfcb..70e67d9eb70ef0dae58504f64d2ed52feae3c77f 100644 (file)
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -6150,7 +6150,7 @@ plpgsql_xact_cb(XactEvent event, void *arg)
     * expect the regular abort recovery procedures to release everything of
     * interest.
     */
-   if (event != XACT_EVENT_ABORT)
+   if (event == XACT_EVENT_COMMIT || event == XACT_EVENT_PREPARE)
    {
        /* Shouldn't be any econtext stack entries left at commit */
        Assert(simple_econtext_stack == NULL);
@@ -6159,7 +6159,7 @@ plpgsql_xact_cb(XactEvent event, void *arg)
            FreeExecutorState(simple_eval_estate);
        simple_eval_estate = NULL;
    }
-   else
+   else if (event == XACT_EVENT_ABORT)
    {
        simple_econtext_stack = NULL;
        simple_eval_estate = NULL;
@@ -6178,19 +6178,19 @@ void
 plpgsql_subxact_cb(SubXactEvent event, SubTransactionId mySubid,
                   SubTransactionId parentSubid, void *arg)
 {
-   if (event == SUBXACT_EVENT_START_SUB)
-       return;
-
-   while (simple_econtext_stack != NULL &&
-          simple_econtext_stack->xact_subxid == mySubid)
+   if (event == SUBXACT_EVENT_COMMIT_SUB || event == SUBXACT_EVENT_ABORT_SUB)
    {
-       SimpleEcontextStackEntry *next;
+       while (simple_econtext_stack != NULL &&
+              simple_econtext_stack->xact_subxid == mySubid)
+       {
+           SimpleEcontextStackEntry *next;
 
-       FreeExprContext(simple_econtext_stack->stack_econtext,
-                       (event == SUBXACT_EVENT_COMMIT_SUB));
-       next = simple_econtext_stack->next;
-       pfree(simple_econtext_stack);
-       simple_econtext_stack = next;
+           FreeExprContext(simple_econtext_stack->stack_econtext,
+                           (event == SUBXACT_EVENT_COMMIT_SUB));
+           next = simple_econtext_stack->next;
+           pfree(simple_econtext_stack);
+           simple_econtext_stack = next;
+       }
    }
 }