Re: pg_upgrade and logical replication - Mailing list pgsql-hackers

From vignesh C
Subject Re: pg_upgrade and logical replication
Date
Msg-id CALDaNm3cw=Y10FoSO6HF-Ys1-8bxw6QBZYW-6mw6YUiGHUQxAg@mail.gmail.com
Whole thread Raw
In response to Re: pg_upgrade and logical replication  (Peter Smith <smithpb2250@gmail.com>)
List pgsql-hackers
On Wed, 10 May 2023 at 13:39, Peter Smith <smithpb2250@gmail.com> wrote:
>
> On Mon, Apr 24, 2023 at 4:19 PM Julien Rouhaud <rjuju123@gmail.com> wrote:
> >
> > Hi,
> >
> > On Thu, Apr 13, 2023 at 03:26:56PM +1000, Peter Smith wrote:
> > >
> > > 1.
> > > All the comments look alike, so it is hard to know what is going on.
> > > If each of the main test parts could be highlighted then the test code
> > > would be easier to read IMO.
> > >
> > > Something like below:
> > > [...]
> >
> > I added a bit more comments about what's is being tested.  I'm not sure that a
> > big TEST CASE prefix is necessary, as it's not really multiple separated test
> > cases and other stuff can be tested in between.  Also AFAICT no other TAP test
> > current needs this kind of banner, even if they're testing more complex
> > scenario.
>
> Hmm, I think there are plenty of examples of subscription TAP tests
> having some kind of highlighted comments as suggested, for better
> readability.
>
> e.g. See src/test/subscription
> t/014_binary.pl
> t/015_stream.pl
> t/016_stream_subxact.pl
> t/018_stream_subxact_abort.pl
> t/021_twophase.pl
> t/022_twophase_cascade.pl
> t/023_twophase_stream.pl
> t/028_row_filter.pl
> t/030_origin.pl
> t/031_column_list.pl
> t/032_subscribe_use_index.pl
>
> A simple #################### to separate the main test parts is all
> that is needed.

Modified

>
> > > 4b.
> > > All these messages like "Table t1 should still have 2 rows on the new
> > > subscriber" don't seem very helpful. e.g. They are not saying anything
> > > about WHAT this is testing or WHY it should still have 2 rows.
> >
> > I don't think that those messages are supposed to say what or why something is
> > tested, just give a quick context / reference on the test in case it's broken.
> > The comments are there to explain in more details what is tested and/or why.
> >
>
> But, why can’t they do both? They can be a quick reference *and* at
> the same time give some more meaning to the error log.  Otherwise,
> these messages might as well just say ‘ref1’, ‘ref2’, ‘ref3’...

Modified

These are handled as part of v7 posted at [1].
[1] - https://www.postgresql.org/message-id/CALDaNm1ZrbHaWpJwwNhDTJocRKWd3rEkgJazuDdZ9Z-WdvonFg%40mail.gmail.com

Regards,
Vignesh



pgsql-hackers by date:

Previous
From: vignesh C
Date:
Subject: Re: pg_upgrade and logical replication
Next
From: Martín Marqués
Date:
Subject: Re: Possibility to disable `ALTER SYSTEM`