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. 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. Thanks, -- Melih Mutlu Microsoft
