On Fri, Feb 5, 2021 at 12:36 PM osumi.takami...@fujitsu.com <osumi.takami...@fujitsu.com> wrote: > > We need to add some tests to prove the new checks of AlterSubscription() work. > I chose TAP tests as we need to set connect = true for the subscription. > When it can contribute to the development, please utilize this. > I used v28 to check my patch and works as we expect. >
Thanks for writing the tests but I don't understand why you need to set connect = true for this test? I have tried below '... with connect = false' and it seems to be working: postgres=# CREATE SUBSCRIPTION mysub postgres-# CONNECTION 'host=localhost port=5432 dbname=postgres' postgres-# PUBLICATION mypublication WITH (connect = false); WARNING: tables were not subscribed, you will have to run ALTER SUBSCRIPTION ... REFRESH PUBLICATION to subscribe the tables CREATE SUBSCRIPTION postgres=# Begin; BEGIN postgres=*# Alter Subscription mysub Refresh Publication; ERROR: ALTER SUBSCRIPTION ... REFRESH is not allowed for disabled subscriptions So, if possible lets write this test in src/test/regress/sql/subscription.sql. I have another idea for a test case: What if we write a test such that it fails PK violation on copy and then drop the subscription. Then check there shouldn't be any dangling slot on the publisher? This is similar to a test in subscription/t/004_sync.pl, we can use some of that framework but have a separate test for this. -- With Regards, Amit Kapila.
