tests for pg_stat_progress_copy.tuples_skipped

Started by jian heabout 1 year ago5 messages
#1jian he
jian.universality@gmail.com
1 attachment(s)

hi.
seems no regress tests for pg_stat_progress_copy.tuples_skipped.

CopyFrom
pgstat_progress_update_param(PROGRESS_COPY_TUPLES_SKIPPED,
cstate->num_errors);

there is coverage for it. but in regress test, we didn't really print
out this value (cstate->num_errors)

The attached patch did minor changes on src/test/regress/sql/copy.sql.
so we can check if pgstat_progress_update_param(PROGRESS_COPY_TUPLES_SKIPPED..)
is working as intended or not.

Attachments:

v1-0001-tests-for-pg_stat_progress_copy.tuples_skipped.patchtext/x-patch; charset=US-ASCII; name=v1-0001-tests-for-pg_stat_progress_copy.tuples_skipped.patchDownload
From f87c3769751e39daaf049728df7f7f5bcb226861 Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Fri, 10 Jan 2025 22:42:18 +0800
Subject: [PATCH v1 1/1] tests for pg_stat_progress_copy.tuples_skipped

---
 src/test/regress/expected/copy.out | 10 +++++++---
 src/test/regress/sql/copy.sql      |  9 ++++++++-
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/src/test/regress/expected/copy.out b/src/test/regress/expected/copy.out
index f554d42c84..11f99fbead 100644
--- a/src/test/regress/expected/copy.out
+++ b/src/test/regress/expected/copy.out
@@ -181,7 +181,8 @@ begin
        bytes_processed > 0 as has_bytes_processed,
        bytes_total > 0 as has_bytes_total,
        tuples_processed,
-       tuples_excluded
+       tuples_excluded,
+	   tuples_skipped
       from pg_stat_progress_copy
       where pid = pg_backend_pid())
   select into report (to_jsonb(r)) as value
@@ -197,13 +198,16 @@ create trigger check_after_tab_progress_reporting
 	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}
+INFO:  progress: {"type": "PIPE", "command": "COPY FROM", "relname": "tab_progress_reporting", "tuples_skipped": 0, "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;
 \set filename :abs_srcdir '/data/emp.data'
 copy tab_progress_reporting from :'filename'
 	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}
+INFO:  progress: {"type": "FILE", "command": "COPY FROM", "relname": "tab_progress_reporting", "tuples_skipped": 0, "has_bytes_total": true, "tuples_excluded": 1, "tuples_processed": 2, "has_bytes_processed": true}
+copy tab_progress_reporting from stdin(on_error ignore);
+NOTICE:  2 rows were skipped due to data type incompatibility
+INFO:  progress: {"type": "PIPE", "command": "COPY FROM", "relname": "tab_progress_reporting", "tuples_skipped": 2, "has_bytes_total": false, "tuples_excluded": 0, "tuples_processed": 1, "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;
diff --git a/src/test/regress/sql/copy.sql b/src/test/regress/sql/copy.sql
index f1699b66b0..438acaf79f 100644
--- a/src/test/regress/sql/copy.sql
+++ b/src/test/regress/sql/copy.sql
@@ -193,7 +193,8 @@ begin
        bytes_processed > 0 as has_bytes_processed,
        bytes_total > 0 as has_bytes_total,
        tuples_processed,
-       tuples_excluded
+       tuples_excluded,
+	   tuples_skipped
       from pg_stat_progress_copy
       where pid = pg_backend_pid())
   select into report (to_jsonb(r)) as value
@@ -222,6 +223,12 @@ truncate tab_progress_reporting;
 copy tab_progress_reporting from :'filename'
 	where (salary < 2000);
 
+copy tab_progress_reporting from stdin(on_error ignore);
+sharon	x	(15,12)	x	sam
+sharon	25	(15,12)	1000	sam
+sharon	y	(15,12)	x	sam
+\.
+
 drop trigger check_after_tab_progress_reporting on tab_progress_reporting;
 drop function notice_after_tab_progress_reporting();
 drop table tab_progress_reporting;
-- 
2.34.1

#2Fujii Masao
masao.fujii@oss.nttdata.com
In reply to: jian he (#1)
1 attachment(s)
Re: tests for pg_stat_progress_copy.tuples_skipped

On 2025/01/10 23:50, jian he wrote:

hi.
seems no regress tests for pg_stat_progress_copy.tuples_skipped.

CopyFrom
pgstat_progress_update_param(PROGRESS_COPY_TUPLES_SKIPPED,
cstate->num_errors);

there is coverage for it. but in regress test, we didn't really print
out this value (cstate->num_errors)

The attached patch did minor changes on src/test/regress/sql/copy.sql.
so we can check if pgstat_progress_update_param(PROGRESS_COPY_TUPLES_SKIPPED..)
is working as intended or not.

Thanks for the patch!

The patch basically looks good to me.
I’ve made some minor cosmetic adjustments — the updated patch is attached.

Unless there are any objections, I'm thinking to commit it.

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

Attachments:

v2-0001-Add-regression-tests-for-pg_stat_progress_copy.tu.patchtext/plain; charset=UTF-8; name=v2-0001-Add-regression-tests-for-pg_stat_progress_copy.tu.patchDownload
From e136b8e656a29995ec64b3fefcb07b08c0b4e43b Mon Sep 17 00:00:00 2001
From: Fujii Masao <fujii@postgresql.org>
Date: Mon, 3 Mar 2025 17:06:10 +0900
Subject: [PATCH v2] Add regression tests for
 pg_stat_progress_copy.tuples_skipped.

This commit adds tests to verify that tuples_skipped in pg_stat_progress_copy
works as expected. While existing tests checked other fields, tuples_skipped
was previously untested.

This improves test coverage and ensures accurate tracking of skipped tuples.

Author: jian he <jian.universality@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/CACJufxFazq-bfyhiO0KBojR=yOr84E25Rqf6mHB0Ow0KPidkKw@mail.gmail.com
---
 src/test/regress/expected/copy.out | 11 ++++++++---
 src/test/regress/sql/copy.sql      | 10 +++++++++-
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/src/test/regress/expected/copy.out b/src/test/regress/expected/copy.out
index e69e34c69b8..06bae8c61ae 100644
--- a/src/test/regress/expected/copy.out
+++ b/src/test/regress/expected/copy.out
@@ -181,7 +181,8 @@ begin
        bytes_processed > 0 as has_bytes_processed,
        bytes_total > 0 as has_bytes_total,
        tuples_processed,
-       tuples_excluded
+       tuples_excluded,
+       tuples_skipped
       from pg_stat_progress_copy
       where pid = pg_backend_pid())
   select into report (to_jsonb(r)) as value
@@ -197,13 +198,17 @@ create trigger check_after_tab_progress_reporting
 	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}
+INFO:  progress: {"type": "PIPE", "command": "COPY FROM", "relname": "tab_progress_reporting", "tuples_skipped": 0, "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;
 \set filename :abs_srcdir '/data/emp.data'
 copy tab_progress_reporting from :'filename'
 	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}
+INFO:  progress: {"type": "FILE", "command": "COPY FROM", "relname": "tab_progress_reporting", "tuples_skipped": 0, "has_bytes_total": true, "tuples_excluded": 1, "tuples_processed": 2, "has_bytes_processed": true}
+-- Generate COPY FROM report with PIPE, with some skipped tuples.
+copy tab_progress_reporting from stdin(on_error ignore);
+NOTICE:  2 rows were skipped due to data type incompatibility
+INFO:  progress: {"type": "PIPE", "command": "COPY FROM", "relname": "tab_progress_reporting", "tuples_skipped": 2, "has_bytes_total": false, "tuples_excluded": 0, "tuples_processed": 1, "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;
diff --git a/src/test/regress/sql/copy.sql b/src/test/regress/sql/copy.sql
index 895479d2d0f..3009bdfdf89 100644
--- a/src/test/regress/sql/copy.sql
+++ b/src/test/regress/sql/copy.sql
@@ -193,7 +193,8 @@ begin
        bytes_processed > 0 as has_bytes_processed,
        bytes_total > 0 as has_bytes_total,
        tuples_processed,
-       tuples_excluded
+       tuples_excluded,
+       tuples_skipped
       from pg_stat_progress_copy
       where pid = pg_backend_pid())
   select into report (to_jsonb(r)) as value
@@ -222,6 +223,13 @@ truncate tab_progress_reporting;
 copy tab_progress_reporting from :'filename'
 	where (salary < 2000);
 
+-- Generate COPY FROM report with PIPE, with some skipped tuples.
+copy tab_progress_reporting from stdin(on_error ignore);
+sharon	x	(15,12)	x	sam
+sharon	25	(15,12)	1000	sam
+sharon	y	(15,12)	x	sam
+\.
+
 drop trigger check_after_tab_progress_reporting on tab_progress_reporting;
 drop function notice_after_tab_progress_reporting();
 drop table tab_progress_reporting;
-- 
2.48.1

#3jian he
jian.universality@gmail.com
In reply to: Fujii Masao (#2)
Re: tests for pg_stat_progress_copy.tuples_skipped

On Mon, Mar 3, 2025 at 5:05 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:

Thanks for the patch!

The patch basically looks good to me.
I’ve made some minor cosmetic adjustments — the updated patch is attached.

Unless there are any objections, I'm thinking to commit it.

I checked it again manually.
It looks good to me.

#4Josef Šimánek
josef.simanek@gmail.com
In reply to: Fujii Masao (#2)
Re: tests for pg_stat_progress_copy.tuples_skipped

po 3. 3. 2025 v 10:05 odesílatel Fujii Masao
<masao.fujii@oss.nttdata.com> napsal:

On 2025/01/10 23:50, jian he wrote:

hi.
seems no regress tests for pg_stat_progress_copy.tuples_skipped.

CopyFrom
pgstat_progress_update_param(PROGRESS_COPY_TUPLES_SKIPPED,
cstate->num_errors);

there is coverage for it. but in regress test, we didn't really print
out this value (cstate->num_errors)

The attached patch did minor changes on src/test/regress/sql/copy.sql.
so we can check if pgstat_progress_update_param(PROGRESS_COPY_TUPLES_SKIPPED..)
is working as intended or not.

Thanks for the patch!

The patch basically looks good to me.
I’ve made some minor cosmetic adjustments — the updated patch is attached.

Unless there are any objections, I'm thinking to commit it.

Looks good to me.

Show quoted text

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

#5Fujii Masao
masao.fujii@oss.nttdata.com
In reply to: jian he (#3)
Re: tests for pg_stat_progress_copy.tuples_skipped

On 2025/03/04 17:57, jian he wrote:

On Mon, Mar 3, 2025 at 5:05 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:

Thanks for the patch!

The patch basically looks good to me.
I’ve made some minor cosmetic adjustments — the updated patch is attached.

Unless there are any objections, I'm thinking to commit it.

I checked it again manually.
It looks good to me.

I've pushed the patch. Thanks!

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION