Re: Improvements and additions to COPY progress reporting

On Thu, Feb 11, 2021, 8:08 PM Josef Šimánek <josef(dot)simanek(at)gmail(dot)com> wrote:

> čt 11. 2. 2021 v 15:27 odesílatel Matthias van de Meent
> <boekewurm+postgres(at)gmail(dot)com> napsal:
> >
> > On Wed, 10 Feb 2021 at 07:43, Bharath Rupireddy
> > <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> > >
> > > On Tue, Feb 9, 2021 at 6:02 PM Matthias van de Meent
> > > <boekewurm+postgres(at)gmail(dot)com> wrote:
> > > > > Also, you can add this to the current commitfest.
> > > >
> > > > See https://commitfest.postgresql.org/32/2977/
> > > >
> > > > On Tue, 9 Feb 2021 at 12:53, Josef Šimánek <josef(dot)simanek(at)gmail(dot)com>
> wrote:
> > > > >
> > > > > OK, would you mind to integrate my regression test initial patch as
> > > > > well in v3 or should I submit it later in a separate way?
> > > >
> > > > Attached, with minor fixes
> > >
> > > Why do we need to have a new test file progress.sql for the test
> > > cases? Can't we add them into existing copy.sql or copy2.sql? Or do
> > > you have a plan to add test cases into progress.sql for other progress
> > > reporting commands?
> >
> > I don't mind moving the test into copy or copy2, but the main reason
> > to put it in a seperate file is to test the 'copy' component of the
> > feature called 'progress reporting'. If the feature instead is 'copy'
> > and 'progress reporting' is part of that feature, then I'd put it in
> > the copy-tests, but because the documentation of this has it's own
> > docs page 'progress reporting', and because 'copy' is a subsection of
> > that, I do think that this feature warrants its own regression test
> > file.
> >
> > There are no other tests for the progress reporting feature yet,
> > because COPY ... FROM is the only command that is progress reported
> > _and_ that can fire triggers while running the command, so checking
> > the progress view during the progress reported command is only
> > feasable in COPY progress reporting. To test the other progress
> > reporting views, we would need multiple sessions, which I believe is
> > impossible in this test format. Please correct me if I'm wrong; I'd
> > love to add tests for the other components. That will not be in this
> > patchset, though.
> >
> > > IMO, it's better not add any new test file but add the tests to
> existing files.
> >
> > In general I agree, but in some cases (e.g. new system component, new
> > full-fledged feature), new test files are needed. I think that this
> > could be one of those cases.
>
> I have split it since it should be the start of progress reporting
> testing at all. If you better consider this as part of COPY testing,
> feel free to move it to already existing copy testing related files.
> There's no real reason to keep it separated if not needed.
>

+1 to move those test cases to existing copy test files.