From: | Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com> |
---|---|
To: | Josef Šimánek <josef(dot)simanek(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: [PATCH] Simple progress reporting for COPY command |
Date: | 2021-01-06 21:44:49 |
Message-ID: | 1562ca9b-ffb5-e5e9-0399-33db90dbe153@enterprisedb.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Geoghegan | 2021-01-06 22:02:05 | Re: data_checksums enabled by default (was: Move --data-checksums to common options in initdb --help) |
Previous Message | Michael Banck | 2021-01-06 21:29:51 | Re: data_checksums enabled by default (was: Move --data-checksums to common options in initdb --help) |