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. One minor point: + format can be faster than the text format, but it is less portable + across machine architectures and PostgreSQL versions. As per email [1], it would be better to use <productname> tag here with PostgreSQL. [1] - https://www.postgresql.org/message-id/932629.1679322674%40sss.pgh.pa.us -- With Regards, Amit Kapila.