Re: [PATCH] Simple progress reporting for COPY command

From: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
To: Josef Šimánek <josef(dot)simanek(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: [PATCH] Simple progress reporting for COPY command
Date: 2021-01-01 10:15:50
Message-ID: CALj2ACWAAbVuTvnLAL3B82ZLwyQaF6nteFsABABwxPJqL7gP1A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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?

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

In response to

Responses

Browse pgsql-hackers by date

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