Re: Allow logical replication to copy tables in binary format

Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, 21 Mar 2023 Sal, 09:03 tarihinde
şunu yazdı:

> On Tue, Mar 21, 2023 at 7:03 AM Peter Smith <smithpb2250(at)gmail(dot)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