Re: Improvements and additions to COPY progress reporting

On Fri, Feb 12, 2021 at 5:40 PM Matthias van de Meent
<boekewurm+postgres(at)gmail(dot)com> wrote:
>
> On Fri, 12 Feb 2021 at 12:23, Matthias van de Meent
> <boekewurm+postgres(at)gmail(dot)com> wrote:
> >
> > On Thu, 11 Feb 2021 at 15:44, Bharath Rupireddy
> > <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> > >
> > >
> > > On Thu, Feb 11, 2021, 8:08 PM Josef Šimánek <josef(dot)simanek(at)gmail(dot)com> wrote:
> > >> 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.
> >
> > Thanks for your reviews. PFA v4 of the patchset, in which the tests
> > are put into copy.sql (well, input/copy.source). This also adds tests
> > for correctly reporting COPY ... FROM 'file'.
>
> PFA v5, which fixes a failure in the pg_upgrade regression tests due
> to incorrect usage of @abs_builddir(at)(dot) I had the changes staged, but
> forgot to add them to the patches.
>
> Sorry for the noise.

Looks like the patch 0001 that was adding tuples_excluded was missing
and cfbot is also not happy with the v5 patch set.

Maybe, we may not need 6 patches as they are relatively very small
patches. IMO, the following are enough:

0001 - tuples_excluded, lines to tuples change, COPY FROM/COPY TO
addition, io_target -- basically all the code related patches can go
into 0001
0002 - documentation
0003 - tests - I think we can only have a simple test(in copy2.sql)
showing stdin/stdout and not have file related tests. Because these
patches work as expected, please find my testing below:

postgres=# select * from pg_stat_progress_copy;
pid | datid | datname | relid | command | io_target |
bytes_processed | bytes_total | tuples_processed | tuples_excluded
---------+-------+----------+-------+-----------+-----------+-----------------+-------------+------------------+-----------------
2886103 | 12977 | postgres | 16384 | COPY FROM | FILE |
83099648 | 85777795 | 9553999 | 1111111
(1 row)

postgres=# select * from pg_stat_progress_copy;
pid | datid | datname | relid | command | io_target |
bytes_processed | bytes_total | tuples_processed | tuples_excluded
---------+-------+----------+-------+-----------+-----------+-----------------+-------------+------------------+-----------------
2886103 | 12977 | postgres | 16384 | COPY FROM | STDIO |
0 | 0 | 0 | 0
(1 row)

postgres=# select * from pg_stat_progress_copy;
pid | datid | datname | relid | command | io_target |
bytes_processed | bytes_total | tuples_processed | tuples_excluded
---------+-------+----------+-------+---------+-----------+-----------------+-------------+------------------+-----------------
2886103 | 12977 | postgres | 16384 | COPY TO | FILE |
37771610 | 0 | 4999228 | 0
(1 row)

postgres=# select * from pg_stat_progress_copy;
pid | datid | datname | relid | command | io_target |
bytes_processed | bytes_total | tuples_processed | tuples_excluded
---------+-------+----------+-------+-----------+-----------+-----------------+-------------+------------------+-----------------
2892816 | 12977 | postgres | 16384 | COPY FROM | CALLBACK |
249777823 | 0 | 31888892 | 0
(1 row)

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com