Re: [PATCH] Simple progress reporting for COPY command - Mailing list pgsql-hackers

From Bharath Rupireddy
Subject Re: [PATCH] Simple progress reporting for COPY command
Date
Msg-id CALj2ACWAAbVuTvnLAL3B82ZLwyQaF6nteFsABABwxPJqL7gP1A@mail.gmail.com
Whole thread Raw
In response to [PATCH] Simple progress reporting for COPY command  (Josef Šimánek <josef.simanek@gmail.com>)
Responses Re: [PATCH] Simple progress reporting for COPY command  (Josef Šimánek <josef.simanek@gmail.com>)
List pgsql-hackers
On Fri, Jan 1, 2021 at 6:55 AM Josef Šimánek <josef.simanek@gmail.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?

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.

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



pgsql-hackers by date:

Previous
From: Bharath Rupireddy
Date:
Subject: Re: [PATCH] postgres_fdw connection caching - cause remote sessions linger till the local session exit
Next
From: Andrey Borodin
Date:
Subject: Re: [PATCH] Simplify permission checking logic in user.c