Re: BUG #15293: Stored Procedure Triggered by Logical Replication is Unable to use Notification Events

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Artur Zakirov <artur(dot)zakirov(at)adjust(dot)com>
Cc: Sergey Fedchenko <seregayoga(at)bk(dot)ru>, Daniel Danzberger <daniel(at)dd-wrt(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Sergei Kornilov <sk(at)zsrv(dot)org>, pgsql-hackers(at)postgresql(dot)org, Peter Eisentraut <peter_e(at)gmx(dot)net>, Marc Dean <marc(dot)dean(dot)jr(at)gmail(dot)com>, Petr Jelinek <petr(at)2ndquadrant(dot)com>, "michael(dot)paul(dot)powers(at)gmail(dot)com" <michael(dot)paul(dot)powers(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Robert Welin <robert(at)vaion(dot)com>
Subject: Re: BUG #15293: Stored Procedure Triggered by Logical Replication is Unable to use Notification Events
Date: 2021-09-11 22:16:09
Message-ID: 170808.1631398569@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

Artur Zakirov <artur(dot)zakirov(at)adjust(dot)com> writes:
> On Mon, Sep 6, 2021 at 9:27 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Hm. So that forecloses back-patching this to earlier than v13.
>> On the other hand, given that we've been ignoring the bug for awhile,
>> maybe a fix that only works in v13+ is good enough. (Or maybe by now
>> it'd be safe to back-patch the v13-era async.c changes? Don't really
>> want to, though.)

> I think it would be better to back-patch the fix to older versions
> than v13. But considering that the new patch renames
> ProcessCompletedNotifies(), it can break existing applications which
> use background workers and NOTIFY. Users can be upset with the change,
> since they don't face the original bug, they just call
> ProcessCompletedNotifies() manually.

I've not looked at your new patch yet, but I did spend a little time
checking into whether it'd be sane to backpatch 51004c717, which
changed SignalBackends' API. (We'd also need the non-catalog parts
of 8b7ae5a82, presumably.) I came away feeling that the benefit
isn't worth the effort and risk. There was an awful lot of churn
in async.c since v12, and to make matters worse, some of it was
back-patched already while some wasn't. So 51004c717 doesn't apply
to v12 at all cleanly, and some of the merge problems are due to
code that's older than that commit but others are due to code that's
newer. It would take some work just to get a patch that applies,
and I would not have a lot of faith in the result. I think we're
better off just settling for a back-patch to v13. The delta in
async.c between v13 and HEAD is not very large, so we'd not be
taking too much risk in going that far back.

As far as ProcessCompletedNotifies is concerned, I think what we
could do in the back branches is leave it in place as an empty,
no-op function, so as not to break extensions that are following
the existing documentation by calling it.

regards, tom lane

In response to

Browse pgsql-bugs by date

  From Date Subject
Next Message Justin Pryzby 2021-09-12 00:51:16 Re: pg_upgrade test for binary compatibility of core data types
Previous Message Artur Zakirov 2021-09-11 19:55:16 Re: BUG #15293: Stored Procedure Triggered by Logical Replication is Unable to use Notification Events

Browse pgsql-hackers by date

  From Date Subject
Next Message Justin Pryzby 2021-09-12 00:51:16 Re: pg_upgrade test for binary compatibility of core data types
Previous Message Aleksander Alekseev 2021-09-11 20:58:14 Re: 2021-09 Commitfest