On Mon, Jul 15, 2024 at 1:42 PM Nishant Sharma
<nishant.sha...@enterprisedb.com> wrote:
>
> Thanks for the patch!
>
> I was not able to understand why we need a special error table for COPY?
> Can't we just log it in a new log error file for COPY in a proper format? Like
> using table approach, PG would be unnecessarily be utilising its resources
> like
> extra CPU to format the data in pages to store in its table format, writing
> and
> keeping the table in its store (which may or may not be required), the user
> would be required to make sure it creates the error table with proper columns
> to be used in COPY, etc..
>
>
> Meanwhile, please find some quick review comments:-
>
> 1) Patch needs re-base.
>
> 2) If the columns of the error table are fixed, then why not create it
> internally using
> some function or something instead of making the user create the table
> correctly
> with all the columns?
I'll think about it more.
previously, i tried to use SPI to create tables, but at that time, i
thought that's kind of excessive.
you need to create the table, check whether the table name is unique,
check the privilege.
now we quickly error out if the error saving table definition does not
meet. I guess that's less work to do.
> 3) I think, error messages can be improved, which looks big to me.
>
> 4) I think no need of below pstrdup, As CStringGetTextDatum creates a text
> copy for
> the same:-
> err_code =
> pstrdup(unpack_sql_state(cstate->escontext->error_data->sqlerrcode));
>
> t_values[9] = CStringGetTextDatum(err_code);
>
> 5) Should 'on_error_rel' as not NULL be checked along with below, as I can
> see it is
> passed as NULL from two locations?
> + if (cstate->opts.on_error == COPY_ON_ERROR_TABLE)
> + {
>
> 6) Below declarations can be shifted to the if block, where they are used.
> Instead of
> keeping them as global in function?
> + HeapTuple on_error_tup;
> + TupleDesc on_error_tupDesc;
>
> + if (cstate->opts.on_error == COPY_ON_ERROR_TABLE)
> + {
>
> 7) Below comment going beyond 80 char width:-
> * if on_error is specified with 'table', then on_error_rel is the error
> saving table
>
> 8) Need space after 'false'
> err_detail ? false: true;
>
> 9) Below call can fit in a single line. No need to keep the 2nd and 3rd
> parameter in
> nextlines.
> + on_error_tup = heap_form_tuple(on_error_tupDesc,
> + t_values,
> + t_isnull);
>
> 10) Variable declarations Tab Spacing issue at multiple places.
>
> 11) Comments in the patch are not matched to PG comment style.
>
>
> Kindly note I have not tested the patch properly yet. Only checked it with a
> positive test
> case. As I will wait for a conclusion on my opinion of the proposed patch.
>
almost all these issues have been addressed.
The error messages are still not so good. I need to polish it.
From f344314bfab06dfba3fd60c7cef16dd2d4d41803 Mon Sep 17 00:00:00 2001
From: jian he <jian.universal...@gmail.com>
Date: Mon, 19 Aug 2024 21:58:46 +0800
Subject: [PATCH v2 1/1] new on_error option: "table" saving error info to
table
introduce on_error table option for COPY FROM.
the syntax is {on_error table, table 'error_saving_tbl'}.
we first check table error_saving_tbl's existence and data definition.
if it does not meet our criteria, then we quickly abort the COPY operation.
we also did preliminary check the lock of error saving table
so the COPY can insert tuples to it.
once there is a error happened, we save the error metedata
and insert it to the error_saving_table.
discussion: https://postgr.es/m/CACJufxH_OJpVra%3D0c4ow8fbxHj7heMcVaTNEPa5vAurSeNA-6Q%40mail.gmail.com
---
doc/src/sgml/ref/copy.sgml | 111 +++++++++++++++++-
src/backend/commands/copy.c | 20 ++++
src/backend/commands/copyfrom.c | 139 +++++++++++++++++++++++
src/backend/commands/copyfromparse.c | 46 ++++++++
src/backend/parser/gram.y | 4 +
src/include/commands/copy.h | 2 +
src/include/commands/copyfrom_internal.h | 1 +
src/test/regress/expected/copy2.out | 96 ++++++++++++++++
src/test/regress/sql/copy2.sql | 83 ++++++++++++++
9 files changed, 501 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml
index 1518af8a04..80c8a1be41 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>
+ TABLE '<replaceable class="parameter">error_saving_tbl</replaceable>'
ENCODING '<replaceable class="parameter">encoding_name</replaceable>'
LOG_VERBOSITY <replaceable class="parameter">verbosity</replaceable>
</synopsis>
@@ -393,7 +394,9 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
input value into its data type.
An <replaceable class="parameter">error_action</replaceable> value of
<literal>stop</literal> means fail the command, while
- <literal>ignore</literal> means discard the input row and continue with the next one.
+ <literal>ignore</literal> means discard the input row and continue with the next one,
+ <literal>TABLE</literal> means <command>COPY</command> skips malformed data and continues copying data,
+ it aslo insert error related information to <replaceable class="parameter">error_saving_tbl</replaceable>.
The default is <literal>stop</literal>.
</para>
<para>
@@ -439,6 +442,112 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>TABLE</literal></term>
+ <listitem>
+ <para>
+ Save error context information to table <replaceable class="parameter">error_saving_table</replaceable>
+ when encountering errors converting a column's input value into its data type in <command>COPY FROM</command> operation.
+ <replaceable class="parameter">error_saving_table</replaceable> is the error saving table name,
+ it must exist in the current database, also its schema must be the same as the schema of
+ <command>COPY FROM</command> destination table.
+ This option is allowed only in <command>COPY FROM</command> and
+ <literal>ON_ERROR</literal> is specified with <literal>TABLE</literal>.
+ The <command>COPY FROM</command> user requires <literal>INSERT</literal> privileges for
+ every column in <replaceable class="parameter">error_saving_table</replaceable>.
+ If this option is omitted, the <literal>ON_ERROR</literal> parameter must not be specified as <literal>TABLE</literal>.
+</para>
+ <para>
+ If table <replaceable class="parameter">error_saving_table</replaceable> does meet the following definition
+ (column ordinal position should be the same as the below table), an error will be raised.
+
+<informaltable>
+ <tgroup cols="3">
+ <thead>
+ <row>
+ <entry>Column name</entry>
+ <entry>Data type</entry>
+ <entry>Description</entry>
+ </row>
+ </thead>
+
+ <tbody>
+ <row>
+ <entry> <literal>userid</literal> </entry>
+ <entry><type>oid</type></entry>
+ <entry>The user generated the error.
+ Reference <link linkend="catalog-pg-authid"><structname>pg_authid</structname></link>.<structfield>oid</structfield>,
+ however there is no hard dependency with catalog <literal>pg_authid</literal>.
+ If the corresponding row on <literal>pg_authid</literal> is deleted, this value becomes stale.
+ </entry>
+ </row>
+
+ <row>
+ <entry> <literal>copy_tbl</literal> </entry>
+ <entry><type>oid</type></entry>
+ <entry>The <command>COPY FROM</command> operation destination table oid.
+ Reference <link linkend="catalog-pg-class"><structname>pg_class</structname></link>.<structfield>oid</structfield>,
+ however there is no hard dependency with catalog <literal>pg_class</literal>.
+ If the corresponding row on <literal>pg_class</literal> is deleted, this value becomes stale.
+ </entry>
+ </row>
+
+ <row>
+ <entry> <literal>filename</literal> </entry>
+ <entry><type>text</type></entry>
+ <entry>The path name of the <command>COPY FROM</command> input</entry>
+ </row>
+
+ <row>
+ <entry> <literal>lineno</literal> </entry>
+ <entry><type>bigint</type></entry>
+ <entry>Line number where the error occurred, counting from 1</entry>
+ </row>
+
+ <row>
+ <entry> <literal>line</literal> </entry>
+ <entry><type>text</type></entry>
+ <entry>Raw content of the error occurred line</entry>
+ </row>
+
+ <row>
+ <entry> <literal>colname</literal> </entry>
+ <entry><type>text</type></entry>
+ <entry>Field where the error occurred</entry>
+ </row>
+
+ <row>
+ <entry> <literal>raw_field_value</literal> </entry>
+ <entry><type>text</type></entry>
+ <entry>Raw content of the error occurred field</entry>
+ </row>
+
+ <row>
+ <entry> <literal>err_message </literal> </entry>
+ <entry><type>text</type></entry>
+ <entry>The error message</entry>
+ </row>
+
+ <row>
+ <entry> <literal>err_detail</literal> </entry>
+ <entry><type>text</type></entry>
+ <entry>Detailed error message </entry>
+ </row>
+
+ <row>
+ <entry> <literal>errorcode </literal> </entry>
+ <entry><type>text</type></entry>
+ <entry>The error code </entry>
+ </row>
+
+ </tbody>
+ </tgroup>
+ </informaltable>
+
+ </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 3bb579a3a4..50512085fc 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -410,6 +410,8 @@ defGetCopyOnErrorChoice(DefElem *def, ParseState *pstate, bool is_from)
if (pg_strcasecmp(sval, "ignore") == 0)
return COPY_ON_ERROR_IGNORE;
+ if (pg_strcasecmp(sval, "table") == 0)
+ return COPY_ON_ERROR_TABLE;
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
/*- translator: first %s is the name of a COPY option, e.g. ON_ERROR */
@@ -469,6 +471,7 @@ ProcessCopyOptions(ParseState *pstate,
bool freeze_specified = false;
bool header_specified = false;
bool on_error_specified = false;
+ bool on_error_tbl_specified = false;
bool log_verbosity_specified = false;
ListCell *option;
@@ -636,6 +639,14 @@ ProcessCopyOptions(ParseState *pstate,
log_verbosity_specified = true;
opts_out->log_verbosity = defGetCopyLogVerbosityChoice(defel, pstate);
}
+ else if (strcmp(defel->defname, "table") == 0)
+ {
+ if (on_error_tbl_specified)
+ errorConflictingDefElem(defel, pstate);
+ on_error_tbl_specified = true;
+
+ opts_out->on_error_tbl = defGetString(defel);
+ }
else
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
@@ -644,6 +655,15 @@ ProcessCopyOptions(ParseState *pstate,
parser_errposition(pstate, defel->location)));
}
+ if (opts_out->on_error_tbl == NULL && opts_out->on_error == COPY_ON_ERROR_TABLE)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("COPY %s \"table\" requires a custom specified error saving table", "ON_ERROR")));
+
+ if (opts_out->on_error_tbl != NULL && opts_out->on_error != COPY_ON_ERROR_TABLE)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("COPY \"table\" option can only be used when %s option is specified as \"table\"", "ON_ERROR")));
/*
* Check for incompatible options (must do these two before inserting
* defaults)
diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c
index 2d3462913e..5fa0801c1f 100644
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -35,12 +35,14 @@
#include "executor/execPartition.h"
#include "executor/executor.h"
#include "executor/nodeModifyTable.h"
+#include "executor/spi.h"
#include "executor/tuptable.h"
#include "foreign/fdwapi.h"
#include "mb/pg_wchar.h"
#include "miscadmin.h"
#include "nodes/miscnodes.h"
#include "optimizer/optimizer.h"
+#include "parser/parse_relation.h"
#include "pgstat.h"
#include "rewrite/rewriteHandler.h"
#include "storage/fd.h"
@@ -1024,6 +1026,12 @@ CopyFrom(CopyFromState cstate)
pgstat_progress_update_param(PROGRESS_COPY_TUPLES_SKIPPED,
++skipped);
+ if (cstate->opts.on_error == COPY_ON_ERROR_TABLE)
+ {
+ cstate->escontext->error_occurred = false;
+ cstate->escontext->details_wanted = true;
+ memset(cstate->escontext->error_data,0, sizeof(ErrorData));
+ }
continue;
}
@@ -1326,6 +1334,13 @@ CopyFrom(CopyFromState cstate)
"%llu rows were skipped due to data type incompatibility",
(unsigned long long) cstate->num_errors,
(unsigned long long) cstate->num_errors));
+ /*
+ * similar to commit a9cf48a
+ * (https://postgr.es/m/7bcfc39d4176faf85ab317d0c26786953646a411.ca...@cybertec.at)
+ * in COPY FROM keep error saving table locks until the transaction end.
+ */
+ if (cstate->error_saving_rel != NULL)
+ table_close(cstate->error_saving_rel, NoLock);
if (bistate != NULL)
FreeBulkInsertState(bistate);
@@ -1480,6 +1495,130 @@ BeginCopyFrom(ParseState *pstate,
else
cstate->escontext = NULL;
+ if (cstate->opts.on_error == COPY_ON_ERROR_TABLE)
+ {
+ StringInfoData querybuf;
+ const char *copy_nspname;
+ Oid err_tbl_oid;
+ Oid copy_nspoid;
+ bool on_error_tbl_ok;
+ bool allow_insert_error_tbl;
+ bool isnull;
+
+ Assert(cstate->escontext != NULL);
+ Assert(cstate->opts.on_error_tbl != NULL);
+
+ /* get copy error saving table reloid */
+ copy_nspname = get_namespace_name(RelationGetNamespace(cstate->rel));
+ copy_nspoid = get_namespace_oid(copy_nspname, false);
+ err_tbl_oid = get_relname_relid(cstate->opts.on_error_tbl, copy_nspoid);
+
+ if (!OidIsValid(err_tbl_oid))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("error saving table \"%s\".\"%s\" does not exist",
+ copy_nspname, cstate->opts.on_error_tbl)));
+
+ /* error saving table must be a normal realtion kind */
+ if (get_rel_relkind(err_tbl_oid) != RELKIND_RELATION)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("COPY %s cannot use relation \"%s\" for error saving",
+ "ON_ERROR", cstate->opts.on_error_tbl),
+ errdetail_relkind_not_supported(get_rel_relkind(err_tbl_oid))));
+
+ /*
+ * we may insert tuples to error-saving table, to do that we need first
+ * check the table lock situation. If it is already under heavy lock,
+ * then our COPY operation would be stuck. instead of let COPY stuck,
+ * just error report that the table is in heavy lock.
+ */
+ initStringInfo(&querybuf);
+ appendStringInfo(&querybuf,
+ "select 1 as exists from ( "
+ "select 1 "
+ "from pg_class, pg_locks "
+ "where pg_class.oid = pg_locks.relation "
+ "and pg_class.relnamespace = %d "
+ "and pg_class.oid = %d "
+ "and mode not in ('AccessShareLock', 'RowShareLock', 'RowExclusiveLock')); "
+ ,copy_nspoid, err_tbl_oid);
+
+ if (SPI_connect() != SPI_OK_CONNECT)
+ elog(ERROR, "SPI_connect failed");
+
+ if (SPI_execute(querybuf.data, false, 0) != SPI_OK_SELECT)
+ elog(ERROR, "SPI_exec failed: %s", querybuf.data);
+
+ if (SPI_processed != 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("table \"%s\".\"%s\" was locked, cannot be used for error saving",
+ copy_nspname, cstate->opts.on_error_tbl)));
+
+ /* current user should have INSERT priviledge on error_saving table */
+ resetStringInfo(&querybuf);
+ appendStringInfo(&querybuf,
+ "SELECT has_table_privilege (CURRENT_USER, %d, 'INSERT')",
+ err_tbl_oid);
+ if (SPI_execute(querybuf.data, false, 0) != SPI_OK_SELECT)
+ elog(ERROR, "SPI_exec failed: %s", querybuf.data);
+
+ allow_insert_error_tbl = DatumGetBool(SPI_getbinval(SPI_tuptable->vals[0],
+ SPI_tuptable->tupdesc,
+ 1, &isnull));
+ if (!allow_insert_error_tbl)
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("permission denied to set table \"%s\".\"%s\" for COPY FROM error saving",
+ copy_nspname, cstate->opts.on_error_tbl),
+ errhint("Ensure current user have enough priviledge on \"%s\".\"%s\" for COPY FROM error saving",
+ copy_nspname, cstate->opts.on_error_tbl)));
+
+ resetStringInfo(&querybuf);
+ /*
+ * Check the error saving table's data definition (column name,
+ * data types) can be used for error saving or not.
+ *
+ */
+ appendStringInfo(&querybuf,
+ "SELECT (array_agg(pa.attname ORDER BY pa.attnum) "
+ "= '{ctid,userid,copy_tbl,filename,lineno, "
+ "line,colname,raw_field_value,err_message,err_detail,errorcode}') "
+ "AND (ARRAY_AGG(pt.typname ORDER BY pa.attnum) "
+ "= '{tid,oid,oid,text,int8,text,text,text,text,text,text}') "
+ "FROM pg_catalog.pg_attribute pa "
+ "JOIN pg_catalog.pg_class pc ON pc.oid = pa.attrelid "
+ "JOIN pg_catalog.pg_type pt ON pt.oid = pa.atttypid "
+ "JOIN pg_catalog.pg_namespace pn "
+ "ON pn.oid = pc.relnamespace WHERE ");
+ appendStringInfo(&querybuf,
+ "pn.nspname = $$%s$$ AND relname = $$%s$$ "
+ "AND pa.attnum >= -1 AND NOT attisdropped ",
+ copy_nspname, cstate->opts.on_error_tbl);
+
+ if (SPI_execute(querybuf.data, false, 0) != SPI_OK_SELECT)
+ elog(ERROR, "SPI_exec failed: %s", querybuf.data);
+
+ on_error_tbl_ok = DatumGetBool(SPI_getbinval(SPI_tuptable->vals[0],
+ SPI_tuptable->tupdesc,
+ 1, &isnull));
+ if(!on_error_tbl_ok)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("table \"%s\".\"%s\" cannot be used for COPY FROM error saving",
+ copy_nspname, cstate->opts.on_error_tbl),
+ errdetail("Table \"%s\".\"%s\" data definition cannot be used for error saving",
+ copy_nspname, cstate->opts.on_error_tbl)));
+
+ if (SPI_finish() != SPI_OK_FINISH)
+ elog(ERROR, "SPI_finish failed");
+
+ /* now error-saving table is ok for error saving, take a lock for insert*/
+ cstate->error_saving_rel = table_open(err_tbl_oid, RowExclusiveLock);
+ cstate->escontext->details_wanted = true;
+ }
+
/* Convert FORCE_NULL name list to per-column flags, check validity */
cstate->opts.force_null_flags = (bool *) palloc0(num_phys_attrs * sizeof(bool));
if (cstate->opts.force_null_all)
diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 7efcb89159..e640259347 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -66,6 +66,7 @@
#include "commands/copyfrom_internal.h"
#include "commands/progress.h"
#include "executor/executor.h"
+#include "access/heapam.h"
#include "libpq/libpq.h"
#include "libpq/pqformat.h"
#include "mb/pg_wchar.h"
@@ -969,6 +970,51 @@ NextCopyFrom(CopyFromState cstate, ExprContext *econtext,
{
Assert(cstate->opts.on_error != COPY_ON_ERROR_STOP);
+ if (cstate->opts.on_error == COPY_ON_ERROR_TABLE)
+ {
+ /*
+ * we mostly use ErrorSaveContext's info to form a tuple and
+ * insert it to the error saving table. we already did lock
+ * condition check in BeginCopyFrom.
+ */
+ #define ERROR_TBL_COLUMNS 10
+ HeapTuple on_error_tup;
+ TupleDesc on_error_tupDesc;
+ char *err_detail;
+ char *err_code;
+ Datum t_values[ERROR_TBL_COLUMNS] = {0};
+ bool t_isnull[ERROR_TBL_COLUMNS] = {0};
+ int j = 0;
+
+ Assert(cstate->rel != NULL);
+ t_values[j++] = ObjectIdGetDatum(GetUserId());
+ t_values[j++] = ObjectIdGetDatum(cstate->rel->rd_rel->oid);
+ t_values[j++] = CStringGetTextDatum(cstate->filename ? cstate->filename : "STDIN");
+ t_values[j++] = Int64GetDatum((long long) cstate->cur_lineno);
+ t_values[j++] = CStringGetTextDatum(cstate->line_buf.data);
+ t_values[j++] = CStringGetTextDatum(cstate->cur_attname);
+ t_values[j++] = CStringGetTextDatum(string);
+ t_values[j++] = CStringGetTextDatum(cstate->escontext->error_data->message);
+
+ if (!cstate->escontext->error_data->detail)
+ err_detail = NULL;
+ else
+ err_detail = cstate->escontext->error_data->detail;
+ t_values[j] = err_detail ? CStringGetTextDatum(err_detail) : (Datum) 0;
+ t_isnull[j++] = err_detail ? false : true;
+
+ err_code = unpack_sql_state(cstate->escontext->error_data->sqlerrcode);
+ t_values[j++] = CStringGetTextDatum(err_code);
+
+ Assert(j == ERROR_TBL_COLUMNS);
+ #undef ERROR_TBL_COLUMNS
+
+ on_error_tupDesc = cstate->error_saving_rel->rd_att;
+ on_error_tup = heap_form_tuple(on_error_tupDesc, t_values, t_isnull);
+ simple_heap_insert(cstate->error_saving_rel, on_error_tup);
+ cstate->num_errors++;
+ return true;
+ }
cstate->num_errors++;
if (cstate->opts.log_verbosity == COPY_LOG_VERBOSITY_VERBOSE)
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index c3f25582c3..cd1e8c5403 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -3496,6 +3496,10 @@ copy_opt_item:
{
$$ = makeDefElem("null", (Node *) makeString($3), @1);
}
+ | TABLE opt_as Sconst
+ {
+ $$ = makeDefElem("table", (Node *) makeString($3), @1);
+ }
| CSV
{
$$ = makeDefElem("format", (Node *) makeString("csv"), @1);
diff --git a/src/include/commands/copy.h b/src/include/commands/copy.h
index 141fd48dc1..6871f27a83 100644
--- a/src/include/commands/copy.h
+++ b/src/include/commands/copy.h
@@ -38,6 +38,7 @@ typedef enum CopyOnErrorChoice
{
COPY_ON_ERROR_STOP = 0, /* immediately throw errors, default */
COPY_ON_ERROR_IGNORE, /* ignore errors */
+ COPY_ON_ERROR_TABLE, /* saving errors info to table */
} CopyOnErrorChoice;
/*
@@ -83,6 +84,7 @@ typedef struct CopyFormatOptions
bool convert_selectively; /* do selective binary conversion? */
CopyOnErrorChoice on_error; /* what to do when error happened */
CopyLogVerbosityChoice log_verbosity; /* verbosity of logged messages */
+ char *on_error_tbl; /* on error, save error info to table */
List *convert_select; /* list of column names (can be NIL) */
} CopyFormatOptions;
diff --git a/src/include/commands/copyfrom_internal.h b/src/include/commands/copyfrom_internal.h
index cad52fcc78..779f86d1ce 100644
--- a/src/include/commands/copyfrom_internal.h
+++ b/src/include/commands/copyfrom_internal.h
@@ -70,6 +70,7 @@ typedef struct CopyFromStateData
/* parameters from the COPY command */
Relation rel; /* relation to copy from */
+ Relation error_saving_rel; /* relation for copy from error saving */
List *attnumlist; /* integer list of attnums to copy */
char *filename; /* filename, or NULL for STDIN */
bool is_program; /* is 'filename' a program to popen? */
diff --git a/src/test/regress/expected/copy2.out b/src/test/regress/expected/copy2.out
index e913f683a6..278fe5a6fd 100644
--- a/src/test/regress/expected/copy2.out
+++ b/src/test/regress/expected/copy2.out
@@ -789,6 +789,99 @@ CONTEXT: COPY check_ign_err, line 1: "1 {1}"
COPY check_ign_err FROM STDIN WITH (on_error ignore);
ERROR: extra data after last expected column
CONTEXT: COPY check_ign_err, line 1: "1 {1} 3 abc"
+create table err_tbl(
+ userid oid, -- the user oid while copy generated this entry
+ copy_tbl oid, --copy table
+ filename text,
+ lineno bigint,
+ line text,
+ colname text,
+ raw_field_value text,
+ err_message text,
+ err_detail text,
+ errorcode text
+);
+create table err_tbl_1(
+ userid oid, -- the user oid while copy generated this entry
+ copy_tbl oid, --copy table
+ filename text,
+ lineno bigint,
+ line text,
+ colname text,
+ raw_field_value text,
+ err_message text,
+ err_detail text
+);
+create table t_copy_tbl(a int, b int, c int);
+--the folowing should all fails
+COPY t_copy_tbl FROM STDIN WITH (on_error 'table');
+ERROR: COPY ON_ERROR "table" requires a custom specified error saving table
+COPY t_copy_tbl TO STDIN WITH (on_error 'table');
+ERROR: COPY ON_ERROR cannot be used with COPY TO
+LINE 1: COPY t_copy_tbl TO STDIN WITH (on_error 'table');
+ ^
+COPY t_copy_tbl FROM STDIN WITH (table err_tbl);
+ERROR: COPY "table" option can only be used when ON_ERROR option is specified as "table"
+COPY t_copy_tbl TO STDIN WITH (table err_tbl);
+ERROR: COPY "table" option can only be used when ON_ERROR option is specified as "table"
+COPY t_copy_tbl(a,b) FROM STDIN WITH (on_error 'table', table not_exists);
+ERROR: error saving table "public"."not_exists" does not exist
+create view s1 as select 1 as a;
+COPY t_copy_tbl(a) FROM STDIN WITH (on_error 'table', table s1);
+ERROR: COPY ON_ERROR cannot use relation "s1" for error saving
+DETAIL: This operation is not supported for views.
+drop view s1;
+--should fail. err_tbl_1 does not meet criteria
+COPY t_copy_tbl(a,b) FROM STDIN WITH (on_error 'table', table err_tbl_1);
+ERROR: table "public"."err_tbl_1" cannot be used for COPY FROM error saving
+DETAIL: Table "public"."err_tbl_1" data definition cannot be used for error saving
+--should fail, extra columns
+COPY t_copy_tbl(a,b) FROM STDIN WITH (delimiter ',', on_error 'table', table err_tbl);
+ERROR: extra data after last expected column
+CONTEXT: COPY t_copy_tbl, line 1: "1,2,3,4"
+--should fail, less columns
+COPY t_copy_tbl(a,b) FROM STDIN WITH (delimiter ',', on_error 'table', table err_tbl);
+ERROR: extra data after last expected column
+CONTEXT: COPY t_copy_tbl, line 1: "1,2,"
+--ok cases.
+COPY t_copy_tbl FROM STDIN WITH (delimiter ',', on_error 'table', table err_tbl);
+NOTICE: 4 rows were skipped due to data type incompatibility
+--should fail. lack priviledge
+begin;
+create user regress_user20;
+grant insert(userid,copy_tbl,filename,lineno,line) on table err_tbl to regress_user20;
+grant insert on table t_copy_tbl to regress_user20;
+set role regress_user20;
+COPY t_copy_tbl FROM STDIN WITH (delimiter ',', on_error 'table', table err_tbl);
+ERROR: permission denied to set table "public"."err_tbl" for COPY FROM error saving
+HINT: Ensure current user have enough priviledge on "public"."err_tbl" for COPY FROM error saving
+ROLLBACK;
+select pg_class.relname as copy_destination
+ ,filename
+ ,lineno
+ ,line
+ ,colname
+ ,raw_field_value
+ ,err_message
+ ,err_detail
+ ,errorcode
+from err_tbl join pg_class on copy_tbl = pg_class.oid;
+ copy_destination | filename | lineno | line | colname | raw_field_value | err_message | err_detail | errorcode
+------------------+----------+--------+--------------------------+---------+---------------------+--------------------------------------------------------------+------------+-----------
+ t_copy_tbl | STDIN | 1 | 1,2,a | c | a | invalid input syntax for type integer: "a" | | 22P02
+ t_copy_tbl | STDIN | 3 | 1,_junk,test | b | _junk | invalid input syntax for type integer: "_junk" | | 22P02
+ t_copy_tbl | STDIN | 4 | cola,colb,colc | a | cola | invalid input syntax for type integer: "cola" | | 22P02
+ t_copy_tbl | STDIN | 7 | 1,11,4238679732489879879 | c | 4238679732489879879 | value "4238679732489879879" is out of range for type integer | | 22003
+(4 rows)
+
+select * from t_copy_tbl;
+ a | b | c
+---+---+----
+ 1 | 2 | 3
+ 4 | 5 | 6
+ 8 | 9 | 10
+(3 rows)
+
-- clean up
DROP TABLE forcetest;
DROP TABLE vistest;
@@ -807,6 +900,9 @@ DROP TABLE check_ign_err;
DROP TABLE check_ign_err2;
DROP DOMAIN dcheck_ign_err2;
DROP TABLE hard_err;
+DROP TABLE err_tbl;
+DROP TABLE err_tbl_1;
+DROP TABLE t_copy_tbl;
--
-- COPY FROM ... DEFAULT
--
diff --git a/src/test/regress/sql/copy2.sql b/src/test/regress/sql/copy2.sql
index 8b14962194..792fde7647 100644
--- a/src/test/regress/sql/copy2.sql
+++ b/src/test/regress/sql/copy2.sql
@@ -557,6 +557,86 @@ COPY check_ign_err FROM STDIN WITH (on_error ignore);
1 {1} 3 abc
\.
+create table err_tbl(
+ userid oid, -- the user oid while copy generated this entry
+ copy_tbl oid, --copy table
+ filename text,
+ lineno bigint,
+ line text,
+ colname text,
+ raw_field_value text,
+ err_message text,
+ err_detail text,
+ errorcode text
+);
+create table err_tbl_1(
+ userid oid, -- the user oid while copy generated this entry
+ copy_tbl oid, --copy table
+ filename text,
+ lineno bigint,
+ line text,
+ colname text,
+ raw_field_value text,
+ err_message text,
+ err_detail text
+);
+create table t_copy_tbl(a int, b int, c int);
+
+--the folowing should all fails
+COPY t_copy_tbl FROM STDIN WITH (on_error 'table');
+COPY t_copy_tbl TO STDIN WITH (on_error 'table');
+COPY t_copy_tbl FROM STDIN WITH (table err_tbl);
+COPY t_copy_tbl TO STDIN WITH (table err_tbl);
+COPY t_copy_tbl(a,b) FROM STDIN WITH (on_error 'table', table not_exists);
+create view s1 as select 1 as a;
+COPY t_copy_tbl(a) FROM STDIN WITH (on_error 'table', table s1);
+drop view s1;
+
+--should fail. err_tbl_1 does not meet criteria
+COPY t_copy_tbl(a,b) FROM STDIN WITH (on_error 'table', table err_tbl_1);
+
+--should fail, extra columns
+COPY t_copy_tbl(a,b) FROM STDIN WITH (delimiter ',', on_error 'table', table err_tbl);
+1,2,3,4
+\.
+
+--should fail, less columns
+COPY t_copy_tbl(a,b) FROM STDIN WITH (delimiter ',', on_error 'table', table err_tbl);
+1,2,
+\.
+
+--ok cases.
+COPY t_copy_tbl FROM STDIN WITH (delimiter ',', on_error 'table', table err_tbl);
+1,2,a
+1,2,3
+1,_junk,test
+cola,colb,colc
+4,5,6
+8,9,10
+1,11,4238679732489879879
+\.
+
+--should fail. lack priviledge
+begin;
+create user regress_user20;
+grant insert(userid,copy_tbl,filename,lineno,line) on table err_tbl to regress_user20;
+grant insert on table t_copy_tbl to regress_user20;
+set role regress_user20;
+COPY t_copy_tbl FROM STDIN WITH (delimiter ',', on_error 'table', table err_tbl);
+ROLLBACK;
+
+select pg_class.relname as copy_destination
+ ,filename
+ ,lineno
+ ,line
+ ,colname
+ ,raw_field_value
+ ,err_message
+ ,err_detail
+ ,errorcode
+from err_tbl join pg_class on copy_tbl = pg_class.oid;
+
+select * from t_copy_tbl;
-- clean up
DROP TABLE forcetest;
DROP TABLE vistest;
@@ -575,6 +655,9 @@ DROP TABLE check_ign_err;
DROP TABLE check_ign_err2;
DROP DOMAIN dcheck_ign_err2;
DROP TABLE hard_err;
+DROP TABLE err_tbl;
+DROP TABLE err_tbl_1;
+DROP TABLE t_copy_tbl;
--
-- COPY FROM ... DEFAULT
--
2.34.1
