From: Tom Lane <tgl@sss.pgh.pa.us>
To: Mark Dilger <mark.dilger@enterprisedb.com>
cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: logical replication of truncate command with trigger causes
 Assert
In-reply-to: <B4A3AF82-79ED-4F4C-A4E5-CD2622098972@enterprisedb.com>
References: <B4A3AF82-79ED-4F4C-A4E5-CD2622098972@enterprisedb.com>
Comments: In-reply-to Mark Dilger <mark.dilger@enterprisedb.com>
	message dated "Tue, 08 Jun 2021 14:52:14 -0700"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1250961.1623192946.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Tue, 08 Jun 2021 18:55:46 -0400
Message-ID: <1250962.1623192946@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/1250962.1623192946%40sss.pgh.pa.us>
Precedence: bulk

Mark Dilger <mark.dilger@enterprisedb.com> writes:
> On master, when a statement level trigger is fired for a replicated trun=
cate command, the following stack trace is generated:

Hmm.

> I believe the issue was introduced in commit 84f5c2908da which added Ens=
urePortalSnapshotExists.   That's not going to work in the case of logical=
 replication, because there isn't an ActivePortal nor a snapshot.

The right way to say that is "commit 84f5c2908da exposed the pre-existing
unsafe behavior of this code".  It's not okay to run arbitrary user code
without holding a snapshot to protect TOAST dereference operations.

I suppose that either apply_dispatch or LogicalRepApplyLoop needs to
grow some more snapshot management logic, but I've not looked at that
code much, so I don't have an opinion on just where to add it.

There's a reasonable case to be made that running user code outside
a Portal is a just-plain-bad idea, so we should fix the logical
apply worker to make it create a suitable Portal.  I speculated about
that in the commit message for 84f5c2908da, but I don't feel like
taking point on such work.

			regards, tom lane