Re: Improvements and additions to COPY progress reporting

From: Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>
To: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
Cc: Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>, Josef Šimánek <josef(dot)simanek(at)gmail(dot)com>
Subject: Re: Improvements and additions to COPY progress reporting
Date: 2021-02-11 14:27:15
Message-ID: CAEze2Wi2BS12wj3ZggQwcacW8e9inruaSZpfbWZ40O3Xp_Q-aA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

With regards,

Matthias van de Meent

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2021-02-11 14:28:10 Re: Extensibility of the PostgreSQL wire protocol
Previous Message Masahiko Sawada 2021-02-11 13:24:28 Re: Transactions involving multiple postgres foreign servers, take 2