On Wed, Mar 30, 2022 at 5:37 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> On Wed, Mar 30, 2022 at 4:29 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Wed, Mar 30, 2022 at 12:22 PM vignesh C <vignesh21@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.
