> On Dec 13, 2021, at 10:18 PM, Jeff Davis <pg...@j-davis.com> wrote:
>
>> This is implemented but I still need to update the documentation
>> before
>> posting.
>
> We also discussed how much of the insert path we want to include in the
> apply worker. The immediate need is for the patch to support RLS, but
> it raised the question: "why not just use the entire ExecInsert()
> path?".
I went a different direction with this. The need is to prevent non-superuser
subscription owners from *circumventing* RLS. For this patch set, I'm just
checking whether RLS should be enforced against the subscription owner, and if
so, raising an ERROR, same as for a privilege violation. This should be
sufficient in practice, as the table owner, roles with bypassrls privilege, and
superusers should still be able to replicate into an RLS enabled table.
The problem with actually *supporting* RLS is that the current logical
replication implementation is a mix of different practical (rather than
theoretical) design choices. After working to get RLS working as part of that,
I became concerned that there may be subtle differences between how I was
making RLS work in logical replication vs. how it works for regular
inserts/updates/deletes. Rather than create a mess that would need to be
supported going forward, I bailed out and just forbade it.
As far as completeness and a clean implementation (but not performance) are
concerned, using ExecInsert is quite appealing. I think that would require
reworking the logical replication protocol to send more than one row at a time,
so that the overhead of such a choice is not paid *per row*. That seems quite
out of scope for this release cycle, though.
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
