> On Nov 28, 2021, at 9:56 PM, Amit Kapila <amit.kapil...@gmail.com> wrote:
> 
> In ExecUpdate(), we convert Update to DELETE+INSERT when the
> partition constraint is failed whereas, on the subscriber-side, it
> will simply fail in this case. It is not clear to me how that is
> directly related to this patch but surely it will be a good
> improvement on its own and might help if that requires us to change
> some infrastructure here like hooking into executor at a higher level.

I would rather get a fix for non-superuser subscription owners committed than 
expand the scope of work and have this patch linger until the v16 development 
cycle.  This particular DELETE+INSERT problem sounds important but unrelated 
and out of scope.

> I agree that if we want to do all of this then that would require a
> lot of changes. However, giving an error for RLS-enabled tables might
> also be too restrictive. The few alternatives could be that (a) we
> allow subscription owners to be either have "bypassrls" attribute or
> they could be superusers. (b) don't allow initial table_sync for rls
> enabled tables. (c) evaluate/analyze what is required to allow Copy
> From to start respecting RLS policies. (d) reject replicating any
> changes to tables that have RLS enabled.
> 
> I see that you are favoring (d) which clearly has merits like lesser
> code/design change but not sure if that is the best way forward or we
> can do something better than that either by following one of (a), (b),
> (c), or something less restrictive than (d).

I was favoring option (d) only when RLS policies exist for one or more of the 
target relations.

Skipping the table_sync step while replicating tables that have RLS policies 
for subscriptions that are owned by users who lack bypassrls is interesting.  
If we make that work, it will be a more complete solution than option (d).

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





Reply via email to