On Mon, May 22, 2023 at 10:36 PM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > On Wed, May 17, 2023 at 10:10 AM Ajin Cherian <itsa...@gmail.com> wrote: > > > > On Mon, May 15, 2023 at 10:47 PM Masahiko Sawada <sawada.m...@gmail.com> > > wrote: > > > > > > On Mon, May 15, 2023 at 5:44 PM Ajin Cherian <itsa...@gmail.com> wrote: > > > > > > > > On Fri, May 12, 2023 at 9:55 PM Ajin Cherian <itsa...@gmail.com> wrote: > > > > > > > > > > If nobody else is working on this, I can come up with a patch to fix > > > > > this > > > > > > > > > > > > > Attaching a patch which attempts to fix this. > > > > > > > > > > Thank you for the patch! I think we might want to have tests for it. > > > > > I have updated the patch with a test case as well. > > Thank you for updating the patch! Here are review comments: > > + /* > + * Make sure that the copy command runs as the table owner, unless > + * the user has opted out of that behaviour. > + */ > + run_as_owner = MySubscription->runasowner; > + if (!run_as_owner) > + SwitchToUntrustedUser(rel->rd_rel->relowner, &ucxt); > + > /* Now do the initial data copy */ > PushActiveSnapshot(GetTransactionSnapshot()); > > I think we should switch users before the acl check in > LogicalRepSyncTableStart(). > > --- > +# Create a trigger on table alice.unpartitioned that writes > +# to a table that regress_alice does not have permission. > +$node_subscriber->safe_psql( > + 'postgres', qq( > +SET SESSION AUTHORIZATION regress_alice; > +CREATE OR REPLACE FUNCTION alice.alice_audit() > +RETURNS trigger AS > +\$\$ > + BEGIN > + insert into public.admin_audit values(2); > + RETURN NEW; > + END; > +\$\$ > +LANGUAGE 'plpgsql'; > +CREATE TRIGGER ALICE_TRIGGER AFTER INSERT ON alice.unpartitioned FOR EACH ROW > +EXECUTE PROCEDURE alice.alice_audit(); > +ALTER TABLE alice.unpartitioned ENABLE ALWAYS TRIGGER ALICE_TRIGGER; > +)); > > While this approach works, I'm not sure we really need a trigger for > this test. I've attached a patch for discussion that doesn't use > triggers for the regression tests. We create a new subscription owned > by a user who doesn't have the permission to the target table. The > test passes only if run_as_owner = true works. > this is better, thanks. Since you are testing run_as_owner = false behaviour during table copy phase, you might as well add a test case that it correctly behaves during insert replication as well.
regards, Ajin Cherian Fujitsu Australia
