Subject: Re: [PATCH] Simple progress reporting for COPY command
To: =?UTF-8?B?Sm9zZWYgxaBpbcOhbmVr?= <josef.simanek@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
References: 
 <CAFp7Qwr6_FmRM6pCO0x_a0mymOfX_Gg+FEKet4XaTGSW=LitKQ@mail.gmail.com>
 <CALj2ACWAAbVuTvnLAL3B82ZLwyQaF6nteFsABABwxPJqL7gP1A@mail.gmail.com>
 <CAFp7QwoWx2voi2j5oaFS4Zk1tVs+OAKb8YqGZTtUVxGqwL8xgA@mail.gmail.com>
 <6e7b056e-faff-40f6-730d-3ad6d2d1b5e9@enterprisedb.com>
 <CAFp7QwqKp6_iM0R7R+f8pjwWfD+xxgm2NbDLLzj3+2RhMhcJog@mail.gmail.com>
 <CAFp7Qwop_j94NaZ4OoOSNpCiLY3m+eq6WO3RmCWCw7Y0tOL_Fw@mail.gmail.com>
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
Message-ID: <1562ca9b-ffb5-e5e9-0399-33db90dbe153@enterprisedb.com>
Date: Wed, 6 Jan 2021 22:44:49 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.6.0
MIME-Version: 1.0
In-Reply-To: 
 <CAFp7Qwop_j94NaZ4OoOSNpCiLY3m+eq6WO3RmCWCw7Y0tOL_Fw@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 1/5/21 11:02 AM, Josef Šimánek wrote:
> I'm attaching the whole patch since commitfest failed to ingest the
> last incremental on CI.
> 

Yeah, the whole patch needs to be attached for the commitfest tester to 
work correctly - it can't apply pieces from multiple messages, etc.

Anyway, I pushed this last version of patch, after a couple more tweaks, 
mainly to the docs - one place used pg_stat_copy_progress, the section 
was not indexed properly, and so on.

I see Matthias proposed to change "lines" to "tuples" - I only saw the 
message after pushing, but I probably wouldn't make that change anyway. 
The CSV docs seem to talk about lines, newlines etc. so it seems fine. 
If not, we can change that.

One more question, though - I now realize the lines_processed ignores 
rows skipped because of BEFORE INSERT triggers. I wonder if that's the 
right thing to do? Imagine you know the number of lines in a file. You 
can't really use (lines_processed / total_lines) to measure progress, 
because that may ignore many "skipped" rows. So maybe this should be 
changed to count all rows. OTOH we still have bytes_processed.


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company