Re: [PATCH] Simple progress reporting for COPY command

pá 1. 1. 2021 v 11:16 odesílatel Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> napsal:
>
> On Fri, Jan 1, 2021 at 6:55 AM Josef Šimánek <josef(dot)simanek(at)gmail(dot)com> wrote:
> > finally I had some time to revisit patch and all comments from
> > https://www.postgresql.org/message-id/CAFp7QwqMGEi4OyyaLEK9DR0%2BE%2BoK3UtA4bEjDVCa4bNkwUY2PQ%40mail.gmail.com
> > and I have prepared simple version of COPY command progress reporting.
>
> Thanks for the patch.
>
> > To keep the patch small as possible, I have introduced only a minimum
> > set of columns. It could be extended later if needed.
> >
> > Columns are inspired by CREATE INDEX progress report system view.
> >
> > pid - integer - PID of backend
> > datid - oid - OID of related database
> > datname - name - name of related database (this seems redundant, since
> > oid should be enough, but it is the same in CREATE INDEX)
> > relid - oid - oid of table related to COPY command, when not known
> > (for example when copying to file, it is 0)
> > bytes_processed - bigint - amount of bytes processed
> > bytes_total - bigint - file size in bytes if COPY FROM file (0 if not
> > COPY FROM file)
>
> It means that, for COPY FROM STDIN, bytes_total will be 0. IIUC, so
> having this parameter as 0 is an indication of the COPY command being
> from STDIN? I'm not sure whether it's discussed or not previously, but
> I personally prefer to have it as a separate column, say a varchar or
> text column with values STDIN, FILE or PROGRAM may be. And also it
> will be better if we have the format of the data streaming in, as a
> separate column, with possible values may be CSV, TEXT, BINARY.
>
> Since this progress reporting is for both COPY FROM and COPY TO,
> wouldn't it be good to have that also as a separate column, say
> command type with values FROM and TO?
>
> Thoughts?

My first patch had more columns (direction - FROM/TO, file bool,
program bool), but it was subject of discussion what's the purpose.
You can check the query by looking at pg_stat_activity by pid to get
more info. To keep it simple I decided to go with a minimal set of
columns for now. It can be extended later. I'm ok to continue on
improving this with more feedback once merged.

> I'm sure some of the points would have already been discussed. I just
> shared my thoughts here.
>
> I have not looked into the patch in detail, but it's missing tests.
> I'm sure we can not add the tests into copy.sql or copy2.sql, can we
> think of adding test cases to TAP or under isolation? I'm not sure
> whether other progress reporting commands have test cases.

I have raised the question in an old thread as well since there are no
tests for other progress commands as well. I was told it is ok for now
to keep it untested, since there's no easy way to do that for now.

https://www.postgresql.org/message-id/20200615001828.GA52676%40paquier.xyz

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