On 2024/11/19 21:40, torikoshia wrote:
These messages may be unexpected for some users because the documentation of
fild_fdw does not explicitly describe that file_fdw uses COPY internally.
(I can find several wordings like "as COPY", though.)
However, since the current file_fdw already has such messages, I came to think
making the messages on "reject_limit" consistent with COPY is reasonable.
I mean, the previous version of the message seems fine.

Agreed. Thanks for your investigation.

I've pushed the 0001 patch. Thanks!


As another comment, do we need to add validator test for on_error and
reject_limit as similar to other options?

That might be better.
Added some queries which had wrong options to cause errors.

I was unsure whether to add it to "validator test" section or "on_error, log_verbosity and 
reject_limit tests" section, but I chose "validator test" as I believe it makes things clearer.

Added 0002 patch since some of the tests are not related to reject_limit but 
just on_error.

How about adding validator tests for log_verbosity and reject_limit=0,
similar to the tests in the copy2.sql regression test? I've added those
two tests, and the latest version of the patch is attached.

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
From b73cf7b8aec5d93490f0d303eda3207bd4acbf8c Mon Sep 17 00:00:00 2001
From: Atsushi Torikoshi <torikos...@oss.nttdata.com>
Date: Tue, 19 Nov 2024 21:12:52 +0900
Subject: [PATCH v6] file_fdw: Add regression tests for ON_ERROR and other
 options.

This commit introduces regression tests to validate incorrect settings
for the ON_ERROR, LOG_VERBOSITY, and REJECT_LIMIT options in file_fdw.
---
 contrib/file_fdw/expected/file_fdw.out | 8 ++++++++
 contrib/file_fdw/sql/file_fdw.sql      | 4 ++++
 2 files changed, 12 insertions(+)

diff --git a/contrib/file_fdw/expected/file_fdw.out 
b/contrib/file_fdw/expected/file_fdw.out
index 4f63c047ec..df8d43b374 100644
--- a/contrib/file_fdw/expected/file_fdw.out
+++ b/contrib/file_fdw/expected/file_fdw.out
@@ -90,8 +90,16 @@ ERROR:  COPY delimiter cannot be newline or carriage return
 CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'csv', null '
 ');       -- ERROR
 ERROR:  COPY null representation cannot use newline or carriage return
+CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (on_error 
'unsupported');       -- ERROR
+ERROR:  COPY ON_ERROR "unsupported" not recognized
+CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'binary', 
on_error 'ignore');       -- ERROR
+ERROR:  only ON_ERROR STOP is allowed in BINARY mode
+CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (log_verbosity 
'unsupported');       -- ERROR
+ERROR:  COPY LOG_VERBOSITY "unsupported" not recognized
 CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (reject_limit '1');     
  -- ERROR
 ERROR:  COPY REJECT_LIMIT requires ON_ERROR to be set to IGNORE
+CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (on_error 'ignore', 
reject_limit '0');       -- ERROR
+ERROR:  REJECT_LIMIT (0) must be greater than zero
 CREATE FOREIGN TABLE tbl () SERVER file_server;  -- ERROR
 ERROR:  either filename or program is required for file_fdw foreign tables
 \set filename :abs_srcdir '/data/agg.data'
diff --git a/contrib/file_fdw/sql/file_fdw.sql 
b/contrib/file_fdw/sql/file_fdw.sql
index 4805ca8c04..2cdbe7a8a4 100644
--- a/contrib/file_fdw/sql/file_fdw.sql
+++ b/contrib/file_fdw/sql/file_fdw.sql
@@ -77,7 +77,11 @@ CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS 
(format 'csv', delimiter
 ');       -- ERROR
 CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'csv', null '
 ');       -- ERROR
+CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (on_error 
'unsupported');       -- ERROR
+CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'binary', 
on_error 'ignore');       -- ERROR
+CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (log_verbosity 
'unsupported');       -- ERROR
 CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (reject_limit '1');     
  -- ERROR
+CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (on_error 'ignore', 
reject_limit '0');       -- ERROR
 CREATE FOREIGN TABLE tbl () SERVER file_server;  -- ERROR
 
 \set filename :abs_srcdir '/data/agg.data'
-- 
2.47.0

Reply via email to