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

Hi Tom, Peter,

On 2018-07-24 21:22:18 +0300, Sergei Kornilov wrote:
> in fact, I've already tried to build fix. Adding ProcessCompletedNotifies to apply_handle_commit fixed this issue and
ithink this is right place. In src/backend/tcop/postgres.c we call ProcessCompletedNotifies similar way after commit.
Thischange pass make check-world.
 
> So i attach my two line patch.

> diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
> index 6ca6cdc..e54bd90 100644
> --- a/src/backend/replication/logical/worker.c
> +++ b/src/backend/replication/logical/worker.c
> @@ -37,6 +37,7 @@
>  
>  #include "commands/tablecmds.h"
>  #include "commands/trigger.h"
> +#include "commands/async.h"
>  
>  #include "executor/executor.h"
>  #include "executor/nodeModifyTable.h"
> @@ -490,6 +491,7 @@ apply_handle_commit(StringInfo s)
>          replorigin_session_origin_timestamp = commit_data.committime;
>  
>          CommitTransactionCommand();
> +        ProcessCompletedNotifies();
>          pgstat_report_stat(false);
>  
>          store_flush_position(commit_data.end_lsn);

That's probably reasonable for the back branches (although I'd put the
store_flush_position before).

But I wonder if we shouldn't actually move the signalling part of
ProcessCompletedNotifies() into CommitTransactionCommand() in v11. Given
that transactions can now commit without a ready command being sent, due
to the addition of procedures, that kind of seems necessary?

Greetings,

Andres Freund