Le 01/07/2021 à 18:47, Tom Lane a écrit :
> Nicolas CHAHWEKILIAN <leptitstagia...@gmail.com> writes:
>> As far as I am concerned, I am totally awaiting for this kind of feature
>> exposed here, for one single reason at this time : the extension
>> pg_statement_rollback will be much more valuable with the ability of
>> processing "rollback to savepoint" without the need for explicit
>> instruction from client side (and this patch is giving this option).
> What exactly do these hooks do that isn't done as well or better
> by the RegisterXactCallback and RegisterSubXactCallback mechanisms?
> Perhaps we need to define some additional event types for those?
> Or pass more data to the callback functions?
Sorry it take me time to recall the reason of the hooks. Actually the
problem is that the callbacks are not called when a statement is
executed after an error so that we fall back to error:
ERROR: current transaction is aborted, commands ignored until end
of transaction block
For example with the rollback at statement level extension:
BEGIN;
UPDATE tbl_rsl SET id = 'two', val = 2 WHERE id = 1; -- >>>>> will fail
LOG: statement: UPDATE tbl_rsl SET id = 'two', val = 2 WHERE id = 1;
ERROR: invalid input syntax for type integer: "two"
LINE 1: UPDATE tbl_rsl SET id = 'two', val = 2 WHERE id = 1;
^
UPDATE tbl_rsl SET id = 'two', val = 2 WHERE id = 1; -- >>>>> will
fail again
LOG: statement: UPDATE tbl_rsl SET id = 'two', val = 2 WHERE id = 1;
ERROR: current transaction is aborted, commands ignored until end
of transaction block
SELECT * FROM tbl_rsl; -- Should show records id 1 + 2
LOG: statement: SELECT * FROM tbl_rsl;
ERROR: current transaction is aborted, commands ignored until end
of transaction block
With the exention and the hook on start_xact_command() we can continue
and execute all the following statements.
I have updated the patch to only keep the hook on start_xact_command(),
as you've suggested the other hooks can be replaced by the use of the
xact callback. The extension has also been updated for testing the
feature, available here https://github.com/darold/pg_statement_rollbackv2
> I quite dislike inventing a hook that's defined as "run during
> start_xact_command", because there is basically nothing that's
> not ad-hoc about that function: it's internal to postgres.c
> and both its responsibilities and its call sites have changed
> over time. I think anyone hooking into that will be displeased
> by the stability of their results.
Unfortunately I had not found a better solution, but I just tried with
placing the hook in function BeginCommand() in src/backend/tcop/dest.c
and the extension is working as espected. Do you think it would be a
better place?In this case I can update the patch. For this feature we
need a hook that is executed before any command even if the transaction
is in abort state to be able to inject the rollback to savepoint, maybe
I'm not looking at the right place to do that.
Thanks
--
Gilles Darold
http://www.darold.net/
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 8cea10c901..4b9f8eeb3c 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -207,6 +207,8 @@ static void log_disconnections(int code, Datum arg);
static void enable_statement_timeout(void);
static void disable_statement_timeout(void);
+/* Hooks for plugins to get control at end of start_xact_command() */
+XactCommandStart_hook_type start_xact_command_hook = NULL;
/* ----------------------------------------------------------------
* routines to obtain user input
@@ -2708,6 +2710,13 @@ start_xact_command(void)
!get_timeout_active(CLIENT_CONNECTION_CHECK_TIMEOUT))
enable_timeout_after(CLIENT_CONNECTION_CHECK_TIMEOUT,
client_connection_check_interval);
+
+ /*
+ * Now give loadable modules a chance to execute code before a transaction
+ * command is processed.
+ */
+ if (start_xact_command_hook)
+ (*start_xact_command_hook) ();
}
static void
diff --git a/src/include/tcop/pquery.h b/src/include/tcop/pquery.h
index 2318f04ff0..540ede42fd 100644
--- a/src/include/tcop/pquery.h
+++ b/src/include/tcop/pquery.h
@@ -48,4 +48,8 @@ extern bool PlannedStmtRequiresSnapshot(struct PlannedStmt *pstmt);
extern void EnsurePortalSnapshotExists(void);
+/* Hook for plugins to get control in start_xact_command() and finish_xact_command() */
+typedef void (*XactCommandStart_hook_type) (void);
+extern PGDLLIMPORT XactCommandStart_hook_type start_xact_command_hook;
+
#endif /* PQUERY_H */
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 64c06cf952..f473c2dc39 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -2934,6 +2934,7 @@ XPVIV
XPVMG
XactCallback
XactCallbackItem
+XactCommandStart_hook_type
XactEvent
XactLockTableWaitInfo
XidBoundsViolation
