Re: [PATCH] Hooks at XactCommand level

Le 01/07/2021 à 18:47, Tom Lane a écrit :
> Nicolas CHAHWEKILIAN <leptitstagiaire(at)gmail(dot)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/