Re: statement_timeout vs DECLARE CURSOR

[ redirect to -hackers ]

I wrote:
>> Christophe Pettus <xof@thebuild.com> writes:
>>> A bit more poking revealed the reason: The ON HOLD cursor's query is executed at commit time (which is, logically,
notinterruptible), but that's all wrapped in the single statement outside of a transaction. 

>> Hmm ... seems like a bit of a UX failure.  I wonder why we don't persist
>> such cursors before we get into the uninterruptible part of COMMIT.

> Oh, I see the issue.  It's not that that part of COMMIT isn't
> interruptible; you can control-C out of it just fine.  The problem
> is that finish_xact_command() disarms the statement timeout before
> starting CommitTransactionCommand at all.

> We could imagine pushing the responsibility for that down into
> xact.c, allowing it to happen after CommitTransaction has finished
> running user-defined code.  But it seems like a bit of a mess
> because there are so many other code paths there.  Not sure how
> to avoid future bugs-of-omission.

Actually ... maybe it needn't be any harder than the attached?

This makes it possible for a statement timeout interrupt to occur
anytime during CommitTransactionCommand, but I think
CommitTransactionCommand had better be able to protect itself
against that anyway, for a couple of reasons:

1. It's not significantly different from a query-cancel interrupt,
which surely could arrive during that window.

2. COMMIT-within-procedures already exposes us to statement timeout
during COMMIT.

            regards, tom lane

diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 0775abe35d..9ec96f2453 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -2713,9 +2713,6 @@ start_xact_command(void)
 static void
 finish_xact_command(void)
 {
-    /* cancel active statement timeout after each command */
-    disable_statement_timeout();
-
     if (xact_started)
     {
         CommitTransactionCommand();
@@ -2733,6 +2730,9 @@ finish_xact_command(void)
 
         xact_started = false;
     }
+
+    /* cancel active statement timeout after each command */
+    disable_statement_timeout();
 }