On Tue, Mar 5, 2024 at 4:48 AM Michael Paquier <mich...@paquier.xyz> wrote:
>
> On Mon, Mar 04, 2024 at 05:00:00AM +0530, Bharath Rupireddy wrote:
> > How about an extra option to error_action ignore-with-verbose which is
> > similar to ignore but when specified emits one NOTICE per malformed
> > row? With this, one can say COPY x FROM stdin (ON_ERROR
> > ignore-with-verbose);.
> >
> > Alternatively, we can think of adding a new option verbose altogether
> > which can be used for not only this but for reporting some other COPY
> > related info/errors etc. With this, one can say COPY x FROM stdin
> > (VERBOSE on, ON_ERROR ignore);.
>
> I would suggest a completely separate option, because that offers more
> flexibility as each option has a separate meaning. My main concern in
> using one option to control them all is that one may want in the
> future to be able to specify more combinations of actions at query
> level, especially if more modes are added to the ON_ERROR mode. One
> option would prevent that.
>
> Perhaps ERROR_VERBOSE or ERROR_VERBOSITY would be better names, but
> I'm never wedded to my naming suggestions. Bad history with the
> matter.
+1 for a separate option and LOG_VERBOSITY seemed a better and generic
naming choice. Because, the ON_ERROR ignore isn't actually an error
per se IMO.
> > There's also another way of having a separate GUC, but -100 from me
> > for it. Because, it not only increases the total number of GUCs by 1,
> > but also might set a wrong precedent to have a new GUC for controlling
> > command level outputs.
>
> What does this have to do with GUCs? The ON_ERROR option does not
> have one.
My thought was to have a separate GUC for deciding log level for COPY
command messages/errors similar to log_replication_commands. But
that's a no-go for sure when compared with a separate option.
Please see the attached v4 patch. If it looks good, I can pull
LOG_VERBOSITY changes out into 0001 and with 0002 containing the
detailed messages for discarded rows.
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
From 802b5bde48cc378dc69baa1e781548bb7182fb45 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Wed, 6 Mar 2024 13:17:41 +0000
Subject: [PATCH v4] Add detailed info when COPY skips soft errors
---
doc/src/sgml/ref/copy.sgml | 21 +++++++++++++++++++--
src/backend/commands/copy.c | 8 ++++++++
src/backend/commands/copyfromparse.c | 10 ++++++++++
src/bin/psql/tab-complete.c | 2 +-
src/include/commands/copy.h | 1 +
src/test/regress/expected/copy2.out | 6 +++++-
src/test/regress/sql/copy2.sql | 2 +-
7 files changed, 45 insertions(+), 5 deletions(-)
diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml
index 55764fc1f2..d0e58c0f9f 100644
--- a/doc/src/sgml/ref/copy.sgml
+++ b/doc/src/sgml/ref/copy.sgml
@@ -44,6 +44,7 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
FORCE_NOT_NULL { ( <replaceable class="parameter">column_name</replaceable> [, ...] ) | * }
FORCE_NULL { ( <replaceable class="parameter">column_name</replaceable> [, ...] ) | * }
ON_ERROR '<replaceable class="parameter">error_action</replaceable>'
+ LOG_VERBOSITY [ <replaceable class="parameter">boolean</replaceable> ]
ENCODING '<replaceable class="parameter">encoding_name</replaceable>'
</synopsis>
</refsynopsisdiv>
@@ -397,8 +398,12 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
when the <literal>FORMAT</literal> is <literal>text</literal> or <literal>csv</literal>.
</para>
<para>
- A <literal>NOTICE</literal> message containing the ignored row count is emitted at the end
- of the <command>COPY FROM</command> if at least one row was discarded.
+ A <literal>NOTICE</literal> message containing the ignored row count is
+ emitted at the end of the <command>COPY FROM</command> if at least one
+ row was discarded. When <literal>LOG_VERBOSITY</literal> option is set to
+ <literal>true</literal> (or equivalent Boolean value), a
+ <literal>NOTICE</literal> message containing the line number and column
+ name for each discarded row is emitted.
</para>
</listitem>
</varlistentry>
@@ -415,6 +420,18 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>LOG_VERBOSITY</literal></term>
+ <listitem>
+ <para>
+ Sets the verbosity of logged messages by <command>COPY</command>
+ command. As an example, see its usage for
+ <command>COPY FROM</command> command's <literal>ON_ERROR</literal>
+ clause with <literal>ignore</literal> option.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><literal>WHERE</literal></term>
<listitem>
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 056b6733c8..aa9fee5a71 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -454,6 +454,7 @@ ProcessCopyOptions(ParseState *pstate,
bool freeze_specified = false;
bool header_specified = false;
bool on_error_specified = false;
+ bool log_verbosity_specified = false;
ListCell *option;
/* Support external use for option sanity checking */
@@ -613,6 +614,13 @@ ProcessCopyOptions(ParseState *pstate,
on_error_specified = true;
opts_out->on_error = defGetCopyOnErrorChoice(defel, pstate, is_from);
}
+ else if (strcmp(defel->defname, "log_verbosity") == 0)
+ {
+ if (log_verbosity_specified)
+ errorConflictingDefElem(defel, pstate);
+ log_verbosity_specified = true;
+ opts_out->log_verbosity = defGetBoolean(defel);
+ }
else
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 5682d5d054..5f6be5c400 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -967,7 +967,17 @@ NextCopyFrom(CopyFromState cstate, ExprContext *econtext,
(Node *) cstate->escontext,
&values[m]))
{
+ Assert(cstate->opts.on_error != COPY_ON_ERROR_STOP);
+
cstate->num_errors++;
+
+ if (cstate->opts.log_verbosity)
+ ereport(NOTICE,
+ errmsg("detected data type incompatibility at line number %llu for column %s; COPY %s",
+ (unsigned long long) cstate->cur_lineno,
+ cstate->cur_attname,
+ cstate->cur_relname));
+
return true;
}
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index aa1acf8523..b6d5767acc 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -2900,7 +2900,7 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH("FORMAT", "FREEZE", "DELIMITER", "NULL",
"HEADER", "QUOTE", "ESCAPE", "FORCE_QUOTE",
"FORCE_NOT_NULL", "FORCE_NULL", "ENCODING", "DEFAULT",
- "ON_ERROR");
+ "ON_ERROR", "LOG_VERBOSITY");
/* Complete COPY <sth> FROM|TO filename WITH (FORMAT */
else if (Matches("COPY|\\copy", MatchAny, "FROM|TO", MatchAny, "WITH", "(", "FORMAT"))
diff --git a/src/include/commands/copy.h b/src/include/commands/copy.h
index b3da3cb0be..e194081fad 100644
--- a/src/include/commands/copy.h
+++ b/src/include/commands/copy.h
@@ -73,6 +73,7 @@ typedef struct CopyFormatOptions
bool *force_null_flags; /* per-column CSV FN flags */
bool convert_selectively; /* do selective binary conversion? */
CopyOnErrorChoice on_error; /* what to do when error happened */
+ bool log_verbosity; /* log more verbose messages? */
List *convert_select; /* list of column names (can be NIL) */
} CopyFormatOptions;
diff --git a/src/test/regress/expected/copy2.out b/src/test/regress/expected/copy2.out
index 25c401ce34..07e52bcd4a 100644
--- a/src/test/regress/expected/copy2.out
+++ b/src/test/regress/expected/copy2.out
@@ -729,7 +729,11 @@ CREATE TABLE check_ign_err (n int, m int[], k int);
COPY check_ign_err FROM STDIN WITH (on_error stop);
ERROR: invalid input syntax for type integer: "a"
CONTEXT: COPY check_ign_err, line 2, column n: "a"
-COPY check_ign_err FROM STDIN WITH (on_error ignore);
+COPY check_ign_err FROM STDIN WITH (on_error ignore, log_verbosity on);
+NOTICE: detected data type incompatibility at line number 2 for column n; COPY check_ign_err
+NOTICE: detected data type incompatibility at line number 3 for column k; COPY check_ign_err
+NOTICE: detected data type incompatibility at line number 4 for column m; COPY check_ign_err
+NOTICE: detected data type incompatibility at line number 5 for column n; COPY check_ign_err
NOTICE: 4 rows were skipped due to data type incompatibility
SELECT * FROM check_ign_err;
n | m | k
diff --git a/src/test/regress/sql/copy2.sql b/src/test/regress/sql/copy2.sql
index b5e549e856..47d131c1ce 100644
--- a/src/test/regress/sql/copy2.sql
+++ b/src/test/regress/sql/copy2.sql
@@ -508,7 +508,7 @@ a {2} 2
5 {5} 5
\.
-COPY check_ign_err FROM STDIN WITH (on_error ignore);
+COPY check_ign_err FROM STDIN WITH (on_error ignore, log_verbosity on);
1 {1} 1
a {2} 2
3 {3} 3333333333
--
2.34.1
