Re: [PATCH] Hooks at XactCommand level

Hi,

On Tue, Dec 08, 2020 at 11:15:12AM +0100, Gilles Darold wrote:
>
> Based on a PoC reported in a previous thread [1] I'd like to propose new
> hooks around transaction commands. The objective of this patch is to
> allow PostgreSQL extension to act at start and end (including abort) of
> a SQL statement in a transaction.
>
> The idea for these hooks is born from the no go given to Takayuki
> Tsunakawa's patch[2] proposing an in core implementation of
> statement-level rollback transaction and the pg_statement_rollback
> extension[3] that we have developed at LzLabs. The extension
> pg_statement_rollback has two limitation, the first one is that the
> client still have to call the ROLLBACK TO SAVEPOINT when an error is
> encountered and the second is that it generates a crash when PostgreSQL
> is compiled with assert that can not be fixed at the extension level.

This topic came up quite often on the mailing list, the last being from Álvaro
at [1]. I think there's a general agreement that customers want that feature,
won't stop asking for it, and many if not all forks ended up implementing it.

I would still prefer if he had a way to support if in vanilla postgres, with of
course all possible safeguards to avoid an epic fiasco.

I personally think that Álvaro's previous approach, giving the ability to
specify the rollback behavior in the TransactionStmt grammar, would be enough
(I mean without the GUC part) to cover realistic and sensible usecases, which
is where the client fully decides whether there's a statement level rollback or
not. One could probably build a custom module on top of that to introduce some
kind of GUC to change the behavior more globally if it wants to take that risk.

If such an approach is still not wanted for core inclusion, then I'm in favor
of adding those hooks. There's already a published extension that tries to
implement that (for complete fairness I'm one of the people to blame), but as
Gilles previously mentioned this is very hackish and the currently available
hooks makes it very hard if not impossible to have a perfect implementation.
It's clear that people will never stop to try doing it, so at least let's make
it possible using a custom module.

It's also probably worthwhile to mention that the custom extension implementing
server side statement level rollback wasn't implemented because it wasn't
doable in the client side, but because the client side implementation was
causing a really big overhead due to the need of sending the extra commands,
and putting it on the server side lead to really significant performance
improvement.

> Although that I have not though about other uses for these hooks, they
> will allow a full server side statement-level rollback feature like in
> commercial DBMSs like DB2 and Oracle. This feature is very often
> requested by users that want to migrate to PostgreSQL.

I also thought about it, and I don't really see other possible usage for those
hooks.

> There is no additional syntax or GUC, the patch just adds three new hooks:
>
>
> * start_xact_command_hook called at end of the start_xact_command()
> function.
> * finish_xact_command called in finish_xact_command() just before
> CommitTransactionCommand().
> * abort_current_transaction_hook called after an error is encountered at
> end of AbortCurrentTransaction().
>
> These hooks allow an external plugins to execute code related to the SQL
> statements executed in a transaction.

The only comment I have for those hooks is for the
abort_current_transaction_hook. AbortCurrentTransaction() can be called
recursively, so should the hook provide some more information about the
CurrentTransactionState, like the blockState, or is
GetCurrentTransactionNestLevel() enough to act only for the wanted calls?

[1] https://www.postgresql.org/message-id/20181207192006.rf4tkfl25oc6pqmv@alvherre.pgsql