On Wed, Mar 30, 2022 at 5:37 PM Bharath Rupireddy <bharath.rupireddyforpostg...@gmail.com> wrote: > > On Wed, Mar 30, 2022 at 4:29 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Wed, Mar 30, 2022 at 12:22 PM vignesh C <vignes...@gmail.com> wrote: > > > > > > I have made the changes for this, attached v17 patch has the changes > > > for the same. > > > > > > > The patch looks good to me. I have made minor edits in the comments > > and docs. See the attached and let me know what you think? I intend to > > commit this tomorrow unless there are more comments or suggestions. > > I have one minor comment: > > + "Create subscription throws warning for multiple non-existent > publications"); > +$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION mysub1;"); > + "CREATE SUBSCRIPTION mysub1 CONNECTION '$publisher_connstr' > PUBLICATION mypub;" > + "ALTER SUBSCRIPTION mysub1 ADD PUBLICATION non_existent_pub" > + "ALTER SUBSCRIPTION mysub1 SET PUBLICATION non_existent_pub" > > Why should we drop the subscription mysub1 and create it for ALTER .. > ADD and ALTER .. SET tests? Can't we just do below which saves > unnecessary subscription creation, drop, wait_for_catchup and > poll_query_until? > > + "Create subscription throws warning for multiple non-existent > publications"); > + "ALTER SUBSCRIPTION mysub1 ADD PUBLICATION non_existent_pub2" > + "ALTER SUBSCRIPTION mysub1 SET PUBLICATION non_existent_pub3"
Or I would even simplify the entire tests as follows: + "CREATE SUBSCRIPTION mysub1 CONNECTION '$publisher_connstr' PUBLICATION mypub, non_existent_pub1" + "Create subscription throws warning for non-existent publication"); >> no drop of mysub1 >> + "CREATE SUBSCRIPTION mysub2 CONNECTION '$publisher_connstr' PUBLICATION non_existent_pub1, non_existent_pub2" + "Create subscription throws warning for multiple non-existent publications"); >> no drop of mysub2 >> + "ALTER SUBSCRIPTION mysub2 ADD PUBLICATION non_existent_pub3" + "Alter subscription add publication throws warning for non-existent publication"); + "ALTER SUBSCRIPTION mysub2 SET PUBLICATION non_existent_pub4" + "Alter subscription set publication throws warning for non-existent publication"); $node_subscriber->stop; $node_publisher->stop; Regards, Bharath Rupireddy.