> On Jun 8, 2021, at 3:55 PM, Tom Lane <tgl@sss.pgh.pa.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,
thingswere working back in April. Those things may not have been working correctly -- I'll have to do some more test
developmentto see if I can get the pre-84f5c2908da to misbehave -- but this may appear to be a regression in version 14
ifwe don't do something.
Calling ExecuteTruncateGuts from within the logical replication worker was introduced in commit 039eb6e92f2, "Logical
replicationsupport 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
torespect 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_userand session_user. I naively thought that adding a built-in role pg_logical_replication which could create
subscriptionswould be of some use. I implemented that but, but now if I create a user named "replication_manager" with
membershipin 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
tablesand execute functions owned by a database superuser for which "replication_manager" has no privileges.) So I
needto 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
