Re: logical replication of truncate command with trigger causes Assert

> On Jun 8, 2021, at 3:55 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> 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.

Sure, I didn't expect that you'd broken things so much as we now have an Assert where, at least for simple commands, things were working back in April. Those things may not have been working correctly -- I'll have to do some more test development to see if I can get the pre-84f5c2908da to misbehave -- but this may appear to be a regression in version 14 if we don't do something.

Calling ExecuteTruncateGuts from within the logical replication worker was introduced in commit 039eb6e92f2, "Logical replication support for TRUNCATE", back in April 2018. So whatever we do will likely need to be backpatched.

> 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. 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.

As it stands, a subscription can only be created by a superuser, and the replication happens under that user's current_user and session_user. I naively thought that adding a built-in role pg_logical_replication which could create subscriptions would be of some use. I implemented that but, but now if I create a user named "replication_manager" with membership in pg_logical_replication but not superuser, it turns out that even though the apply worker runs as replication_manager, the insert/update/delete commands work without checking privileges. (They can insert/update/delete tables and execute functions owned by a database superuser for which "replication_manager" has no privileges.) So I need to go a bit further to get acl checks called from this code path.

> 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.

I'll dig into this a bit more.

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company