On Wed, Mar 10, 2021 at 09:35:10AM +0100, Matthias van de Meent wrote: > There are examples in which pg_stat_progress_* -views report > inaccurate data. I think it is fairly reasonable to at least validate > some part of the progress reporting, as it is one of the few methods > for administrators to look at the state of currently running > administrative tasks, and as such, this user-visible api should be > validated.
Looking closer at 0002, the size numbers are actually incorrect on Windows for the second query. The CRLFs at the end of each line of emp.data add three bytes to the report of COPY FROM, so this finishes with 82 bytes for bytes_total and bytes_processed instead of 79. Let's make this useful but simpler here, so I propose to check that the counters are higher than zero instead of an exact number. Let's also add the relation name relid::regclass while on it. The tests introduced are rather limited, but you are right that something is better than nothing here, and I have slightly updated what the tests sent previously as per the attached. What do you think? -- Michael
diff --git a/src/test/regress/input/copy.source b/src/test/regress/input/copy.source index a1d529ad36..9e4766898d 100644 --- a/src/test/regress/input/copy.source +++ b/src/test/regress/input/copy.source @@ -201,3 +201,65 @@ select * from parted_copytest where b = 1; select * from parted_copytest where b = 2; drop table parted_copytest; + +-- +-- Progress reporting for COPY +-- +create table tab_progress_reporting ( + name text, + age int4, + location point, + salary int4, + manager name +); + +-- Add a trigger to catch and print the contents of the catalog view +-- pg_stat_progress_copy during data insertion. This allows to test +-- the validation of some progress reports for COPY FROM where the trigger +-- would fire. +create function notice_after_tab_progress_reporting() returns trigger AS +$$ +declare report record; +begin + -- The fields ignored here are the ones that may not remain + -- consistent across multiple runs. The sizes reported may differ + -- across platforms, so just check if these are strictly positive. + with progress_data as ( + select + relid::regclass::text as relname, + command, + type, + bytes_processed > 0 as has_bytes_processed, + bytes_total > 0 as has_bytes_total, + tuples_processed, + tuples_excluded + from pg_stat_progress_copy + where pid = pg_backend_pid()) + select into report (to_jsonb(r)) as value + from progress_data r; + + raise info 'progress: %', report.value::text; + return new; +end; +$$ language plpgsql; + +create trigger check_after_tab_progress_reporting + after insert on tab_progress_reporting + for each statement + execute function notice_after_tab_progress_reporting(); + +-- Generate COPY FROM report with PIPE. +copy tab_progress_reporting from stdin; +sharon 25 (15,12) 1000 sam +sam 30 (10,5) 2000 bill +bill 20 (11,10) 1000 sharon +\. + +-- Generate COPY FROM report with FILE, with some excluded tuples. +truncate tab_progress_reporting; +copy tab_progress_reporting from '@abs_srcdir@/data/emp.data' + where (salary < 2000); + +drop trigger check_after_tab_progress_reporting on tab_progress_reporting; +drop function notice_after_tab_progress_reporting(); +drop table tab_progress_reporting; diff --git a/src/test/regress/output/copy.source b/src/test/regress/output/copy.source index 938d3551da..8cf37dc257 100644 --- a/src/test/regress/output/copy.source +++ b/src/test/regress/output/copy.source @@ -165,3 +165,57 @@ select * from parted_copytest where b = 2; (1 row) drop table parted_copytest; +-- +-- Progress reporting for COPY +-- +create table tab_progress_reporting ( + name text, + age int4, + location point, + salary int4, + manager name +); +-- Add a trigger to catch and print the contents of the catalog view +-- pg_stat_progress_copy during data insertion. This allows to test +-- the validation of some progress reports for COPY FROM where the trigger +-- would fire. +create function notice_after_tab_progress_reporting() returns trigger AS +$$ +declare report record; +begin + -- The fields ignored here are the ones that may not remain + -- consistent across multiple runs. The sizes reported may differ + -- across platforms, so just check if these are strictly positive. + with progress_data as ( + select + relid::regclass::text as relname, + command, + type, + bytes_processed > 0 as has_bytes_processed, + bytes_total > 0 as has_bytes_total, + tuples_processed, + tuples_excluded + from pg_stat_progress_copy + where pid = pg_backend_pid()) + select into report (to_jsonb(r)) as value + from progress_data r; + + raise info 'progress: %', report.value::text; + return new; +end; +$$ language plpgsql; +create trigger check_after_tab_progress_reporting + after insert on tab_progress_reporting + for each statement + execute function notice_after_tab_progress_reporting(); +-- Generate COPY FROM report with PIPE. +copy tab_progress_reporting from stdin; +INFO: progress: {"type": "PIPE", "command": "COPY FROM", "relname": "tab_progress_reporting", "has_bytes_total": false, "tuples_excluded": 0, "tuples_processed": 3, "has_bytes_processed": true} +-- Generate COPY FROM report with FILE, with some excluded tuples. +truncate tab_progress_reporting; +copy tab_progress_reporting from '@abs_srcdir@/data/emp.data' + where (salary < 2000); +INFO: progress: {"type": "FILE", "command": "COPY FROM", "relname": "tab_progress_reporting", "has_bytes_total": true, "tuples_excluded": 1, "tuples_processed": 2, "has_bytes_processed": true} +drop trigger check_after_tab_progress_reporting on tab_progress_reporting; +drop function notice_after_tab_progress_reporting(); +drop table tab_progress_reporting;
signature.asc
Description: PGP signature