Hi, On 2021-07-30 17:49:09 -0400, Tom Lane wrote: > Andres Freund <and...@anarazel.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 know I've wanted additional events in XactEvent before that'd also be problematic for pg_fdw, but not make sense as a separate event. E.g. an event when an xid is assigned. > > 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'm more favorable than you on the overall goal. Migrations to PG are a frequent and good thing and as discussed before, lots of PG forks ended up implementing a version of this. Clearly there's demand. However, I think a proper implementation would require a substantial amount of effort. Including things like optimizing the subtransaction logic so that switching the feature on doesn't lead to xid wraparound issues. Adding odd hooks doesn't move us towards a real solution imo. Greetings, Andres Freund
