Re: Improvements and additions to COPY progress reporting - Mailing list pgsql-hackers

From Matthias van de Meent
Subject Re: Improvements and additions to COPY progress reporting
Date
Msg-id CAEze2Wi2BS12wj3ZggQwcacW8e9inruaSZpfbWZ40O3Xp_Q-aA@mail.gmail.com
Whole thread Raw
In response to Re: Improvements and additions to COPY progress reporting  (Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>)
Responses Re: Improvements and additions to COPY progress reporting
List pgsql-hackers
On Wed, 10 Feb 2021 at 07:43, Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> On Tue, Feb 9, 2021 at 6:02 PM Matthias van de Meent
> <boekewurm+postgres@gmail.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.simanek@gmail.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.


With regards,

Matthias van de Meent



pgsql-hackers by date:

Previous
From: Masahiko Sawada
Date:
Subject: Re: Transactions involving multiple postgres foreign servers, take 2
Next
From: Robert Haas
Date:
Subject: Re: Extensibility of the PostgreSQL wire protocol