From: Tom Lane <tgl@sss.pgh.pa.us>
To: Mark Dilger <mark.dilger@enterprisedb.com>
cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
        Peter Eisentraut <peter.eisentraut@enterprisedb.com>,
        Simon Riggs <simon.riggs@enterprisedb.com>
Subject: Re: logical replication of truncate command with trigger causes
 Assert
In-reply-to: <CBED95D4-5072-4A16-9271-72AB73478817@enterprisedb.com>
References: <B4A3AF82-79ED-4F4C-A4E5-CD2622098972@enterprisedb.com>
 <1250962.1623192946@sss.pgh.pa.us>
 <CBED95D4-5072-4A16-9271-72AB73478817@enterprisedb.com>
Comments: In-reply-to Mark Dilger <mark.dilger@enterprisedb.com>
	message dated "Tue, 08 Jun 2021 16:23:47 -0700"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1254156.1623196736.1@sss.pgh.pa.us>
Date: Tue, 08 Jun 2021 19:58:56 -0400
Message-ID: <1254157.1623196736@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/1254157.1623196736%40sss.pgh.pa.us>
Precedence: bulk

Mark Dilger <mark.dilger@enterprisedb.com> writes:
> On Jun 8, 2021, at 3:55 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> 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.

> I was looking at those for other reasons prior to hitting this bug.

After looking at it a bit, I see a couple of options:

1. Just wrap the call of ExecuteTruncateGuts with
PushActiveSnapshot(GetTransactionSnapshot()) and PopActiveSnapshot().

2. Decide that we ought to ensure that a snapshot exists throughout
most of this code.  It's not entirely obvious to me that there is no
code path reachable from, say, apply_handle_truncate's collection of
relation OIDs that needs a snapshot.  If we went for that, I'd think
the right solution is to do PushActiveSnapshot right after each
ensure_transaction call, and then PopActiveSnapshot on the way out of
the respective subroutine.  We could then drop the snapshot management
calls that are currently associated with the executor state.

> My purpose was to figure out how to get the code to respect
> privileges.  Perhaps the solution to these two issues is related.
> I don't know yet.

Doesn't seem tremendously related.  But yeah, there is Stuff That
Is Missing in these code paths.

			regards, tom lane