On Tue, Mar 21, 2023 at 7:03 AM Peter Smith <smithpb2250@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.
