From 94876abe0ab9c28a6f4b0ac006f356251ca4746c Mon Sep 17 00:00:00 2001 From: Matthias van de Meent Date: Mon, 8 Mar 2021 16:08:24 +0100 Subject: [PATCH v12 3/3] Adapt COPY progress reporting to report processed and inserted tuples Previously, tuples_processed was implied to be the amount of tuples inserted in COPY ... FROM. This code is now changed to make tuples_processed count all tuples that were identified in the COPY ... FROM command, and make tuples_inserted count all tuples that were inserted into the table (that is, it excludes the tuples excluded using the WHERE clause, and those that were not inserted due to triggers, or failure to insert into foreign tables). This also renumbers the columns to be back in-order, before the stamping of pg14 makes those numbers effectively immutable --- doc/src/sgml/monitoring.sgml | 30 ++++++++++++++++++++++++ src/backend/catalog/system_views.sql | 13 ++++++----- src/backend/commands/copyfrom.c | 34 ++++++++++++++++++++++++---- src/include/commands/progress.h | 13 ++++++----- src/test/regress/expected/rules.out | 13 ++++++----- src/test/regress/output/copy.source | 4 ++-- 6 files changed, 83 insertions(+), 24 deletions(-) diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 32bebc70db..aa2e15a748 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -6595,6 +6595,24 @@ SELECT pg_stat_get_backend_pid(s.backendid) AS pid, Number of tuples already processed by COPY command. + For COPY FROM this includes all tuples from the + source material (including tuples excluded by the + WHERE clause of the COPY + command, and tuples that were not inserted as a result of trigger + behaviour). For COPY TO this includes all tuples + that exist in the table, or those that are returned by the + SELECT. + + + + + + tuples_inserted bigint + + + Number of tuples inserted into the table with the + COPY FROM command. Is 0 for + COPY TO. @@ -6610,6 +6628,18 @@ SELECT pg_stat_get_backend_pid(s.backendid) AS pid, + + + The tuples_excluded column does not count the tuples + that failed to insert, but only those tuples that were available in the + source material and not eligible for insertion through exclusion by the + WHERE clause. You can calculate the number of tuples + that failed to insert into the table due to triggers (or that otherwise + silently fail to insert) by subtracting both + tuples_excluded and tuples_inserted + from tuples_processed instead. + + diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 8f9987f311..f59de36742 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -1130,18 +1130,19 @@ CREATE VIEW pg_stat_progress_copy AS SELECT S.pid AS pid, S.datid AS datid, D.datname AS datname, S.relid AS relid, - CASE S.param5 WHEN 1 THEN 'COPY FROM' + CASE S.param1 WHEN 1 THEN 'COPY FROM' WHEN 2 THEN 'COPY TO' END AS command, - CASE S.param6 WHEN 1 THEN 'FILE' + CASE S.param2 WHEN 1 THEN 'FILE' WHEN 2 THEN 'PROGRAM' WHEN 3 THEN 'PIPE' WHEN 4 THEN 'CALLBACK' END AS "type", - S.param1 AS bytes_processed, - S.param2 AS bytes_total, - S.param3 AS tuples_processed, - S.param4 AS tuples_excluded + S.param3 AS bytes_processed, + S.param4 AS bytes_total, + S.param5 AS tuples_processed, + S.param6 AS tuples_inserted, + S.param7 AS tuples_excluded FROM pg_stat_get_progress_info('COPY') AS S LEFT JOIN pg_database D ON S.datid = D.oid; diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c index 8856bf215b..0aae5ae71b 100644 --- a/src/backend/commands/copyfrom.c +++ b/src/backend/commands/copyfrom.c @@ -540,6 +540,7 @@ CopyFrom(CopyFromState cstate) CopyInsertMethod insertMethod; CopyMultiInsertInfo multiInsertInfo = {0}; /* pacify compiler */ int64 processed = 0; + int64 inserted = 0; int64 excluded = 0; bool has_before_insert_row_trig; bool has_instead_insert_row_trig; @@ -854,6 +855,9 @@ CopyFrom(CopyFromState cstate) if (!NextCopyFrom(cstate, econtext, myslot->tts_values, myslot->tts_isnull)) break; + /* We're starting processing the next tuple, so update processed */ + ++processed; + ExecStoreVirtualTuple(myslot); /* @@ -871,8 +875,15 @@ CopyFrom(CopyFromState cstate) /* Skip items that don't match COPY's WHERE clause */ if (!ExecQual(cstate->qualexpr, econtext)) { - /* Report that this tuple was filtered out by the WHERE clause */ - pgstat_progress_update_param(PROGRESS_COPY_TUPLES_EXCLUDED, ++excluded); + const int progress_params[] = { + PROGRESS_COPY_TUPLES_PROCESSED, + PROGRESS_COPY_TUPLES_EXCLUDED + }; + const int64 progress_vals[] = { + processed, + ++excluded + }; + pgstat_progress_update_multi_param(2, progress_params, progress_vals); continue; } } @@ -1001,8 +1012,18 @@ CopyFrom(CopyFromState cstate) skip_tuple = true; /* "do nothing" */ } - if (!skip_tuple) + if (skip_tuple) + pgstat_progress_update_param(PROGRESS_COPY_TUPLES_PROCESSED, processed); + else { + const int progress_params[] = { + PROGRESS_COPY_TUPLES_PROCESSED, + PROGRESS_COPY_TUPLES_INSERTED + }; + int64 progress_vals[] = { + processed, + 0 + }; /* * If there is an INSTEAD OF INSERT ROW trigger, let it handle the * tuple. Otherwise, proceed with inserting the tuple into the @@ -1073,7 +1094,11 @@ CopyFrom(CopyFromState cstate) NULL); if (myslot == NULL) /* "do nothing" */ + { + /* `processed` was updated at the top of this iteration; report the progress */ + pgstat_progress_update_param(PROGRESS_COPY_TUPLES_PROCESSED, processed); continue; /* next tuple please */ + } /* * AFTER ROW Triggers might reference the tableoid @@ -1112,7 +1137,8 @@ CopyFrom(CopyFromState cstate) * for counting tuples inserted by an INSERT command. Update * progress of the COPY command as well. */ - pgstat_progress_update_param(PROGRESS_COPY_TUPLES_PROCESSED, ++processed); + progress_vals[1] = ++inserted; + pgstat_progress_update_multi_param(2, progress_params, progress_vals); } } diff --git a/src/include/commands/progress.h b/src/include/commands/progress.h index 7884a11383..6fa78a5405 100644 --- a/src/include/commands/progress.h +++ b/src/include/commands/progress.h @@ -134,12 +134,13 @@ #define PROGRESS_BASEBACKUP_PHASE_TRANSFER_WAL 5 /* Progress parameters for PROGRESS_COPY */ -#define PROGRESS_COPY_BYTES_PROCESSED 0 -#define PROGRESS_COPY_BYTES_TOTAL 1 -#define PROGRESS_COPY_TUPLES_PROCESSED 2 -#define PROGRESS_COPY_TUPLES_EXCLUDED 3 -#define PROGRESS_COPY_COMMAND 4 -#define PROGRESS_COPY_TYPE 5 +#define PROGRESS_COPY_COMMAND 0 +#define PROGRESS_COPY_TYPE 1 +#define PROGRESS_COPY_BYTES_PROCESSED 2 +#define PROGRESS_COPY_BYTES_TOTAL 3 +#define PROGRESS_COPY_TUPLES_PROCESSED 4 +#define PROGRESS_COPY_TUPLES_INSERTED 5 +#define PROGRESS_COPY_TUPLES_EXCLUDED 6 /* Commands of PROGRESS_COPY_COMMAND */ #define PROGRESS_COPY_COMMAND_FROM 1 diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index 02f5e7c905..8558fec72a 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -1950,22 +1950,23 @@ pg_stat_progress_copy| SELECT s.pid, s.datid, d.datname, s.relid, - CASE s.param5 + CASE s.param1 WHEN 1 THEN 'COPY FROM'::text WHEN 2 THEN 'COPY TO'::text ELSE NULL::text END AS command, - CASE s.param6 + CASE s.param2 WHEN 1 THEN 'FILE'::text WHEN 2 THEN 'PROGRAM'::text WHEN 3 THEN 'PIPE'::text WHEN 4 THEN 'CALLBACK'::text ELSE NULL::text END AS type, - s.param1 AS bytes_processed, - s.param2 AS bytes_total, - s.param3 AS tuples_processed, - s.param4 AS tuples_excluded + s.param3 AS bytes_processed, + s.param4 AS bytes_total, + s.param5 AS tuples_processed, + s.param6 AS tuples_inserted, + s.param7 AS tuples_excluded FROM (pg_stat_get_progress_info('COPY'::text) s(pid, datid, relid, param1, param2, param3, param4, param5, param6, param7, param8, param9, param10, param11, param12, param13, param14, param15, param16, param17, param18, param19, param20) LEFT JOIN pg_database d ON ((s.datid = d.oid))); pg_stat_progress_create_index| SELECT s.pid, diff --git a/src/test/regress/output/copy.source b/src/test/regress/output/copy.source index 6819c1334c..eb3f38ea9e 100644 --- a/src/test/regress/output/copy.source +++ b/src/test/regress/output/copy.source @@ -203,11 +203,11 @@ create trigger check_after_progress_reporting execute function notice_after_progress_reporting(); -- reporting of STDIO imports, and correct bytes-processed/tuples-processed reporting copy progress_reporting from stdin; -INFO: progress: {"type": "PIPE", "command": "COPY FROM", "datname": "regression", "bytes_total": 0, "bytes_processed": 79, "tuples_excluded": 0, "tuples_processed": 3} +INFO: progress: {"type": "PIPE", "command": "COPY FROM", "datname": "regression", "bytes_total": 0, "bytes_processed": 79, "tuples_excluded": 0, "tuples_inserted": 3, "tuples_processed": 3} -- reporting of FILE imports, and correct reporting of tuples-excluded copy progress_reporting from '@abs_srcdir@/data/emp.data' where (salary < 2000); -INFO: progress: {"type": "FILE", "command": "COPY FROM", "datname": "regression", "bytes_total": 79, "bytes_processed": 79, "tuples_excluded": 1, "tuples_processed": 2} +INFO: progress: {"type": "FILE", "command": "COPY FROM", "datname": "regression", "bytes_total": 79, "bytes_processed": 79, "tuples_excluded": 1, "tuples_inserted": 2, "tuples_processed": 3} -- cleanup progress_reporting drop trigger check_after_progress_reporting on progress_reporting; drop function notice_after_progress_reporting(); -- 2.20.1