Re: [PATCH] Hooks at XactCommand level

From: Gilles Darold <gilles(at)darold(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andres Freund <andres(at)anarazel(dot)de>
Cc: Nicolas CHAHWEKILIAN <leptitstagiaire(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: [PATCH] Hooks at XactCommand level
Date: 2021-08-10 08:41:20
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Le 30/07/2021 à 23:49, Tom Lane a écrit :
> Andres Freund <andres(at)anarazel(dot)de> writes:
>> On 2021-07-30 13:58:51 -0400, Tom Lane wrote:
>>> I've not read this version of the patch, but I see from the cfbot's
>>> results that it's broken postgres_fdw.
>> I think this may partially be an issue with the way that postgres_fdw
>> uses the callback than with the patch. It disconnects from the server
>> *regardless* of the XactEvent passed to the callback. That makes it
>> really hard to extend the callback mechanism to further events...
> Perhaps. Nonetheless, I thought upthread that adding these events
> as Xact/SubXactCallback events was the wrong design, and I still
> think that. A new hook would be a more sensible way.
>> I'm *very* unconvinced it makes sense to implement a feature like this
>> in an extension / that we should expose API for that purpose. To me the
>> top-level transaction state is way too tied to our internals for it to
>> be reasonably dealt with in an extension.
> Yeah, that's the other major problem --- the use-case doesn't seem
> very convincing. I'm not even sold on the goal, let alone on trying
> to implement it by hooking into these particular places. I think
> that'll end up being buggy and fragile as well as not very performant.

I've attached the new version v5 of the patch that use a hook instead of
the use of a xact callback. Compared to the first implementation calls
to the hook have been extracted from the start_xact_command() function.
The test extension have also be updated.

If I understand well the last discussions there is no chance of having
this hook included. If there is no contrary opinion I will withdraw the
patch from the commitfest. However thank you so much to have taken time
to review this proposal.

Best regards,

Gilles Darold

Attachment Content-Type Size
command-start-hook-v5.patch text/x-patch 4.7 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Yura Sokolov 2021-08-10 08:52:59 Bug in huge simplehash
Previous Message 孙诗浩 (思才) 2021-08-10 08:27:20 add operator ^= to mean not equal (like != and <>)