Hi,
4ac2a9bec introduced reject_limit option to the COPY command, and I was
wondering if it might be beneficial to add the same option to file_fdw.
Although there may be fewer practical use cases compared to COPY, it
could still be useful in situations where the file being read via
file_fdw is subject to modifications and there is a need to tolerate a
limited number of errors.
What do you think?
I've attached a patch.
Based on the synopsis of the CREATE/ALTER FOREIGN TABLE commands, the
value for the foreign table's option must be single-quoted. I’m not
entirely sure if this is the correct approach, but in order to
accommodate this, the patch modifies the value of reject_limit option to
accept not only numeric values but also strings.
--
Regards,
--
Atsushi Torikoshi
NTT DATA Group Corporation
From 8d969465257ebc511a0d1b1520f29095103ae4af Mon Sep 17 00:00:00 2001
From: Atsushi Torikoshi <torikos...@oss.nttdata.com>
Date: Thu, 17 Oct 2024 22:19:06 +0900
Subject: [PATCH v1] file_fdw: Add reject_limit option to file_fdw
---
contrib/file_fdw/expected/file_fdw.out | 10 +++++++++-
contrib/file_fdw/file_fdw.c | 8 ++++++++
contrib/file_fdw/sql/file_fdw.sql | 4 +++-
doc/src/sgml/file-fdw.sgml | 12 ++++++++++++
src/backend/commands/copy.c | 12 +++++++++++-
5 files changed, 43 insertions(+), 3 deletions(-)
diff --git a/contrib/file_fdw/expected/file_fdw.out b/contrib/file_fdw/expected/file_fdw.out
index 593fdc782e..1accc36d68 100644
--- a/contrib/file_fdw/expected/file_fdw.out
+++ b/contrib/file_fdw/expected/file_fdw.out
@@ -206,7 +206,7 @@ SELECT * FROM agg_csv c JOIN agg_text t ON (t.a = c.a) ORDER BY c.a;
SELECT * FROM agg_bad; -- ERROR
ERROR: invalid input syntax for type real: "aaa"
CONTEXT: COPY agg_bad, line 3, column b: "aaa"
--- on_error and log_verbosity tests
+-- on_error, log_verbosity and reject_limit tests
ALTER FOREIGN TABLE agg_bad OPTIONS (ADD on_error 'ignore');
SELECT * FROM agg_bad;
NOTICE: 1 row was skipped due to data type incompatibility
@@ -224,6 +224,14 @@ SELECT * FROM agg_bad;
42 | 324.78
(2 rows)
+ALTER FOREIGN TABLE agg_bad OPTIONS (reject_limit '1');
+SELECT * FROM agg_bad;
+ a | b
+-----+--------
+ 100 | 99.097
+ 42 | 324.78
+(2 rows)
+
ANALYZE agg_bad;
-- misc query tests
\t on
diff --git a/contrib/file_fdw/file_fdw.c b/contrib/file_fdw/file_fdw.c
index 043204c3e7..2ae83ba09b 100644
--- a/contrib/file_fdw/file_fdw.c
+++ b/contrib/file_fdw/file_fdw.c
@@ -77,6 +77,7 @@ static const struct FileFdwOption valid_options[] = {
{"encoding", ForeignTableRelationId},
{"on_error", ForeignTableRelationId},
{"log_verbosity", ForeignTableRelationId},
+ {"reject_limit", ForeignTableRelationId},
{"force_not_null", AttributeRelationId},
{"force_null", AttributeRelationId},
@@ -788,6 +789,13 @@ retry:
*/
ResetPerTupleExprContext(estate);
+ if (cstate->opts.reject_limit > 0 && \
+ cstate->num_errors > cstate->opts.reject_limit)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+ errmsg("skipped more than REJECT_LIMIT (%lld) rows due to data type incompatibility",
+ (long long) cstate->opts.reject_limit)));
+
/* Repeat NextCopyFrom() until no soft error occurs */
goto retry;
}
diff --git a/contrib/file_fdw/sql/file_fdw.sql b/contrib/file_fdw/sql/file_fdw.sql
index edd77c5cd2..ee66d7d831 100644
--- a/contrib/file_fdw/sql/file_fdw.sql
+++ b/contrib/file_fdw/sql/file_fdw.sql
@@ -150,11 +150,13 @@ SELECT * FROM agg_csv c JOIN agg_text t ON (t.a = c.a) ORDER BY c.a;
-- error context report tests
SELECT * FROM agg_bad; -- ERROR
--- on_error and log_verbosity tests
+-- on_error, log_verbosity and reject_limit tests
ALTER FOREIGN TABLE agg_bad OPTIONS (ADD on_error 'ignore');
SELECT * FROM agg_bad;
ALTER FOREIGN TABLE agg_bad OPTIONS (ADD log_verbosity 'silent');
SELECT * FROM agg_bad;
+ALTER FOREIGN TABLE agg_bad OPTIONS (reject_limit '1');
+SELECT * FROM agg_bad;
ANALYZE agg_bad;
-- misc query tests
diff --git a/doc/src/sgml/file-fdw.sgml b/doc/src/sgml/file-fdw.sgml
index bb3579b077..b05085fd7d 100644
--- a/doc/src/sgml/file-fdw.sgml
+++ b/doc/src/sgml/file-fdw.sgml
@@ -149,6 +149,18 @@
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>reject_limit</literal></term>
+
+ <listitem>
+ <para>
+ Specifies the maximum number of errors tolerated while converting a column's
+ input value to its data type, the same as <command>COPY</command>'s
+ <literal>REJECT_LIMIT</literal> option.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
<para>
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 3485ba8663..c6c8bc10b1 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -424,7 +424,17 @@ defGetCopyOnErrorChoice(DefElem *def, ParseState *pstate, bool is_from)
static int64
defGetCopyRejectLimitOption(DefElem *def)
{
- int64 reject_limit = defGetInt64(def);
+ int64 reject_limit;
+
+ if (def->arg == NULL)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("%s requires a numeric value",
+ def->defname)));
+ else if (nodeTag(def->arg) == T_String)
+ reject_limit = pg_strtoint64(strVal(def->arg));
+ else
+ reject_limit = defGetInt64(def);
if (reject_limit <= 0)
ereport(ERROR,
--
2.39.2
