On Tue, Mar 21, 2023 at 2:16 PM Melih Mutlu <m.melihmu...@gmail.com> wrote: > > Amit Kapila <amit.kapil...@gmail.com>, 21 Mar 2023 Sal, 09:03 tarihinde şunu > yazdı: >> >> On Tue, Mar 21, 2023 at 7:03 AM Peter Smith <smithpb2...@gmail.com> wrote: >> > >> > >> > ====== >> > src/test/subscription/t/014_binary.pl >> > >> > 4. >> > # ----------------------------------------------------- >> > # Test mismatched column types with/without binary mode >> > # ----------------------------------------------------- >> > >> > # Test syncing tables with mismatching column types >> > $node_publisher->safe_psql( >> > 'postgres', qq( >> > CREATE TABLE public.test_mismatching_types ( >> > a bigint PRIMARY KEY >> > ); >> > INSERT INTO public.test_mismatching_types (a) >> > VALUES (1), (2); >> > )); >> > >> > # Check the subscriber log from now on. >> > $offset = -s $node_subscriber->logfile; >> > >> > # Ensure the subscription is enabled. disable_on_error is still true, >> > # so the subscription can be disabled due to missing realtion until >> > # the test_mismatching_types table is created. >> > $node_subscriber->safe_psql( >> > 'postgres', qq( >> > CREATE TABLE public.test_mismatching_types ( >> > a int PRIMARY KEY >> > ); >> > ALTER SUBSCRIPTION tsub ENABLE; >> > ALTER SUBSCRIPTION tsub REFRESH PUBLICATION; >> > )); >> > >> > ~~ >> > >> > I found the "Ensure the subscription is enabled..." comment and the >> > necessity for enabling the subscription to be confusing. >> > >> > Can't some complications all be eliminated just by creating the table >> > on the subscribe side first? >> > >> >> Hmm, that would make this test inconsistent with other tests and >> probably difficult to understand and extend. I don't like to say this >> but I think introducing disable_on_error has introduced more >> complexities in the patch due to the requirement of enabling >> subscription again and again. I feel it would be better without using >> disable_on_error option in these tests. > > > While this change would make the test inconsistent, I think it also would > make more confusing. >
I also think so. > Explaining the issue explicitly with a comment seems better to me than the > trick of changing order of table creation just for some test cases. > But I'm also ok with removing the use of disable_on_error if that's what you > agree on. > Let's do that way for now. -- With Regards, Amit Kapila.
