public inbox for [email protected]
help / color / mirror / Atom feedon_error table, saving error info to a table
13+ messages / 4 participants
[nested] [flat]
* on_error table, saving error info to a table
@ 2024-02-03 06:22 jian he <[email protected]>
2024-07-15 05:42 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
0 siblings, 1 reply; 13+ messages in thread
From: jian he @ 2024-02-03 06:22 UTC (permalink / raw)
To: PostgreSQL Hackers <[email protected]>
Hi.
I previously did some work in COPY FROM save error information to a table.
still based on this suggestion:
https://www.postgresql.org/message-id/752672.1699474336%40sss.pgh.pa.us
Now I refactored it.
the syntax:
ON_ERROR 'table', TABLE 'error_saving_tbl'
if ON_ERROR is not specified with 'table', TABLE is specified, then error.
if ON_ERROR is specified with 'table', TABLE is not specified or
error_saving_tbl does not exist, then error.
In BeginCopyFrom, we check the data definition of error_saving_table,
we also check if the user has INSERT privilege to error_saving_table
(all the columns).
We also did a preliminary check of the lock condition of error_saving_table.
if it does not meet these conditions, we quickly error out.
error_saving_table will be the same schema as the copy from table.
Because "table" is a keyword, I have to add the following changes to gram.y.
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -3420,6 +3420,10 @@ copy_opt_item:
{
$$ = makeDefElem("null", (Node *) makeString($3), @1);
}
+ | TABLE opt_as Sconst
+ {
+ $$ = makeDefElem("table", (Node *) makeString($3), @1);
+ }
since "table" is already a keyword, so there is no influence on the
parsing speed?
demo:
create table err_tbl(
userid oid, -- the user oid while copy generated this entry
copy_tbl oid, --copy table
filename text,
lineno int8,
line text,
colname text,
raw_field_value text,
err_message text,
err_detail text,
errorcode text
);
create table t_copy_tbl(a int, b int, c int);
COPY t_copy_tbl FROM STDIN WITH (delimiter ',', on_error 'table',
table err_tbl);
1,2,a
\.
table err_tbl \gx
-[ RECORD 1 ]---+-------------------------------------------
userid | 10
copy_tbl | 17920
filename | STDIN
lineno | 1
line | 1,2,a
colname | c
raw_field_value | a
err_message | invalid input syntax for type integer: "a"
err_detail |
errorcode | 22P02
Attachments:
[application/x-patch] v1-0001-on_error-table-saving-error-info-to-a-table.patch (30.2K, 2-v1-0001-on_error-table-saving-error-info-to-a-table.patch)
download | inline diff:
From bb6f263ef4d37c2871086db44dca217fa91f5080 Mon Sep 17 00:00:00 2001
From: jian he <[email protected]>
Date: Sat, 3 Feb 2024 11:13:14 +0800
Subject: [PATCH v1 1/1] on_error table, saving error info to a 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.
---
contrib/file_fdw/file_fdw.c | 4 +-
doc/src/sgml/ref/copy.sgml | 106 +++++++++++++++++++++-
src/backend/commands/copy.c | 26 ++++++
src/backend/commands/copyfrom.c | 129 ++++++++++++++++++++++++++-
src/backend/commands/copyfromparse.c | 52 ++++++++++-
src/backend/parser/gram.y | 4 +
src/include/commands/copy.h | 4 +-
src/test/regress/expected/copy2.out | 93 +++++++++++++++++++
src/test/regress/sql/copy2.sql | 80 +++++++++++++++++
9 files changed, 492 insertions(+), 6 deletions(-)
diff --git a/contrib/file_fdw/file_fdw.c b/contrib/file_fdw/file_fdw.c
index 249d82d3..1d536e9e 100644
--- a/contrib/file_fdw/file_fdw.c
+++ b/contrib/file_fdw/file_fdw.c
@@ -751,7 +751,7 @@ fileIterateForeignScan(ForeignScanState *node)
*/
oldcontext = MemoryContextSwitchTo(GetPerTupleMemoryContext(estate));
found = NextCopyFrom(festate->cstate, econtext,
- slot->tts_values, slot->tts_isnull);
+ slot->tts_values, slot->tts_isnull, NULL);
if (found)
ExecStoreVirtualTuple(slot);
@@ -1183,7 +1183,7 @@ file_acquire_sample_rows(Relation onerel, int elevel,
MemoryContextReset(tupcontext);
MemoryContextSwitchTo(tupcontext);
- found = NextCopyFrom(cstate, NULL, values, nulls);
+ found = NextCopyFrom(cstate, NULL, values, nulls, NULL);
MemoryContextSwitchTo(oldcontext);
diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml
index 21a5c4a0..ae13c3b6 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>'
</synopsis>
</refsynopsisdiv>
@@ -380,12 +381,14 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
<para>
Specifies which <replaceable class="parameter">
error_action</replaceable> to perform when there is malformed data in the input.
- Currently, only <literal>stop</literal> (default) and <literal>ignore</literal>
+ Currently, only <literal>stop</literal> (default) , <literal>ignore</literal>, <literal>table</literal>
values are supported.
If the <literal>stop</literal> value is specified,
<command>COPY</command> stops operation at the first error.
If the <literal>ignore</literal> value is specified,
<command>COPY</command> skips malformed data and continues copying data.
+ If the <literal>table</literal> value is specified,
+ <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 option is allowed only in <command>COPY FROM</command>.
Only <literal>stop</literal> value is allowed when
using <literal>binary</literal> format.
@@ -405,6 +408,107 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>table</literal></term>
+ <listitem>
+ <para>
+ Save error information to the table <replaceable class="parameter">error_saving_table</replaceable>
+ when it encounters an error while doing <command>COPY FROM</command> operation.
+ The table, <replaceable class="parameter">error_saving_table</replaceable>'s 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 ecah column in <replaceable class="parameter">error_saving_table</replaceable>.
+ If this option is omitted, the <literal>ON_ERROR</literal> parameter must not specified with <literal>table</literal>.
+</para>
+ <para>
+ If table <replaceable class="parameter">error_saving_table</replaceable> does not exists or meet the following definition (column order should also be the same), 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 depenedency with catalog <literal>pg_authid</literal>. If the correspond row on <literal>pg_authid</literal> deleted, this value become 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 depenedency with catalog <literal>pg_class</literal>. If the correspond row on <literal>pg_class</literal> deleted, this value become 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 cc0786c6..c7d24917 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -423,6 +423,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),
errmsg("COPY ON_ERROR \"%s\" not recognized", sval),
@@ -456,6 +458,7 @@ ProcessCopyOptions(ParseState *pstate,
bool freeze_specified = false;
bool header_specified = false;
bool on_error_specified = false;
+ bool on_error_tbl_specified = false;
ListCell *option;
/* Support external use for option sanity checking */
@@ -615,6 +618,20 @@ ProcessCopyOptions(ParseState *pstate,
on_error_specified = true;
opts_out->on_error = defGetCopyOnErrorChoice(defel, pstate, is_from);
}
+ 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);
+ if (!opts_out->on_error_tbl)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("argument to option \"%s\" must be a valid table name",
+ defel->defname),
+ parser_errposition(pstate, defel->location)));
+ }
else
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
@@ -623,6 +640,15 @@ ProcessCopyOptions(ParseState *pstate,
parser_errposition(pstate, defel->location)));
}
+ if (!on_error_tbl_specified && opts_out->on_error == COPY_ON_ERROR_TABLE)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("argument to option \"table\" can only applied when ON ERROR is specified")));
+
+ if (on_error_tbl_specified && opts_out->on_error != COPY_ON_ERROR_TABLE)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("argument to option \"table\" can only applied when ON ERROR is specified")));
/*
* 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 1fe70b91..b5678317 100644
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -37,6 +37,7 @@
#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 "libpq/libpq.h"
@@ -47,6 +48,7 @@
#include "pgstat.h"
#include "rewrite/rewriteHandler.h"
#include "storage/fd.h"
+#include "storage/lmgr.h"
#include "tcop/tcopprot.h"
#include "utils/lsyscache.h"
#include "utils/memutils.h"
@@ -655,11 +657,20 @@ CopyFrom(CopyFromState cstate)
bool has_instead_insert_row_trig;
bool leafpart_use_multi_insert = false;
+ /* the error inforamtion while doing NextCopyFrom saving table */
+ Relation on_error_rel = NULL;
+
Assert(cstate->rel);
Assert(list_length(cstate->range_table) == 1);
if (cstate->opts.on_error != COPY_ON_ERROR_STOP)
Assert(cstate->escontext);
+ if (cstate->opts.on_error == COPY_ON_ERROR_TABLE)
+ {
+ Assert(cstate->opts.on_error_tbl);
+ on_error_rel = table_open(RelnameGetRelid(cstate->opts.on_error_tbl), AccessShareLock);
+ table_close(on_error_rel, AccessShareLock);
+ }
/*
* The target must be a plain, foreign, or partitioned relation, or have
@@ -994,7 +1005,7 @@ CopyFrom(CopyFromState cstate)
ExecClearTuple(myslot);
/* Directly store the values/nulls array in the slot */
- if (!NextCopyFrom(cstate, econtext, myslot->tts_values, myslot->tts_isnull))
+ if (!NextCopyFrom(cstate, econtext, myslot->tts_values, myslot->tts_isnull, on_error_rel))
break;
if (cstate->opts.on_error != COPY_ON_ERROR_STOP &&
@@ -1017,6 +1028,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;
}
@@ -1472,6 +1489,116 @@ BeginCopyFrom(ParseState *pstate,
else
cstate->escontext = NULL;
+ if (cstate->opts.on_error == COPY_ON_ERROR_TABLE)
+ {
+ StringInfoData querybuf;
+ Oid err_tbl;
+ const char* copy_nspname;
+ Oid copy_nspoid;
+ bool on_error_tbl_ok;
+ bool isnull;
+
+ Assert(cstate->escontext != NULL);
+ Assert(cstate->opts.on_error_tbl != NULL);
+
+ /* COPY FROM destination table schemas should be same as the error_saving table */
+ copy_nspname = get_namespace_name(RelationGetNamespace(cstate->rel));
+ copy_nspoid = get_namespace_oid(copy_nspname, false);
+ err_tbl = get_relname_relid(cstate->opts.on_error_tbl, copy_nspoid);
+
+ if (!OidIsValid(err_tbl))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("error saving table \"%s\".\"%s\" does not exist",
+ copy_nspname, cstate->opts.on_error_tbl)));
+
+ /*
+ * we may need insert tuples to the error saving table, to do that we need
+ * first check the lock condition. If the table is already udner heavy locked,
+ * 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);
+
+ 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)));
+ SPI_processed = 0;
+ resetStringInfo(&querybuf);
+
+ /*
+ *
+ * Verify whether the error saving table already exists, and if so,
+ * examine it's column names and data types. current user requires
+ * INSERT previledge for each columns in error saving table
+ *
+ */
+ 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 "
+ "AND has_column_privilege (CURRENT_USER, $$%s$$, 1::smallint, 'INSERT') "
+ "AND has_column_privilege (CURRENT_USER, $$%s$$, 2::smallint, 'INSERT') "
+ "AND has_column_privilege (CURRENT_USER, $$%s$$, 3::smallint, 'INSERT') "
+ "AND has_column_privilege (CURRENT_USER, $$%s$$, 4::smallint, 'INSERT') "
+ "AND has_column_privilege (CURRENT_USER, $$%s$$, 5::smallint, 'INSERT') "
+ "AND has_column_privilege (CURRENT_USER, $$%s$$, 6::smallint, 'INSERT') "
+ "AND has_column_privilege (CURRENT_USER, $$%s$$, 7::smallint, 'INSERT') "
+ "AND has_column_privilege (CURRENT_USER, $$%s$$, 8::smallint, 'INSERT') "
+ "AND has_column_privilege (CURRENT_USER, $$%s$$, 9::smallint, 'INSERT') "
+ "AND has_column_privilege (CURRENT_USER, $$%s$$, 10::smallint, 'INSERT') ",
+ copy_nspname, cstate->opts.on_error_tbl,
+ cstate->opts.on_error_tbl, cstate->opts.on_error_tbl, cstate->opts.on_error_tbl,
+ cstate->opts.on_error_tbl, cstate->opts.on_error_tbl, cstate->opts.on_error_tbl,
+ cstate->opts.on_error_tbl, cstate->opts.on_error_tbl, cstate->opts.on_error_tbl,
+ 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("The table (\"%s\".\"%s\") data definition cannot be used for error saving or current user don't enough priviledge on it",
+ copy_nspname, cstate->opts.on_error_tbl),
+ errhint("Ensure current user have enough priviledge on \"%s\".\"%s\", also ensure the data definition can be used for error saving",
+ copy_nspname, cstate->opts.on_error_tbl)));
+
+ if (SPI_finish() != SPI_OK_FINISH)
+ elog(ERROR, "SPI_finish failed");
+ 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 7cacd0b7..157017cf 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"
@@ -850,10 +851,11 @@ NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields)
*
* 'values' and 'nulls' arrays must be the same length as columns of the
* relation passed to BeginCopyFrom. This function fills the arrays.
+ * if on_error is specified with 'table', then on_error_rel is the error saving table
*/
bool
NextCopyFrom(CopyFromState cstate, ExprContext *econtext,
- Datum *values, bool *nulls)
+ Datum *values, bool *nulls, Relation on_error_rel)
{
TupleDesc tupDesc;
AttrNumber num_phys_attrs,
@@ -862,6 +864,8 @@ NextCopyFrom(CopyFromState cstate, ExprContext *econtext,
FmgrInfo *in_functions = cstate->in_functions;
Oid *typioparams = cstate->typioparams;
int i;
+ HeapTuple on_error_tup;
+ TupleDesc on_error_tupDesc;
int *defmap = cstate->defmap;
ExprState **defexprs = cstate->defexprs;
@@ -968,6 +972,52 @@ NextCopyFrom(CopyFromState cstate, ExprContext *econtext,
(Node *) cstate->escontext,
&values[m]))
{
+ if (cstate->opts.on_error == COPY_ON_ERROR_TABLE)
+ {
+ /*
+ * details of data definition of error saving table,
+ * see function BeginCopyFrom. here we based on node ErrorSaveContext
+ * content, form a tuple and insert tuple to the error saving table.
+ * we also did lock check in BeginCopyFrom.
+ */
+ char *err_detail;
+ char *err_code;
+ Datum t_values[10] = {0};
+ bool t_isnull[10] = {0};
+
+ err_code = pstrdup(unpack_sql_state(cstate->escontext->error_data->sqlerrcode));
+
+ if (!cstate->escontext->error_data->detail)
+ err_detail = NULL;
+ else
+ err_detail = cstate->escontext->error_data->detail;
+
+ t_values[0] = ObjectIdGetDatum(GetCurrentRoleId());
+ t_isnull[0] = false;
+ t_values[1] = ObjectIdGetDatum(cstate->rel->rd_rel->oid);
+ t_isnull[1] = false;
+ t_values[2] = CStringGetTextDatum(cstate->filename ? cstate->filename : "STDIN");
+ t_isnull[2] = false;
+ t_values[3] = Int64GetDatum((long long) cstate->cur_lineno);
+ t_isnull[3] = false;
+ t_values[4] = CStringGetTextDatum(cstate->line_buf.data);
+ t_isnull[4] = false;
+ t_values[5] = CStringGetTextDatum(cstate->cur_attname);
+ t_isnull[5] = false;
+ t_values[6] = CStringGetTextDatum(string);
+ t_isnull[6] = false;
+ t_values[7] = CStringGetTextDatum(cstate->escontext->error_data->message);
+ t_isnull[7] = false;
+ t_values[8] = err_detail ? CStringGetTextDatum(err_detail) : (Datum) 0;
+ t_isnull[8] = err_detail ? false: true;
+ t_values[9] = CStringGetTextDatum(err_code);
+ t_isnull[9] = false;
+ on_error_tupDesc = on_error_rel->rd_att;
+ on_error_tup = heap_form_tuple(on_error_tupDesc,
+ t_values,
+ t_isnull);
+ simple_heap_insert(on_error_rel, on_error_tup);
+ }
cstate->num_errors++;
return true;
}
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 130f7fc7..46fba94d 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -3420,6 +3420,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 b3da3cb0..1db62e57 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;
/*
@@ -73,6 +74,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 */
+ char *on_error_tbl; /* on error, save error info to table */
List *convert_select; /* list of column names (can be NIL) */
} CopyFormatOptions;
@@ -93,7 +95,7 @@ extern CopyFromState BeginCopyFrom(ParseState *pstate, Relation rel, Node *where
bool is_program, copy_data_source_cb data_source_cb, List *attnamelist, List *options);
extern void EndCopyFrom(CopyFromState cstate);
extern bool NextCopyFrom(CopyFromState cstate, ExprContext *econtext,
- Datum *values, bool *nulls);
+ Datum *values, bool *nulls, Relation on_error_rel);
extern bool NextCopyFromRawFields(CopyFromState cstate,
char ***fields, int *nfields);
extern void CopyFromErrorCallback(void *arg);
diff --git a/src/test/regress/expected/copy2.out b/src/test/regress/expected/copy2.out
index 25c401ce..423be157 100644
--- a/src/test/regress/expected/copy2.out
+++ b/src/test/regress/expected/copy2.out
@@ -751,6 +751,96 @@ 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 int8,
+ 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 int8,
+ 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);
+--should fail.
+COPY t_copy_tbl FROM STDIN WITH (on_error 'table');
+ERROR: argument to option "table" can only applied when ON ERROR is specified
+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: argument to option "table" can only applied when ON ERROR is specified
+COPY t_copy_tbl TO STDIN WITH (table err_tbl);
+ERROR: argument to option "table" can only applied when ON ERROR is specified
+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
+--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: The table ("public"."err_tbl_1") data definition cannot be used for error saving or current user don't enough priviledge on it
+HINT: Ensure current user have enough priviledge on "public"."err_tbl_1", also ensure the data definition can 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.
+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 on column errorcode
+begin;
+create user regress_user20;
+grant insert(userid,copy_tbl,filename,lineno,line,colname,raw_field_value,err_message,err_detail) on table err_tbl to alice;
+ERROR: role "alice" does not exist
+grant insert on table t_copy_tbl to alice;
+ERROR: current transaction is aborted, commands ignored until end of transaction block
+set role regress_user20;
+ERROR: current transaction is aborted, commands ignored until end of transaction block
+COPY t_copy_tbl FROM STDIN WITH (delimiter ',', on_error 'table', table err_tbl);
+ERROR: current transaction is aborted, commands ignored until end of transaction block
+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;
@@ -767,6 +857,9 @@ DROP VIEW instead_of_insert_tbl_view_2;
DROP FUNCTION fun_instead_of_insert_tbl();
DROP TABLE check_ign_err;
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 b5e549e8..811ebee3 100644
--- a/src/test/regress/sql/copy2.sql
+++ b/src/test/regress/sql/copy2.sql
@@ -534,6 +534,83 @@ 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 int8,
+ 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 int8,
+ 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);
+
+--should fail.
+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);
+
+--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.
+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 on column errorcode
+begin;
+create user regress_user20;
+grant insert(userid,copy_tbl,filename,lineno,line,colname,raw_field_value,err_message,err_detail) on table err_tbl to alice;
+grant insert on table t_copy_tbl to alice;
+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;
@@ -550,6 +627,9 @@ DROP VIEW instead_of_insert_tbl_view_2;
DROP FUNCTION fun_instead_of_insert_tbl();
DROP TABLE check_ign_err;
DROP TABLE hard_err;
+DROP TABLE err_tbl;
+DROP TABLE err_tbl_1;
+DROP TABLE t_copy_tbl;
--
-- COPY FROM ... DEFAULT
--
2.34.1
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: on_error table, saving error info to a table
2024-02-03 06:22 on_error table, saving error info to a table jian he <[email protected]>
@ 2024-07-15 05:42 ` Nishant Sharma <[email protected]>
2024-08-20 00:00 ` Re: on_error table, saving error info to a table jian he <[email protected]>
0 siblings, 1 reply; 13+ messages in thread
From: Nishant Sharma @ 2024-07-15 05:42 UTC (permalink / raw)
To: jian he <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
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?
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.
Regards,
Nishant Sharma.
EnterpriseDB, Pune.
On Sat, Feb 3, 2024 at 11:52 AM jian he <[email protected]> wrote:
> Hi.
> I previously did some work in COPY FROM save error information to a table.
> still based on this suggestion:
> https://www.postgresql.org/message-id/752672.1699474336%40sss.pgh.pa.us
> Now I refactored it.
>
> the syntax:
> ON_ERROR 'table', TABLE 'error_saving_tbl'
>
> if ON_ERROR is not specified with 'table', TABLE is specified, then error.
> if ON_ERROR is specified with 'table', TABLE is not specified or
> error_saving_tbl does not exist, then error.
>
> In BeginCopyFrom, we check the data definition of error_saving_table,
> we also check if the user has INSERT privilege to error_saving_table
> (all the columns).
> We also did a preliminary check of the lock condition of
> error_saving_table.
>
> if it does not meet these conditions, we quickly error out.
> error_saving_table will be the same schema as the copy from table.
>
> Because "table" is a keyword, I have to add the following changes to
> gram.y.
> --- a/src/backend/parser/gram.y
> +++ b/src/backend/parser/gram.y
> @@ -3420,6 +3420,10 @@ copy_opt_item:
> {
> $$ = makeDefElem("null", (Node *) makeString($3), @1);
> }
> + | TABLE opt_as Sconst
> + {
> + $$ = makeDefElem("table", (Node *) makeString($3), @1);
> + }
>
> since "table" is already a keyword, so there is no influence on the
> parsing speed?
>
> demo:
>
> create table err_tbl(
> userid oid, -- the user oid while copy generated this entry
> copy_tbl oid, --copy table
> filename text,
> lineno int8,
> line text,
> colname text,
> raw_field_value text,
> err_message text,
> err_detail text,
> errorcode text
> );
> create table t_copy_tbl(a int, b int, c int);
> COPY t_copy_tbl FROM STDIN WITH (delimiter ',', on_error 'table',
> table err_tbl);
> 1,2,a
> \.
>
> table err_tbl \gx
> -[ RECORD 1 ]---+-------------------------------------------
> userid | 10
> copy_tbl | 17920
> filename | STDIN
> lineno | 1
> line | 1,2,a
> colname | c
> raw_field_value | a
> err_message | invalid input syntax for type integer: "a"
> err_detail |
> errorcode | 22P02
>
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: on_error table, saving error info to a table
2024-02-03 06:22 on_error table, saving error info to a table jian he <[email protected]>
2024-07-15 05:42 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
@ 2024-08-20 00:00 ` jian he <[email protected]>
2024-11-05 10:30 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
0 siblings, 1 reply; 13+ messages in thread
From: jian he @ 2024-08-20 00:00 UTC (permalink / raw)
To: Nishant Sharma <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On Mon, Jul 15, 2024 at 1:42 PM Nishant Sharma
<[email protected]> 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.
Attachments:
[text/x-patch] v2-0001-new-on_error-option-table-saving-error-info-to-ta.patch (27.8K, 2-v2-0001-new-on_error-option-table-saving-error-info-to-ta.patch)
download | inline diff:
From f344314bfab06dfba3fd60c7cef16dd2d4d41803 Mon Sep 17 00:00:00 2001
From: jian he <[email protected]>
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/[email protected])
+ * 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
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: on_error table, saving error info to a table
2024-02-03 06:22 on_error table, saving error info to a table jian he <[email protected]>
2024-07-15 05:42 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
2024-08-20 00:00 ` Re: on_error table, saving error info to a table jian he <[email protected]>
@ 2024-11-05 10:30 ` Nishant Sharma <[email protected]>
2024-12-03 04:28 ` Re: on_error table, saving error info to a table jian he <[email protected]>
0 siblings, 1 reply; 13+ messages in thread
From: Nishant Sharma @ 2024-11-05 10:30 UTC (permalink / raw)
To: jian he <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
Thanks for the v2 patch!
I see v1 review comments got addressed in v2 along with some
further improvements.
1) v2 Patch again needs re-base.
2) I think we need not worry whether table name is unique or not,
table name can be provided by user and we can check if it does
not exists then simply we can create it with appropriate columns,
if it exists we use it to check if its correct on_error table and
proceed.
3) Using #define in between the code? I don't see that style in
copyfromparse.c file. I do see such style in other src file. So, not
sure if committer would allow it or not.
#define ERROR_TBL_COLUMNS 10
4) Below appears redundant to me, it was not the case in v1 patch
set, where it had only one return and one increment of error as new
added code was at the end of the block:-
+ cstate->num_errors++;
+ return true;
+ }
cstate->num_errors++;
I was not able to test the v2 due to conflicts in v2, I did attempt to
resolve but I saw many failures in make world.
Regards,
Nishant.
On Tue, Aug 20, 2024 at 5:31 AM jian he <[email protected]> wrote:
> On Mon, Jul 15, 2024 at 1:42 PM Nishant Sharma
> <[email protected]> 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.
>
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: on_error table, saving error info to a table
2024-02-03 06:22 on_error table, saving error info to a table jian he <[email protected]>
2024-07-15 05:42 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
2024-08-20 00:00 ` Re: on_error table, saving error info to a table jian he <[email protected]>
2024-11-05 10:30 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
@ 2024-12-03 04:28 ` jian he <[email protected]>
2024-12-03 10:28 ` Re: on_error table, saving error info to a table Kirill Reshke <[email protected]>
0 siblings, 1 reply; 13+ messages in thread
From: jian he @ 2024-12-03 04:28 UTC (permalink / raw)
To: Nishant Sharma <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On Tue, Nov 5, 2024 at 6:30 PM Nishant Sharma
<[email protected]> wrote:
>
> Thanks for the v2 patch!
>
> I see v1 review comments got addressed in v2 along with some
> further improvements.
>
> 1) v2 Patch again needs re-base.
>
> 2) I think we need not worry whether table name is unique or not,
> table name can be provided by user and we can check if it does
> not exists then simply we can create it with appropriate columns,
> if it exists we use it to check if its correct on_error table and
> proceed.
"simply we can create it with appropriate columns,"
that would be more work.
so i stick to if there is a table can use to
error saving then use it, otherwise error out.
>
> 3) Using #define in between the code? I don't see that style in
> copyfromparse.c file. I do see such style in other src file. So, not
> sure if committer would allow it or not.
> #define ERROR_TBL_COLUMNS 10
>
let's wait and see.
> 4) Below appears redundant to me, it was not the case in v1 patch
> set, where it had only one return and one increment of error as new
> added code was at the end of the block:-
> + cstate->num_errors++;
> + return true;
> + }
> cstate->num_errors++;
>
changed per your advice.
> I was not able to test the v2 due to conflicts in v2, I did attempt to
> resolve but I saw many failures in make world.
>
I get rid of all the SPI code.
Instead, now I iterate through AttributeRelationId to check if the
error saving table is ok or not,
using DirectFunctionCall3 to do the privilege check.
removed gram.y change, turns out it is not necessary.
and other kinds of refactoring.
please check attached.
Attachments:
[text/x-patch] v3-0001-introduce-on_error-table-option-for-COPY-FROM.patch (31.6K, 2-v3-0001-introduce-on_error-table-option-for-COPY-FROM.patch)
download | inline diff:
From b62dfa333cb0bc7efd45f221662c01a5db6d5c24 Mon Sep 17 00:00:00 2001
From: jian he <[email protected]>
Date: Tue, 3 Dec 2024 12:13:29 +0800
Subject: [PATCH v3 1/1] 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 error out.
we also did preliminary check the lock of error saving table so the insert to
error saving table won't stuck.
once there is a error happened, we save the error metedata and insert it to the
error_saving_table. and continue to the next row. That means for one row, we
can only catch the first filed that have errors.
discussion: https://postgr.es/m/CACJufxH_OJpVra%3D0c4ow8fbxHj7heMcVaTNEPa5vAurSeNA-6Q%40mail.gmail.com
---
doc/src/sgml/ref/copy.sgml | 118 ++++++++++++-
src/backend/commands/copy.c | 31 ++++
src/backend/commands/copyfrom.c | 203 ++++++++++++++++++++++-
src/backend/commands/copyfromparse.c | 45 +++++
src/include/commands/copy.h | 2 +
src/include/commands/copyfrom_internal.h | 1 +
src/test/regress/expected/copy2.out | 108 ++++++++++++
src/test/regress/sql/copy2.sql | 91 ++++++++++
8 files changed, 590 insertions(+), 9 deletions(-)
diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml
index 8394402f09..72ad4461ac 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_table</replaceable>'
REJECT_LIMIT <replaceable class="parameter">maxerror</replaceable>
ENCODING '<replaceable class="parameter">encoding_name</replaceable>'
LOG_VERBOSITY <replaceable class="parameter">verbosity</replaceable>
@@ -395,11 +396,13 @@ 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> insert error related information to <replaceable class="parameter">error_saving_table</replaceable>
+ and continue with the next one.
The default is <literal>stop</literal>.
</para>
<para>
- The <literal>ignore</literal> option is applicable only for <command>COPY FROM</command>
+ The <literal>ignore</literal> and <literal>table</literal> option is applicable only for <command>COPY FROM</command>
when the <literal>FORMAT</literal> is <literal>text</literal> or <literal>csv</literal>.
</para>
<para>
@@ -463,6 +466,117 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>TABLE</literal></term>
+ <listitem>
+ <para>
+ Save error context details to the table <replaceable class="parameter">error_saving_table</replaceable>
+ when errors occur during the conversion of a column's input value into its data type in a <command>COPY FROM</command> operation.
+ </para>
+
+ <para>
+ The <replaceable class="parameter">error_saving_table</replaceable> must exist in the current database
+ and share the same schema as the destination table of the <command>COPY FROM</command> operation.
+ This option is allowed only in <command>COPY FROM</command> and
+ <literal>ON_ERROR</literal> is specified with <literal>TABLE</literal>.
+
+ The user performing the <command>COPY FROM</command>
+ operation must have <literal>INSERT</literal> privileges for all columns
+ in the <replaceable class="parameter">error_saving_table</replaceable>.
+ If this option is not specified, the <literal>ON_ERROR</literal> parameter cannot be set to <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 2d98ecf3f4..f159502a9b 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 */
@@ -502,6 +504,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;
bool reject_limit_specified = false;
ListCell *option;
@@ -677,6 +680,14 @@ ProcessCopyOptions(ParseState *pstate,
reject_limit_specified = true;
opts_out->reject_limit = defGetCopyRejectLimitOption(defel);
}
+ 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),
@@ -685,6 +696,26 @@ ProcessCopyOptions(ParseState *pstate,
parser_errposition(pstate, defel->location)));
}
+ if (opts_out->on_error == COPY_ON_ERROR_TABLE && opts_out->on_error_tbl == NULL)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("COPY %s \"table\" requires a custom specified error saving table", "ON_ERROR"),
+ errhint("You need also specify \"TABLE\" option.")));
+
+ if (opts_out->on_error == COPY_ON_ERROR_TABLE && opts_out->reject_limit)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("cannot specify %s option when %s option is specified as \"table\"", "REJECT_LIMIT", "ON_ERROR")));
+
+ if (opts_out->on_error == COPY_ON_ERROR_TABLE && opts_out->log_verbosity == COPY_LOG_VERBOSITY_VERBOSE)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("cannot specify %s as \"verbose\" when %s option is specified as \"table\"", "log_verbosity", "ON_ERROR")));
+
+ if (opts_out->on_error != COPY_ON_ERROR_TABLE && opts_out->on_error_tbl != NULL)
+ 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 three before inserting
* defaults)
diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c
index 4d52c93c30..376d3f04c9 100644
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -28,6 +28,7 @@
#include "access/tableam.h"
#include "access/xact.h"
#include "catalog/namespace.h"
+#include "catalog/pg_type.h"
#include "commands/copy.h"
#include "commands/copyfrom_internal.h"
#include "commands/progress.h"
@@ -44,7 +45,10 @@
#include "pgstat.h"
#include "rewrite/rewriteHandler.h"
#include "storage/fd.h"
+#include "storage/lmgr.h"
#include "tcop/tcopprot.h"
+#include "utils/builtins.h"
+#include "utils/fmgroids.h"
#include "utils/lsyscache.h"
#include "utils/memutils.h"
#include "utils/portal.h"
@@ -1029,6 +1033,16 @@ CopyFrom(CopyFromState cstate)
continue;
}
+ if (cstate->opts.on_error == COPY_ON_ERROR_TABLE &&
+ cstate->escontext->error_occurred)
+ {
+ cstate->escontext->error_occurred = false;
+ cstate->escontext->details_wanted = true;
+ memset(cstate->escontext->error_data, 0, sizeof(ErrorData));
+ /* Repeat NextCopyFrom() until no soft error occurs */
+ continue;
+ }
+
ExecStoreVirtualTuple(myslot);
/*
@@ -1321,14 +1335,31 @@ CopyFrom(CopyFromState cstate)
/* Done, clean up */
error_context_stack = errcallback.previous;
- if (cstate->opts.on_error != COPY_ON_ERROR_STOP &&
- cstate->num_errors > 0 &&
+ if (cstate->num_errors > 0 &&
cstate->opts.log_verbosity >= COPY_LOG_VERBOSITY_DEFAULT)
- ereport(NOTICE,
- errmsg_plural("%llu row was skipped due to data type incompatibility",
- "%llu rows were skipped due to data type incompatibility",
- (unsigned long long) cstate->num_errors,
- (unsigned long long) cstate->num_errors));
+ {
+ if(cstate->opts.on_error == COPY_ON_ERROR_IGNORE)
+ ereport(NOTICE,
+ errmsg_plural("%llu row was skipped due to data type incompatibility",
+ "%llu rows were skipped due to data type incompatibility",
+ (unsigned long long) cstate->num_errors,
+ (unsigned long long) cstate->num_errors));
+ else if(cstate->opts.on_error == COPY_ON_ERROR_TABLE)
+ ereport(NOTICE,
+ errmsg_plural("%llu row was saved to table \"%s\" due to data type incompatibility",
+ "%llu rows were saved to table \"%s\" due to data type incompatibility",
+ (unsigned long long) cstate->num_errors,
+ (unsigned long long) cstate->num_errors,
+ RelationGetRelationName(cstate->error_saving_rel)));
+ }
+
+ /*
+ * similar to commit a9cf48a
+ * (https://postgr.es/m/[email protected])
+ * 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);
@@ -1483,6 +1514,164 @@ BeginCopyFrom(ParseState *pstate,
else
cstate->escontext = NULL;
+ if (cstate->opts.on_error == COPY_ON_ERROR_TABLE)
+ {
+ const char *copy_nspname;
+ Datum ins_prev;
+ Oid err_tbl_oid;
+ Oid copy_nspoid;
+ bool on_error_tbl_ok = true;
+ Relation arel;
+ ScanKeyData akey;
+ SysScanDesc ascan;
+ HeapTuple atup;
+ Form_pg_attribute attForm;
+ int maxattnum = 0;
+
+ Assert(cstate->escontext != NULL);
+ Assert(cstate->opts.on_error_tbl != NULL);
+
+ 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))));
+
+ /* current user should have INSERT privilege on error_saving table */
+ ins_prev = DirectFunctionCall3(has_table_privilege_id_id,
+ ObjectIdGetDatum(GetUserId()),
+ ObjectIdGetDatum(err_tbl_oid),
+ CStringGetTextDatum("INSERT"));
+ if (!DatumGetBool(ins_prev))
+ 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 privilege on \"%s\".\"%s\" for COPY FROM error saving",
+ copy_nspname, cstate->opts.on_error_tbl)));
+
+ /*
+ * we may insert tuples to error-saving table, to do that we need first
+ * check it's lock situation. If it is already under heavy lock, then
+ * our COPY operation would be stuck. instead of let COPY stuck, just
+ * error report the error-saving table is in heavy lock.
+ */
+ if (!ConditionalLockRelationOid(err_tbl_oid, RowExclusiveLock))
+ 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)));
+ cstate->error_saving_rel = table_open(err_tbl_oid, NoLock);
+
+ /*
+ * can this error saving table (cstate->error_saving_rel) be used for
+ * error saving or not? for that we need to check the table's (column
+ * name, column data types, number of column)
+ *
+ */
+ arel = table_open(AttributeRelationId, AccessShareLock);
+ ScanKeyInit(&akey,
+ Anum_pg_attribute_attrelid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(err_tbl_oid));
+
+ ascan = systable_beginscan(arel, AttributeRelidNumIndexId, true,
+ SnapshotSelf, 1, &akey);
+ while (HeapTupleIsValid(atup = systable_getnext(ascan)))
+ {
+ attForm = (Form_pg_attribute) GETSTRUCT(atup);
+ if (attForm->attnum < 1 || attForm->attisdropped)
+ continue;
+
+ if (maxattnum <= attForm->attnum)
+ maxattnum = attForm->attnum;
+
+ switch (attForm->attnum)
+ {
+ case 1:
+ if((attForm->atttypid != OIDOID) ||
+ (strcmp(NameStr(attForm->attname), "userid") != 0))
+ on_error_tbl_ok = false;
+ break;
+ case 2:
+ if ((attForm->atttypid != OIDOID) ||
+ (strcmp(NameStr(attForm->attname), "copy_tbl") != 0))
+ on_error_tbl_ok = false;
+ break;
+ case 3:
+ if ((attForm->atttypid != TEXTOID) ||
+ (strcmp(NameStr(attForm->attname), "filename") != 0))
+ on_error_tbl_ok = false;
+ break;
+ case 4:
+ if ((attForm->atttypid != INT8OID) ||
+ (strcmp(NameStr(attForm->attname), "lineno") != 0))
+ on_error_tbl_ok = false;
+ break;
+ case 5:
+ if ((attForm->atttypid != TEXTOID) ||
+ (strcmp(NameStr(attForm->attname), "line") != 0))
+ on_error_tbl_ok = false;
+ break;
+ case 6:
+ if ((attForm->atttypid != TEXTOID) ||
+ (strcmp(NameStr(attForm->attname), "colname") != 0))
+ on_error_tbl_ok = false;
+ break;
+ case 7:
+ if ((attForm->atttypid != TEXTOID) ||
+ (strcmp(NameStr(attForm->attname), "raw_field_value") != 0))
+ on_error_tbl_ok = false;
+ break;
+ case 8:
+ if ((attForm->atttypid != TEXTOID) ||
+ (strcmp(NameStr(attForm->attname), "err_message") != 0))
+ on_error_tbl_ok = false;
+ break;
+ case 9:
+ if ((attForm->atttypid != TEXTOID) ||
+ (strcmp(NameStr(attForm->attname), "err_detail") != 0))
+ on_error_tbl_ok = false;
+ break;
+ case 10:
+ if ((attForm->atttypid != TEXTOID) ||
+ (strcmp(NameStr(attForm->attname), "errorcode") != 0))
+ on_error_tbl_ok = false;
+ break;
+ default:
+ on_error_tbl_ok = false;
+ break;
+ }
+ }
+ if (maxattnum != 10)
+ on_error_tbl_ok = false;
+
+ systable_endscan(ascan);
+ table_close(arel, AccessShareLock);
+
+ 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)));
+
+ 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 d1d43b53d8..1f1bf24788 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"
@@ -959,7 +960,51 @@ NextCopyFrom(CopyFromState cstate, ExprContext *econtext,
&values[m]))
{
Assert(cstate->opts.on_error != COPY_ON_ERROR_STOP);
+ Assert(cstate->escontext->error_occurred);
+ 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 acquired
+ * lock on error_saving_rel 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++;
if (cstate->opts.log_verbosity == COPY_LOG_VERBOSITY_VERBOSE)
diff --git a/src/include/commands/copy.h b/src/include/commands/copy.h
index 4002a7f538..59365c81da 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;
/*
@@ -86,6 +87,7 @@ typedef struct CopyFormatOptions
CopyOnErrorChoice on_error; /* what to do when error happened */
CopyLogVerbosityChoice log_verbosity; /* verbosity of logged messages */
int64 reject_limit; /* maximum tolerable number of errors */
+ char *on_error_tbl; /* on error, save error info to the table, table name */
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 64ea33aeae..6a5290b53f 100644
--- a/src/test/regress/expected/copy2.out
+++ b/src/test/regress/expected/copy2.out
@@ -813,6 +813,110 @@ ERROR: skipped more than REJECT_LIMIT (3) rows due to data type incompatibility
CONTEXT: COPY check_ign_err, line 5, column n: ""
COPY check_ign_err FROM STDIN WITH (on_error ignore, reject_limit 4);
NOTICE: 4 rows were skipped due to data type incompatibility
+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
+);
+--cannot use for error saving.
+create table err_tbl_1(
+userid oid,
+copy_tbl oid,
+filename text,
+lineno bigint,
+line text,
+colname text,
+raw_field_value text,
+err_message text,
+err_detail text
+);
+--cannot use for error saving.
+create table err_tbl_2(
+ userid oid, copy_tbl oid, filename text, lineno bigint,line text,
+ colname text, raw_field_value text, err_message text,
+ err_detail text,
+ errorcode text,
+ errorcode1 text
+);
+create table t_copy_tbl(a int, b int, c int);
+create view s1 as select 1 as a;
+----invalid options, the below all should fails
+COPY t_copy_tbl FROM STDIN WITH (on_error 'table');
+ERROR: COPY ON_ERROR "table" requires a custom specified error saving table
+HINT: You need also specify "TABLE" option.
+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 (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(a,b) FROM STDIN WITH (on_error 'table', reject_limit 10, table err_tbl);
+ERROR: cannot specify REJECT_LIMIT option when ON_ERROR option is specified as "table"
+COPY t_copy_tbl(a,b) FROM STDIN WITH (on_error 'table', log_verbosity verbose, table err_tbl);
+ERROR: cannot specify log_verbosity as "verbose" 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
+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.
+----invalid options, the above all should fails
+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. err_tbl_2 does not meet criteria
+COPY t_copy_tbl(a,b) FROM STDIN WITH (on_error 'table', table err_tbl_2);
+ERROR: table "public"."err_tbl_2" cannot be used for COPY FROM error saving
+DETAIL: Table "public"."err_tbl_2" data definition cannot be used for error saving
+--should fail, copied data have 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, copied data have 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 saved to table "err_tbl" due to data type incompatibility
+--should fail. lack privilege
+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 privilege 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 | 6 | 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
+(2 rows)
+
-- clean up
DROP TABLE forcetest;
DROP TABLE vistest;
@@ -831,6 +935,10 @@ 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 err_tbl_2;
+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 45273557ce..aa5fc12cb3 100644
--- a/src/test/regress/sql/copy2.sql
+++ b/src/test/regress/sql/copy2.sql
@@ -588,6 +588,93 @@ a {7} 7
10 {10} 10
\.
+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
+);
+--cannot use for error saving.
+create table err_tbl_1(
+userid oid,
+copy_tbl oid,
+filename text,
+lineno bigint,
+line text,
+colname text,
+raw_field_value text,
+err_message text,
+err_detail text
+);
+--cannot use for error saving.
+create table err_tbl_2(
+ userid oid, copy_tbl oid, filename text, lineno bigint,line text,
+ colname text, raw_field_value text, err_message text,
+ err_detail text,
+ errorcode text,
+ errorcode1 text
+);
+create table t_copy_tbl(a int, b int, c int);
+create view s1 as select 1 as a;
+
+----invalid options, the below all should fails
+COPY t_copy_tbl FROM STDIN WITH (on_error 'table');
+COPY t_copy_tbl FROM STDIN WITH (table err_tbl);
+COPY t_copy_tbl TO STDIN WITH (on_error 'table');
+COPY t_copy_tbl(a,b) FROM STDIN WITH (on_error 'table', reject_limit 10, table err_tbl);
+COPY t_copy_tbl(a,b) FROM STDIN WITH (on_error 'table', log_verbosity verbose, table err_tbl);
+COPY t_copy_tbl(a,b) FROM STDIN WITH (on_error 'table', table not_exists);
+COPY t_copy_tbl(a) FROM STDIN WITH (on_error 'table', table s1);
+----invalid options, the above all should fails
+
+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. err_tbl_2 does not meet criteria
+COPY t_copy_tbl(a,b) FROM STDIN WITH (on_error 'table', table err_tbl_2);
+
+--should fail, copied data have extra columns
+COPY t_copy_tbl(a,b) FROM STDIN WITH (DELIMITER ',', on_error 'table', table err_tbl);
+1,2,3,4
+\.
+
+--should fail, copied data have 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
+1,11,4238679732489879879
+\.
+
+--should fail. lack privilege
+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;
@@ -606,6 +693,10 @@ 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 err_tbl_2;
+DROP TABLE t_copy_tbl;
--
-- COPY FROM ... DEFAULT
--
2.34.1
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: on_error table, saving error info to a table
2024-02-03 06:22 on_error table, saving error info to a table jian he <[email protected]>
2024-07-15 05:42 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
2024-08-20 00:00 ` Re: on_error table, saving error info to a table jian he <[email protected]>
2024-11-05 10:30 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
2024-12-03 04:28 ` Re: on_error table, saving error info to a table jian he <[email protected]>
@ 2024-12-03 10:28 ` Kirill Reshke <[email protected]>
2024-12-11 11:41 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
0 siblings, 1 reply; 13+ messages in thread
From: Kirill Reshke @ 2024-12-03 10:28 UTC (permalink / raw)
To: jian he <[email protected]>; +Cc: Nishant Sharma <[email protected]>; PostgreSQL Hackers <[email protected]>
On Tue, 3 Dec 2024 at 09:29, jian he <[email protected]> wrote:
>
> On Tue, Nov 5, 2024 at 6:30 PM Nishant Sharma
> <[email protected]> wrote:
> >
> > Thanks for the v2 patch!
> >
> > I see v1 review comments got addressed in v2 along with some
> > further improvements.
> >
> > 1) v2 Patch again needs re-base.
> >
> > 2) I think we need not worry whether table name is unique or not,
> > table name can be provided by user and we can check if it does
> > not exists then simply we can create it with appropriate columns,
> > if it exists we use it to check if its correct on_error table and
> > proceed.
>
> "simply we can create it with appropriate columns,"
> that would be more work.
> so i stick to if there is a table can use to
> error saving then use it, otherwise error out.
>
>
> >
> > 3) Using #define in between the code? I don't see that style in
> > copyfromparse.c file. I do see such style in other src file. So, not
> > sure if committer would allow it or not.
> > #define ERROR_TBL_COLUMNS 10
> >
> let's wait and see.
>
> > 4) Below appears redundant to me, it was not the case in v1 patch
> > set, where it had only one return and one increment of error as new
> > added code was at the end of the block:-
> > + cstate->num_errors++;
> > + return true;
> > + }
> > cstate->num_errors++;
> >
> changed per your advice.
>
> > I was not able to test the v2 due to conflicts in v2, I did attempt to
> > resolve but I saw many failures in make world.
> >
> I get rid of all the SPI code.
>
> Instead, now I iterate through AttributeRelationId to check if the
> error saving table is ok or not,
> using DirectFunctionCall3 to do the privilege check.
> removed gram.y change, turns out it is not necessary.
> and other kinds of refactoring.
>
> please check attached.
Hi!
1)
> + switch (attForm->attnum)
> + {
> + case 1:
> + (.....)
> + case 2:
case 1,2,3 ... Is too random. Other parts of core tend to use `#define
Anum_<relname>_<columname> <num>`. Can we follow this style?
2)
>+ /*
> + * similar to commit a9cf48a
> + * (https://postgr.es/m/[email protected])
> + * in COPY FROM keep error saving table locks until the transaction end.
> + */
I can rarely see other comments referencing commits, and even few
referencing a mail archive thread.
Can we just write proper comment explaining the reasons?
===== overall
Patch design is a little dubious for me. We give users some really
incomprehensible API. To use on_error *relation* feature user must
create tables with proper schema.
Maybe a better design will be to auto-create on_error table if this
table does not exist.
Thoughts?
--
Best regards,
Kirill Reshke
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: on_error table, saving error info to a table
2024-02-03 06:22 on_error table, saving error info to a table jian he <[email protected]>
2024-07-15 05:42 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
2024-08-20 00:00 ` Re: on_error table, saving error info to a table jian he <[email protected]>
2024-11-05 10:30 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
2024-12-03 04:28 ` Re: on_error table, saving error info to a table jian he <[email protected]>
2024-12-03 10:28 ` Re: on_error table, saving error info to a table Kirill Reshke <[email protected]>
@ 2024-12-11 11:41 ` Nishant Sharma <[email protected]>
2024-12-13 08:26 ` Re: on_error table, saving error info to a table jian he <[email protected]>
0 siblings, 1 reply; 13+ messages in thread
From: Nishant Sharma @ 2024-12-11 11:41 UTC (permalink / raw)
To: jian he <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>; Kirill Reshke <[email protected]>
Thanks for the v3 patch!
Please find review comments on v3:-
1) I think no need to change the below if condition, we can keep
it the way it was before i.e with
"cstate->opts.on_error != COPY_ON_ERROR_STOP" and we
add a new error ereport the way v3 has. Because for
cstate->opts.on_error as COPY_ON_ERROR_STOP cases we
can avoid two if conditions inside upper if.
+ if (cstate->num_errors > 0 &&
cstate->opts.log_verbosity >= COPY_LOG_VERBOSITY_DEFAULT)
2) No need for the below "if" check for maxattnum. We can simply
increment it with "++maxattnum" and later check if we have exactly
10 attributes for the error table. Because even if we drop any
attribute and maxattnum is 10 in pg_attribute for that rel, we should
still error out. Maybe we can rename it to "totalatts"?
+ if (maxattnum <= attForm->attnum)
+ maxattnum = attForm->attnum;
3) #define would be better, also as mentioned by Kirill switch
condition with proper #define would be better.
+ if (maxattnum != 10)
+ on_error_tbl_ok = false;
4)
>
> that would be more work.
> so i stick to if there is a table can use to
> error saving then use it, otherwise error out.
>
YES. but that would lead to a better design with an error table.
Also, I think Krill mentions the same. That is to auto create, if it
does not exist.
Regards,
Nishant Sharma (EDB).
On Tue, Dec 3, 2024 at 3:58 PM Kirill Reshke <[email protected]> wrote:
> On Tue, 3 Dec 2024 at 09:29, jian he <[email protected]> wrote:
> >
> > On Tue, Nov 5, 2024 at 6:30 PM Nishant Sharma
> > <[email protected]> wrote:
> > >
> > > Thanks for the v2 patch!
> > >
> > > I see v1 review comments got addressed in v2 along with some
> > > further improvements.
> > >
> > > 1) v2 Patch again needs re-base.
> > >
> > > 2) I think we need not worry whether table name is unique or not,
> > > table name can be provided by user and we can check if it does
> > > not exists then simply we can create it with appropriate columns,
> > > if it exists we use it to check if its correct on_error table and
> > > proceed.
> >
> > "simply we can create it with appropriate columns,"
> > that would be more work.
> > so i stick to if there is a table can use to
> > error saving then use it, otherwise error out.
> >
> >
> > >
> > > 3) Using #define in between the code? I don't see that style in
> > > copyfromparse.c file. I do see such style in other src file. So, not
> > > sure if committer would allow it or not.
> > > #define ERROR_TBL_COLUMNS 10
> > >
> > let's wait and see.
> >
> > > 4) Below appears redundant to me, it was not the case in v1 patch
> > > set, where it had only one return and one increment of error as new
> > > added code was at the end of the block:-
> > > + cstate->num_errors++;
> > > + return true;
> > > + }
> > > cstate->num_errors++;
> > >
> > changed per your advice.
> >
> > > I was not able to test the v2 due to conflicts in v2, I did attempt to
> > > resolve but I saw many failures in make world.
> > >
> > I get rid of all the SPI code.
> >
> > Instead, now I iterate through AttributeRelationId to check if the
> > error saving table is ok or not,
> > using DirectFunctionCall3 to do the privilege check.
> > removed gram.y change, turns out it is not necessary.
> > and other kinds of refactoring.
> >
> > please check attached.
>
>
> Hi!
>
> 1)
> > + switch (attForm->attnum)
> > + {
> > + case 1:
> > + (.....)
> > + case 2:
>
> case 1,2,3 ... Is too random. Other parts of core tend to use `#define
> Anum_<relname>_<columname> <num>`. Can we follow this style?
>
> 2)
> >+ /*
> > + * similar to commit a9cf48a
> > + * (
> https://postgr.es/m/[email protected]
> )
> > + * in COPY FROM keep error saving table locks until the transaction end.
> > + */
>
> I can rarely see other comments referencing commits, and even few
> referencing a mail archive thread.
> Can we just write proper comment explaining the reasons?
>
>
> ===== overall
>
> Patch design is a little dubious for me. We give users some really
> incomprehensible API. To use on_error *relation* feature user must
> create tables with proper schema.
> Maybe a better design will be to auto-create on_error table if this
> table does not exist.
>
>
> Thoughts?
>
> --
> Best regards,
> Kirill Reshke
>
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: on_error table, saving error info to a table
2024-02-03 06:22 on_error table, saving error info to a table jian he <[email protected]>
2024-07-15 05:42 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
2024-08-20 00:00 ` Re: on_error table, saving error info to a table jian he <[email protected]>
2024-11-05 10:30 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
2024-12-03 04:28 ` Re: on_error table, saving error info to a table jian he <[email protected]>
2024-12-03 10:28 ` Re: on_error table, saving error info to a table Kirill Reshke <[email protected]>
2024-12-11 11:41 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
@ 2024-12-13 08:26 ` jian he <[email protected]>
2024-12-16 11:50 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
0 siblings, 1 reply; 13+ messages in thread
From: jian he @ 2024-12-13 08:26 UTC (permalink / raw)
To: Nishant Sharma <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>; Kirill Reshke <[email protected]>
On Wed, Dec 11, 2024 at 7:41 PM Nishant Sharma
<[email protected]> wrote:
>
> Thanks for the v3 patch!
>
> Please find review comments on v3:-
>
> 1) I think no need to change the below if condition, we can keep
> it the way it was before i.e with
> "cstate->opts.on_error != COPY_ON_ERROR_STOP" and we
> add a new error ereport the way v3 has. Because for
> cstate->opts.on_error as COPY_ON_ERROR_STOP cases we
> can avoid two if conditions inside upper if.
>
> + if (cstate->num_errors > 0 &&
> cstate->opts.log_verbosity >= COPY_LOG_VERBOSITY_DEFAULT)
> 2) No need for the below "if" check for maxattnum. We can simply
> increment it with "++maxattnum" and later check if we have exactly
> 10 attributes for the error table. Because even if we drop any
> attribute and maxattnum is 10 in pg_attribute for that rel, we should
> still error out. Maybe we can rename it to "totalatts"?
>
> + if (maxattnum <= attForm->attnum)
> + maxattnum = attForm->attnum;
>
> 3) #define would be better, also as mentioned by Kirill switch
> condition with proper #define would be better.
>
> + if (maxattnum != 10)
> + on_error_tbl_ok = false;
>
> 4)
hi. Thanks for the review.
The attached v4 patch addressed these two issues.
> > that would be more work.
> > so i stick to if there is a table can use to
> > error saving then use it, otherwise error out.
> >
> YES. but that would lead to a better design with an error table.
> Also, I think Krill mentions the same. That is to auto create, if it
> does not exist.
>
I decided not to auto-create the table.
main reason not to do it:
1. utility COPY command with another SPI utility CREATE TABLE command may work.
but there is no precedent.
2. if we auto-create the on_error table with BeginCopyFrom.
then later we have to use get_relname_relid to get the newly created table Oid,
I think it somehow counts as repeating name lookups, see relevant
linke [1], [2].
[1] https://postgr.es/m/[email protected]
[2] https://postgr.es/m/CA+TgmobHYix=Nn8D4RUHa6fhUVPR88KGAMq1pBfnGfOfEjRixA@mail.gmail.com
Attachments:
[text/x-patch] v4-0001-introduce-on_error-table-option-for-COPY-FROM.patch (31.5K, 2-v4-0001-introduce-on_error-table-option-for-COPY-FROM.patch)
download | inline diff:
From 4e38c800a7481c1a4fad1b6beb3c11dd24235001 Mon Sep 17 00:00:00 2001
From: jian he <[email protected]>
Date: Fri, 13 Dec 2024 16:23:17 +0800
Subject: [PATCH v4 1/1] 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 error out.
we also did preliminary check the lock of error saving table so the insert to
error saving table won't stuck.
once there is a error happened, we save the error metedata and insert it to the
error_saving_table. and continue to the next row. That means for one row, we
can only catch the first field that have errors.
discussion: https://postgr.es/m/CACJufxH_OJpVra%3D0c4ow8fbxHj7heMcVaTNEPa5vAurSeNA-6Q%40mail.gmail.com
context: https://www.postgresql.org/message-id/752672.1699474336%40sss.pgh.pa.us
commitfest: https://commitfest.postgresql.org/51/4817/
---
doc/src/sgml/ref/copy.sgml | 118 +++++++++++++-
src/backend/commands/copy.c | 31 ++++
src/backend/commands/copyfrom.c | 193 ++++++++++++++++++++++-
src/backend/commands/copyfromparse.c | 43 +++++
src/include/commands/copy.h | 8 +
src/include/commands/copyfrom_internal.h | 1 +
src/test/regress/expected/copy2.out | 107 +++++++++++++
src/test/regress/sql/copy2.sql | 89 +++++++++++
8 files changed, 583 insertions(+), 7 deletions(-)
diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml
index 8394402f09..e66474c74f 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_table</replaceable>'
REJECT_LIMIT <replaceable class="parameter">maxerror</replaceable>
ENCODING '<replaceable class="parameter">encoding_name</replaceable>'
LOG_VERBOSITY <replaceable class="parameter">verbosity</replaceable>
@@ -395,11 +396,13 @@ 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> insert error related information to <replaceable class="parameter">error_saving_table</replaceable>
+ and continue with the next one.
The default is <literal>stop</literal>.
</para>
<para>
- The <literal>ignore</literal> option is applicable only for <command>COPY FROM</command>
+ The <literal>ignore</literal> and <literal>table</literal> option is applicable only for <command>COPY FROM</command>
when the <literal>FORMAT</literal> is <literal>text</literal> or <literal>csv</literal>.
</para>
<para>
@@ -463,6 +466,117 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>TABLE</literal></term>
+ <listitem>
+ <para>
+ Save error context details to the table <replaceable class="parameter">error_saving_table</replaceable>
+ when <literal>ON_ERROR</literal> is set to <literal>TABLE</literal>.
+ </para>
+
+ <para>
+ The <replaceable class="parameter">error_saving_table</replaceable> must exist in the current database
+ and share the same schema as the destination table of the <command>COPY FROM</command> operation.
+ This option is allowed only in <command>COPY FROM</command> and
+ <literal>ON_ERROR</literal> is specified with <literal>TABLE</literal>.
+
+ The user performing the <command>COPY FROM</command>
+ operation must have <literal>INSERT</literal> privileges for all columns
+ in the <replaceable class="parameter">error_saving_table</replaceable>.
+ If this option is not specified, the <literal>ON_ERROR</literal> parameter cannot be set to <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 2d98ecf3f4..a63989223c 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 */
@@ -502,6 +504,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;
bool reject_limit_specified = false;
ListCell *option;
@@ -677,6 +680,14 @@ ProcessCopyOptions(ParseState *pstate,
reject_limit_specified = true;
opts_out->reject_limit = defGetCopyRejectLimitOption(defel);
}
+ 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),
@@ -685,6 +696,26 @@ ProcessCopyOptions(ParseState *pstate,
parser_errposition(pstate, defel->location)));
}
+ if (opts_out->on_error == COPY_ON_ERROR_TABLE && opts_out->on_error_tbl == NULL)
+ ereport(ERROR,
+ errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("COPY %s \"table\" requires a custom specified error saving table", "ON_ERROR"),
+ errhint("You need also specify \"TABLE\" option."));
+
+ if (opts_out->on_error == COPY_ON_ERROR_TABLE && opts_out->reject_limit)
+ ereport(ERROR,
+ errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("cannot specify %s option when %s option is specified as \"table\"", "REJECT_LIMIT", "ON_ERROR"));
+
+ if (opts_out->on_error == COPY_ON_ERROR_TABLE && opts_out->log_verbosity == COPY_LOG_VERBOSITY_VERBOSE)
+ ereport(ERROR,
+ errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("cannot specify %s as \"verbose\" when %s option is specified as \"table\"", "log_verbosity", "ON_ERROR"));
+
+ if (opts_out->on_error != COPY_ON_ERROR_TABLE && opts_out->on_error_tbl != NULL)
+ 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 three before inserting
* defaults)
diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c
index 4d52c93c30..423944860c 100644
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -28,6 +28,7 @@
#include "access/tableam.h"
#include "access/xact.h"
#include "catalog/namespace.h"
+#include "catalog/pg_type.h"
#include "commands/copy.h"
#include "commands/copyfrom_internal.h"
#include "commands/progress.h"
@@ -35,6 +36,7 @@
#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"
@@ -44,7 +46,10 @@
#include "pgstat.h"
#include "rewrite/rewriteHandler.h"
#include "storage/fd.h"
+#include "storage/lmgr.h"
#include "tcop/tcopprot.h"
+#include "utils/builtins.h"
+#include "utils/fmgroids.h"
#include "utils/lsyscache.h"
#include "utils/memutils.h"
#include "utils/portal.h"
@@ -1029,6 +1034,16 @@ CopyFrom(CopyFromState cstate)
continue;
}
+ if (cstate->opts.on_error == COPY_ON_ERROR_TABLE &&
+ cstate->escontext->error_occurred)
+ {
+ cstate->escontext->error_occurred = false;
+ cstate->escontext->details_wanted = true;
+ memset(cstate->escontext->error_data, 0, sizeof(ErrorData));
+ /* Repeat NextCopyFrom() until no soft error occurs */
+ continue;
+ }
+
ExecStoreVirtualTuple(myslot);
/*
@@ -1324,11 +1339,29 @@ CopyFrom(CopyFromState cstate)
if (cstate->opts.on_error != COPY_ON_ERROR_STOP &&
cstate->num_errors > 0 &&
cstate->opts.log_verbosity >= COPY_LOG_VERBOSITY_DEFAULT)
- ereport(NOTICE,
- errmsg_plural("%llu row was skipped due to data type incompatibility",
- "%llu rows were skipped due to data type incompatibility",
- (unsigned long long) cstate->num_errors,
- (unsigned long long) cstate->num_errors));
+ {
+ if(cstate->opts.on_error == COPY_ON_ERROR_IGNORE)
+ ereport(NOTICE,
+ errmsg_plural("%llu row was skipped due to data type incompatibility",
+ "%llu rows were skipped due to data type incompatibility",
+ (unsigned long long) cstate->num_errors,
+ (unsigned long long) cstate->num_errors));
+ else
+ ereport(NOTICE,
+ errmsg_plural("%llu row was saved to table \"%s\" due to data type incompatibility",
+ "%llu rows were saved to table \"%s\" due to data type incompatibility",
+ (unsigned long long) cstate->num_errors,
+ (unsigned long long) cstate->num_errors,
+ RelationGetRelationName(cstate->error_saving_rel)));
+ }
+
+ /*
+ * similar to
+ * (https://postgr.es/m/[email protected])
+ * 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);
@@ -1483,6 +1516,156 @@ BeginCopyFrom(ParseState *pstate,
else
cstate->escontext = NULL;
+ if (cstate->opts.on_error == COPY_ON_ERROR_TABLE)
+ {
+ const char *copy_nspname;
+ Datum ins_prev;
+ Oid err_tbl_oid;
+ Oid copy_nspoid;
+ Relation arel;
+ ScanKeyData akey;
+ SysScanDesc ascan;
+ HeapTuple atup;
+ Form_pg_attribute attForm;
+ int attcnt = 1;
+
+ Assert(cstate->escontext != NULL);
+ Assert(cstate->opts.on_error_tbl != NULL);
+
+ 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)));
+
+ /* current user should have INSERT privilege on error_saving table */
+ ins_prev = DirectFunctionCall3(has_table_privilege_id_id,
+ ObjectIdGetDatum(GetUserId()),
+ ObjectIdGetDatum(err_tbl_oid),
+ CStringGetTextDatum("INSERT"));
+ if (!DatumGetBool(ins_prev))
+ 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 privilege on \"%s\".\"%s\" for COPY FROM error saving",
+ copy_nspname, cstate->opts.on_error_tbl));
+
+ /*
+ * we may insert tuples to error-saving table, to do that we need first
+ * check it's lock situation. If it is already under heavy lock, then
+ * our COPY operation would stuck. Instead of let COPY stuck, just
+ * report ERROR that the error-saving table is under heavy lock.
+ */
+ if (!ConditionalLockRelationOid(err_tbl_oid, RowExclusiveLock))
+ 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));
+ cstate->error_saving_rel = table_open(err_tbl_oid, NoLock);
+
+ /*
+ * can this error saving table (cstate->error_saving_rel) be used for
+ * error saving? for that we need to check the table's (column
+ * name, column data types, number of column)
+ *
+ */
+ arel = table_open(AttributeRelationId, AccessShareLock);
+ ScanKeyInit(&akey,
+ Anum_pg_attribute_attrelid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(err_tbl_oid));
+
+ ascan = systable_beginscan(arel, AttributeRelidNumIndexId, true,
+ SnapshotSelf, 1, &akey);
+ while (HeapTupleIsValid(atup = systable_getnext(ascan)))
+ {
+ attForm = (Form_pg_attribute) GETSTRUCT(atup);
+ if (attForm->attnum < 1 || attForm->attisdropped)
+ continue;
+
+ switch (attForm->attnum)
+ {
+ case 1:
+ if(attForm->atttypid == OIDOID &&
+ strcmp(NameStr(attForm->attname), "userid") == 0)
+ attcnt++;
+ break;
+ case 2:
+ if (attForm->atttypid == OIDOID &&
+ strcmp(NameStr(attForm->attname), "copy_tbl") == 0)
+ attcnt++;
+ break;
+ case 3:
+ if (attForm->atttypid == TEXTOID &&
+ strcmp(NameStr(attForm->attname), "filename") == 0)
+ attcnt++;
+ break;
+ case 4:
+ if (attForm->atttypid != INT8OID &&
+ strcmp(NameStr(attForm->attname), "lineno") == 0)
+ attcnt++;
+ break;
+ case 5:
+ if (attForm->atttypid == TEXTOID &&
+ strcmp(NameStr(attForm->attname), "line") == 0)
+ attcnt++;
+ break;
+ case 6:
+ if (attForm->atttypid == TEXTOID &&
+ strcmp(NameStr(attForm->attname), "colname") == 0)
+ attcnt++;
+ break;
+ case 7:
+ if (attForm->atttypid == TEXTOID &&
+ strcmp(NameStr(attForm->attname), "raw_field_value") == 0)
+ attcnt++;
+ break;
+ case 8:
+ if (attForm->atttypid == TEXTOID &&
+ strcmp(NameStr(attForm->attname), "err_message") == 0)
+ attcnt++;
+ break;
+ case 9:
+ if (attForm->atttypid == TEXTOID &&
+ strcmp(NameStr(attForm->attname), "err_detail") == 0)
+ attcnt++;
+ break;
+ case 10:
+ if (attForm->atttypid == TEXTOID &&
+ strcmp(NameStr(attForm->attname), "errorcode") == 0)
+ attcnt++;
+ break;
+ default:
+ attcnt++;
+ break;
+ }
+ }
+ systable_endscan(ascan);
+ table_close(arel, AccessShareLock);
+
+ if (attcnt != ERROR_TBL_COLUMNS)
+ 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));
+
+ 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 d1d43b53d8..e1606db94c 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"
@@ -959,7 +960,49 @@ NextCopyFrom(CopyFromState cstate, ExprContext *econtext,
&values[m]))
{
Assert(cstate->opts.on_error != COPY_ON_ERROR_STOP);
+ Assert(cstate->escontext->error_occurred);
+ 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 acquired
+ * lock on error_saving_rel in BeginCopyFrom.
+ */
+ 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);
+
+ on_error_tupDesc = RelationGetDescr(cstate->error_saving_rel);
+ 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++;
if (cstate->opts.log_verbosity == COPY_LOG_VERBOSITY_VERBOSE)
diff --git a/src/include/commands/copy.h b/src/include/commands/copy.h
index 4002a7f538..40ab35e4a3 100644
--- a/src/include/commands/copy.h
+++ b/src/include/commands/copy.h
@@ -38,8 +38,15 @@ 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;
+/*
+ * used for (COPY on_error 'table'); the error saving table saving error info
+ * only allow have 10 columns.
+*/
+#define ERROR_TBL_COLUMNS 10
+
/*
* Represents verbosity of logged messages by COPY command.
*/
@@ -86,6 +93,7 @@ typedef struct CopyFormatOptions
CopyOnErrorChoice on_error; /* what to do when error happened */
CopyLogVerbosityChoice log_verbosity; /* verbosity of logged messages */
int64 reject_limit; /* maximum tolerable number of errors */
+ char *on_error_tbl; /* on error, save error info to the table, table name */
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 64ea33aeae..1b477ad753 100644
--- a/src/test/regress/expected/copy2.out
+++ b/src/test/regress/expected/copy2.out
@@ -813,6 +813,109 @@ ERROR: skipped more than REJECT_LIMIT (3) rows due to data type incompatibility
CONTEXT: COPY check_ign_err, line 5, column n: ""
COPY check_ign_err FROM STDIN WITH (on_error ignore, reject_limit 4);
NOTICE: 4 rows were skipped due to data type incompatibility
+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
+);
+--cannot use for error saving.
+create table err_tbl_1(
+userid oid,
+copy_tbl oid,
+filename text,
+lineno bigint,
+line text,
+colname text,
+raw_field_value text,
+err_message text,
+err_detail text
+);
+--cannot use for error saving.
+create table err_tbl_2(
+ userid oid, copy_tbl oid, filename text, lineno bigint,line text,
+ colname text, raw_field_value text, err_message text,
+ err_detail text,
+ errorcode text,
+ errorcode1 text
+);
+create table t_copy_tbl(a int, b int, c int);
+create view s1 as select 1 as a;
+----invalid options, the below all should fails
+COPY t_copy_tbl FROM STDIN WITH (on_error 'table');
+ERROR: COPY ON_ERROR "table" requires a custom specified error saving table
+HINT: You need also specify "TABLE" option.
+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 (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(a,b) FROM STDIN WITH (on_error 'table', reject_limit 10, table err_tbl);
+ERROR: cannot specify REJECT_LIMIT option when ON_ERROR option is specified as "table"
+COPY t_copy_tbl(a,b) FROM STDIN WITH (on_error 'table', log_verbosity verbose, table err_tbl);
+ERROR: cannot specify log_verbosity as "verbose" 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
+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.
+--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. err_tbl_2 does not meet criteria
+COPY t_copy_tbl(a,b) FROM STDIN WITH (on_error 'table', table err_tbl_2);
+ERROR: table "public"."err_tbl_2" cannot be used for COPY FROM error saving
+DETAIL: Table "public"."err_tbl_2" data definition cannot be used for error saving
+----invalid options, the above all should fails
+--should fail, copied data have 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, copied data have 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 saved to table "err_tbl" due to data type incompatibility
+--should fail. lack privilege
+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 privilege 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 | 6 | 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
+(2 rows)
+
-- clean up
DROP TABLE forcetest;
DROP TABLE vistest;
@@ -831,6 +934,10 @@ 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 err_tbl_2;
+DROP TABLE t_copy_tbl CASCADE;
--
-- COPY FROM ... DEFAULT
--
diff --git a/src/test/regress/sql/copy2.sql b/src/test/regress/sql/copy2.sql
index 45273557ce..655a768945 100644
--- a/src/test/regress/sql/copy2.sql
+++ b/src/test/regress/sql/copy2.sql
@@ -588,6 +588,91 @@ a {7} 7
10 {10} 10
\.
+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
+);
+--cannot use for error saving.
+create table err_tbl_1(
+userid oid,
+copy_tbl oid,
+filename text,
+lineno bigint,
+line text,
+colname text,
+raw_field_value text,
+err_message text,
+err_detail text
+);
+--cannot use for error saving.
+create table err_tbl_2(
+ userid oid, copy_tbl oid, filename text, lineno bigint,line text,
+ colname text, raw_field_value text, err_message text,
+ err_detail text,
+ errorcode text,
+ errorcode1 text
+);
+create table t_copy_tbl(a int, b int, c int);
+create view s1 as select 1 as a;
+
+----invalid options, the below all should fails
+COPY t_copy_tbl FROM STDIN WITH (on_error 'table');
+COPY t_copy_tbl FROM STDIN WITH (table err_tbl);
+COPY t_copy_tbl TO STDIN WITH (on_error 'table');
+COPY t_copy_tbl(a,b) FROM STDIN WITH (on_error 'table', reject_limit 10, table err_tbl);
+COPY t_copy_tbl(a,b) FROM STDIN WITH (on_error 'table', log_verbosity verbose, table err_tbl);
+COPY t_copy_tbl(a,b) FROM STDIN WITH (on_error 'table', table not_exists);
+COPY t_copy_tbl(a) FROM STDIN WITH (on_error 'table', table 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. err_tbl_2 does not meet criteria
+COPY t_copy_tbl(a,b) FROM STDIN WITH (on_error 'table', table err_tbl_2);
+----invalid options, the above all should fails
+
+
+--should fail, copied data have extra columns
+COPY t_copy_tbl(a,b) FROM STDIN WITH (DELIMITER ',', on_error 'table', table err_tbl);
+1,2,3,4
+\.
+
+--should fail, copied data have 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
+1,11,4238679732489879879
+\.
+
+--should fail. lack privilege
+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;
@@ -606,6 +691,10 @@ 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 err_tbl_2;
+DROP TABLE t_copy_tbl CASCADE;
--
-- COPY FROM ... DEFAULT
--
2.34.1
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: on_error table, saving error info to a table
2024-02-03 06:22 on_error table, saving error info to a table jian he <[email protected]>
2024-07-15 05:42 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
2024-08-20 00:00 ` Re: on_error table, saving error info to a table jian he <[email protected]>
2024-11-05 10:30 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
2024-12-03 04:28 ` Re: on_error table, saving error info to a table jian he <[email protected]>
2024-12-03 10:28 ` Re: on_error table, saving error info to a table Kirill Reshke <[email protected]>
2024-12-11 11:41 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
2024-12-13 08:26 ` Re: on_error table, saving error info to a table jian he <[email protected]>
@ 2024-12-16 11:50 ` Nishant Sharma <[email protected]>
2024-12-17 04:31 ` Re: on_error table, saving error info to a table Kirill Reshke <[email protected]>
0 siblings, 1 reply; 13+ messages in thread
From: Nishant Sharma @ 2024-12-16 11:50 UTC (permalink / raw)
To: jian he <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>; Kirill Reshke <[email protected]>
On Fri, Dec 13, 2024 at 1:57 PM jian he <[email protected]> wrote:
> On Wed, Dec 11, 2024 at 7:41 PM Nishant Sharma
> <[email protected]> wrote:
> >
> > Thanks for the v3 patch!
> >
> > Please find review comments on v3:-
> >
> > 1) I think no need to change the below if condition, we can keep
> > it the way it was before i.e with
> > "cstate->opts.on_error != COPY_ON_ERROR_STOP" and we
> > add a new error ereport the way v3 has. Because for
> > cstate->opts.on_error as COPY_ON_ERROR_STOP cases we
> > can avoid two if conditions inside upper if.
> >
> > + if (cstate->num_errors > 0 &&
> > cstate->opts.log_verbosity >= COPY_LOG_VERBOSITY_DEFAULT)
>
> > 2) No need for the below "if" check for maxattnum. We can simply
> > increment it with "++maxattnum" and later check if we have exactly
> > 10 attributes for the error table. Because even if we drop any
> > attribute and maxattnum is 10 in pg_attribute for that rel, we should
> > still error out. Maybe we can rename it to "totalatts"?
> >
> > + if (maxattnum <= attForm->attnum)
> > + maxattnum = attForm->attnum;
> >
> > 3) #define would be better, also as mentioned by Kirill switch
> > condition with proper #define would be better.
> >
> > + if (maxattnum != 10)
> > + on_error_tbl_ok = false;
> >
> > 4)
>
> hi. Thanks for the review.
> The attached v4 patch addressed these two issues.
>
> > > that would be more work.
> > > so i stick to if there is a table can use to
> > > error saving then use it, otherwise error out.
> > >
> > YES. but that would lead to a better design with an error table.
> > Also, I think Krill mentions the same. That is to auto create, if it
> > does not exist.
> >
> I decided not to auto-create the table.
> main reason not to do it:
> 1. utility COPY command with another SPI utility CREATE TABLE command may
> work.
> but there is no precedent.
>
> 2. if we auto-create the on_error table with BeginCopyFrom.
> then later we have to use get_relname_relid to get the newly created table
> Oid,
> I think it somehow counts as repeating name lookups, see relevant
> linke [1], [2].
>
> [1] https://postgr.es/m/[email protected]
> [2]
> https://postgr.es/m/CA+TgmobHYix=Nn8D4RUHa6fhUVPR88KGAMq1pBfnGfOfEjRixA@mail.gmail.com
Thanks for the v4 patch!
Review comment on v4:-
1) The new switch logic does not look correct to me. It will pass for
a failing scenario. I think you can use v3's logic instead with below
changes:-
a)
while (HeapTupleIsValid(atup = systable_getnext(ascan))) -->
while (HeapTupleIsValid(atup = systable_getnext(ascan)) && on_error_tbl_ok)
b)
attcnt++; --> just before the "switch (attForm->attnum)".
Thats it.
Also, I think Andrew's suggestion can resolve the concern me and Krill
had on forcing users to create tables with correct column names and
numbers. Also, will make error table checking simpler. No need for the
above kind of checks.
Regards,
Nishant.
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: on_error table, saving error info to a table
2024-02-03 06:22 on_error table, saving error info to a table jian he <[email protected]>
2024-07-15 05:42 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
2024-08-20 00:00 ` Re: on_error table, saving error info to a table jian he <[email protected]>
2024-11-05 10:30 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
2024-12-03 04:28 ` Re: on_error table, saving error info to a table jian he <[email protected]>
2024-12-03 10:28 ` Re: on_error table, saving error info to a table Kirill Reshke <[email protected]>
2024-12-11 11:41 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
2024-12-13 08:26 ` Re: on_error table, saving error info to a table jian he <[email protected]>
2024-12-16 11:50 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
@ 2024-12-17 04:31 ` Kirill Reshke <[email protected]>
2025-04-25 13:46 ` Re: on_error table, saving error info to a table jian he <[email protected]>
0 siblings, 1 reply; 13+ messages in thread
From: Kirill Reshke @ 2024-12-17 04:31 UTC (permalink / raw)
To: Nishant Sharma <[email protected]>; +Cc: jian he <[email protected]>; PostgreSQL Hackers <[email protected]>
On Mon, 16 Dec 2024 at 16:50, Nishant Sharma
<[email protected]> wrote:
> Also, I think Andrew's suggestion can resolve the concern me and Krill
> had on forcing users to create tables with correct column names and
> numbers. Also, will make error table checking simpler. No need for the
> above kind of checks.
+1 on that.
--
Best regards,
Kirill Reshke
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: on_error table, saving error info to a table
2024-02-03 06:22 on_error table, saving error info to a table jian he <[email protected]>
2024-07-15 05:42 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
2024-08-20 00:00 ` Re: on_error table, saving error info to a table jian he <[email protected]>
2024-11-05 10:30 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
2024-12-03 04:28 ` Re: on_error table, saving error info to a table jian he <[email protected]>
2024-12-03 10:28 ` Re: on_error table, saving error info to a table Kirill Reshke <[email protected]>
2024-12-11 11:41 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
2024-12-13 08:26 ` Re: on_error table, saving error info to a table jian he <[email protected]>
2024-12-16 11:50 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
2024-12-17 04:31 ` Re: on_error table, saving error info to a table Kirill Reshke <[email protected]>
@ 2025-04-25 13:46 ` jian he <[email protected]>
2026-05-25 08:13 ` Re: on_error table, saving error info to a table jian he <[email protected]>
0 siblings, 1 reply; 13+ messages in thread
From: jian he @ 2025-04-25 13:46 UTC (permalink / raw)
To: Kirill Reshke <[email protected]>; +Cc: Nishant Sharma <[email protected]>; PostgreSQL Hackers <[email protected]>
On Mon, Dec 16, 2024 at 7:50 PM Nishant Sharma
<[email protected]> wrote:
>
>
> 1) The new switch logic does not look correct to me. It will pass for
> a failing scenario. I think you can use v3's logic instead with below
> changes:-
>
> a)
> while (HeapTupleIsValid(atup = systable_getnext(ascan))) -->
> while (HeapTupleIsValid(atup = systable_getnext(ascan)) && on_error_tbl_ok)
>
> b)
> attcnt++; --> just before the "switch (attForm->attnum)".
>
> Thats it.
>
You are right about this.
On Tue, Dec 17, 2024 at 12:31 PM Kirill Reshke <[email protected]> wrote:
>
> On Mon, 16 Dec 2024 at 16:50, Nishant Sharma
> <[email protected]> wrote:
> > Also, I think Andrew's suggestion can resolve the concern me and Krill
> > had on forcing users to create tables with correct column names and
> > numbers. Also, will make error table checking simpler. No need for the
> > above kind of checks.
>
> +1 on that.
>
Syntax: COPY (on_error table, table error_saving_tbl);
seems not ideal.
but auto-create on_error table if this table does not exist, seems way
more harder.
Since we can not use SPI interface here, maybe we can use DefineRelation
also, to auto-create a table, what if table already exists, then
our operation would be stuck for not COPY related reason.
also auto-create means we need to come up with a magic table name for
all COPY (on_error table)
operations, which seems not ideal IMO.
i realized we should error out case like:
COPY err_tbl FROM STDIN WITH (DELIMITER ',', on_error table, table err_tbl);
also by changing copy_generic_opt_arg, now we can
COPY err_tbl FROM STDIN WITH (DELIMITER ',', on_error table, table x);
previously, we can only do
COPY err_tbl FROM STDIN WITH (DELIMITER ',', on_error 'table', table x);
Attachments:
[text/x-patch] v5-0001-COPY-FROM-on_error-table.patch (32.4K, 2-v5-0001-COPY-FROM-on_error-table.patch)
download | inline diff:
From 4d5458c1c5de85b9f03c22727b92aec45f0cd73d Mon Sep 17 00:00:00 2001
From: jian he <[email protected]>
Date: Fri, 25 Apr 2025 21:43:39 +0800
Subject: [PATCH v5 1/1] COPY FROM (on_error table)
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 error out.
we also did preliminary check the lock of error saving table so the insert to
error saving table won't stuck.
once there is a error happened, we save the error metedata and insert it to the
error_saving_table. and continue to the next row. That means for one row, we
can only catch the first field that have errors.
discussion: https://postgr.es/m/CACJufxH_OJpVra%3D0c4ow8fbxHj7heMcVaTNEPa5vAurSeNA-6Q%40mail.gmail.com
context: https://www.postgresql.org/message-id/752672.1699474336%40sss.pgh.pa.us
commitfest: https://commitfest.postgresql.org/51/4817/
---
doc/src/sgml/ref/copy.sgml | 119 ++++++++++++-
src/backend/commands/copy.c | 29 ++++
src/backend/commands/copyfrom.c | 202 ++++++++++++++++++++++-
src/backend/commands/copyfromparse.c | 50 +++++-
src/backend/parser/gram.y | 1 +
src/include/commands/copy.h | 8 +
src/include/commands/copyfrom_internal.h | 1 +
src/test/regress/expected/copy2.out | 101 ++++++++++++
src/test/regress/sql/copy2.sql | 84 ++++++++++
9 files changed, 582 insertions(+), 13 deletions(-)
diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml
index 8433344e5b6..b627d422b38 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_table</replaceable>
REJECT_LIMIT <replaceable class="parameter">maxerror</replaceable>
ENCODING '<replaceable class="parameter">encoding_name</replaceable>'
LOG_VERBOSITY <replaceable class="parameter">verbosity</replaceable>
@@ -395,15 +396,25 @@ 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 save error details to <replaceable class="parameter">error_saving_table</replaceable>
+ and continue with the next one.
The default is <literal>stop</literal>.
</para>
<para>
- The <literal>ignore</literal> option is applicable only for <command>COPY FROM</command>
+ The <literal>ignore</literal> and <literal>table</literal> option are applicable only for <command>COPY FROM</command>
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
+ If <literal>ON_ERROR</literal>=<literal>table</literal>,
+ a <literal>NOTICE</literal> message containing the row count that is saved to
+ <replaceable class="parameter">error_saving_table</replaceable> is
+ emitted at the end of the <command>COPY FROM</command> if at least one
+ row was saved.
+ </para>
+
+ <para>
+ If <literal>ON_ERROR</literal>=<literal>ignore</literal>, 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>verbose</literal>, a <literal>NOTICE</literal> message
@@ -463,6 +474,108 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>TABLE</literal></term>
+ <listitem>
+ <para>
+ Save error context details to the table <replaceable class="parameter">error_saving_table</replaceable>.
+ This option is allowed only in <command>COPY FROM</command> and
+ <literal>ON_ERROR</literal> is specified with <literal>TABLE</literal>.
+ It also require user have <literal>INSERT</literal> privileges on all columns
+ in the <replaceable class="parameter">error_saving_table</replaceable>.
+ </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), 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 74ae42b19a7..a5492214117 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 */
@@ -502,6 +504,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;
bool reject_limit_specified = false;
ListCell *option;
@@ -677,6 +680,13 @@ ProcessCopyOptions(ParseState *pstate,
reject_limit_specified = true;
opts_out->reject_limit = defGetCopyRejectLimitOption(defel);
}
+ 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),
@@ -685,6 +695,25 @@ ProcessCopyOptions(ParseState *pstate,
parser_errposition(pstate, defel->location)));
}
+ if (opts_out->on_error == COPY_ON_ERROR_TABLE)
+ {
+ if (opts_out->on_error_tbl == NULL)
+ ereport(ERROR,
+ errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("cannot specify %s option value to \"%s\" when %s is not specified", "ON_ERROR", "TABLE", "TABLE"),
+ errhint("You may need also specify \"%s\" option.", "TABLE"));
+
+ if (opts_out->reject_limit)
+ ereport(ERROR,
+ errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("cannot specify %s option when %s option is specified as \"%s\"", "REJECT_LIMIT", "ON_ERROR", "TABLE"));
+ }
+
+ if (opts_out->on_error != COPY_ON_ERROR_TABLE && opts_out->on_error_tbl != NULL)
+ ereport(ERROR,
+ errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("COPY %s option can only be used when %s option is specified as \"%s\"", "TABLE", "ON_ERROR", "TABLE"));
+
/*
* Check for incompatible options (must do these three before inserting
* defaults)
diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c
index fbbbc09a97b..91816a7f781 100644
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -44,7 +44,10 @@
#include "pgstat.h"
#include "rewrite/rewriteHandler.h"
#include "storage/fd.h"
+#include "storage/lmgr.h"
#include "tcop/tcopprot.h"
+#include "utils/builtins.h"
+#include "utils/fmgroids.h"
#include "utils/lsyscache.h"
#include "utils/memutils.h"
#include "utils/portal.h"
@@ -1175,6 +1178,16 @@ CopyFrom(CopyFromState cstate)
continue;
}
+ if (cstate->opts.on_error == COPY_ON_ERROR_TABLE &&
+ cstate->escontext->error_occurred)
+ {
+ cstate->escontext->error_occurred = false;
+ cstate->escontext->details_wanted = true;
+ memset(cstate->escontext->error_data, 0, sizeof(ErrorData));
+ /* Repeat NextCopyFrom() until no soft error occurs */
+ continue;
+ }
+
ExecStoreVirtualTuple(myslot);
/*
@@ -1467,14 +1480,31 @@ CopyFrom(CopyFromState cstate)
/* Done, clean up */
error_context_stack = errcallback.previous;
- if (cstate->opts.on_error != COPY_ON_ERROR_STOP &&
- cstate->num_errors > 0 &&
+ if (cstate->num_errors > 0 &&
cstate->opts.log_verbosity >= COPY_LOG_VERBOSITY_DEFAULT)
- ereport(NOTICE,
- errmsg_plural("%" PRIu64 " row was skipped due to data type incompatibility",
- "%" PRIu64 " rows were skipped due to data type incompatibility",
- cstate->num_errors,
- cstate->num_errors));
+ {
+ if (cstate->opts.on_error == COPY_ON_ERROR_IGNORE)
+ ereport(NOTICE,
+ errmsg_plural("%" PRIu64 " row was skipped due to data type incompatibility",
+ "%" PRIu64 " rows were skipped due to data type incompatibility",
+ cstate->num_errors,
+ cstate->num_errors));
+ else if (cstate->opts.on_error == COPY_ON_ERROR_TABLE)
+ ereport(NOTICE,
+ errmsg_plural("%" PRIu64 " row was saved to table \"%s\" due to data type incompatibility",
+ "%" PRIu64 " rows were saved to table \"%s\" due to data type incompatibility",
+ cstate->num_errors,
+ cstate->num_errors,
+ RelationGetRelationName(cstate->error_saving_rel)));
+ }
+
+ /*
+ * similar to
+ * (https://postgr.es/m/[email protected])
+ * 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);
@@ -1622,15 +1652,169 @@ BeginCopyFrom(ParseState *pstate,
cstate->escontext->error_occurred = false;
/*
- * Currently we only support COPY_ON_ERROR_IGNORE. We'll add other
- * options later
+ * Currently we only support COPY_ON_ERROR_IGNORE, COPY_ON_ERROR_TABLE.
+ * We'll add other options later.
*/
if (cstate->opts.on_error == COPY_ON_ERROR_IGNORE)
cstate->escontext->details_wanted = false;
+ else if (cstate->opts.on_error == COPY_ON_ERROR_TABLE)
+ cstate->escontext->details_wanted = true;
}
else
cstate->escontext = NULL;
+ if (cstate->opts.on_error == COPY_ON_ERROR_TABLE)
+ {
+ Datum ins_prev;
+ Oid err_tbl_oid;
+ Relation pg_attribute;
+ ScanKeyData scankey;
+ SysScanDesc scan;
+ HeapTuple atup;
+ Form_pg_attribute attForm;
+ int attcnt = 0;
+ bool on_error_tbl_ok = true;
+
+ Assert(cstate->opts.on_error_tbl != NULL);
+
+ err_tbl_oid = RelnameGetRelid(cstate->opts.on_error_tbl);
+ if (!OidIsValid(err_tbl_oid))
+ ereport(ERROR,
+ errcode(ERRCODE_UNDEFINED_TABLE),
+ errmsg("relation \"%s\" does not exist",
+ cstate->opts.on_error_tbl));
+
+ if (RelationGetRelid(cstate->rel) == err_tbl_oid)
+ ereport(ERROR,
+ errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("cannot use relation \"%s\" for COPY error saving while copying data to it",
+ cstate->opts.on_error_tbl));
+
+ /* error saving table must be a regular realtion */
+ if (get_rel_relkind(err_tbl_oid) != RELKIND_RELATION)
+ ereport(ERROR,
+ errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot use relation \"%s\" for COPY error saving",
+ cstate->opts.on_error_tbl),
+ errdetail_relkind_not_supported(get_rel_relkind(err_tbl_oid)));
+
+ /*
+ * we may insert tuples to error-saving table later, to do that we need
+ * first check it's lock situation. If it is already under heavy lock,
+ * then our COPY operation would stuck. Instead of let COPY FROM stuck,
+ * just error report the error saving table is under heavy lock.
+ */
+ if (!ConditionalLockRelationOid(err_tbl_oid, RowExclusiveLock))
+ ereport(ERROR,
+ errcode(ERRCODE_OBJECT_IN_USE),
+ errmsg("can not use table \"%s\" for error saving because it was being locked",
+ cstate->opts.on_error_tbl));
+
+ cstate->error_saving_rel = table_open(err_tbl_oid, RowExclusiveLock);
+
+ /* current user should have INSERT privilege on error_saving table */
+ ins_prev = DirectFunctionCall3(has_table_privilege_id_id,
+ ObjectIdGetDatum(GetUserId()),
+ ObjectIdGetDatum(err_tbl_oid),
+ CStringGetTextDatum("INSERT"));
+ if (!DatumGetBool(ins_prev))
+ ereport(ERROR,
+ errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("permission denied to set table \"%s\" for COPY FROM error saving",
+ RelationGetRelationName(cstate->error_saving_rel)),
+ errhint("Ensure current user have enough privilege on \"%s\" for COPY FROM error saving",
+ RelationGetRelationName(cstate->error_saving_rel)));
+
+ /*
+ * Verify whether the definition of the table
+ * (cstate->error_saving_rel)— including column names, data types, and
+ * the number of columns— is suitable for use in error saving.
+ */
+ pg_attribute = table_open(AttributeRelationId, AccessShareLock);
+ ScanKeyInit(&scankey,
+ Anum_pg_attribute_attrelid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(err_tbl_oid));
+
+ scan = systable_beginscan(pg_attribute, AttributeRelidNumIndexId, true,
+ SnapshotSelf, 1, &scankey);
+ while (HeapTupleIsValid(atup = systable_getnext(scan)) && on_error_tbl_ok)
+ {
+ attForm = (Form_pg_attribute) GETSTRUCT(atup);
+
+ if (attForm->attnum < 1 || attForm->attisdropped)
+ continue;
+
+ attcnt++;
+ switch (attForm->attnum)
+ {
+ case 1:
+ if (attForm->atttypid != OIDOID ||
+ strcmp(NameStr(attForm->attname), "userid") != 0)
+ on_error_tbl_ok = false;
+ break;
+ case 2:
+ if (attForm->atttypid != OIDOID ||
+ strcmp(NameStr(attForm->attname), "copy_tbl") != 0)
+ on_error_tbl_ok = false;
+ break;
+ case 3:
+ if (attForm->atttypid != TEXTOID ||
+ strcmp(NameStr(attForm->attname), "filename") != 0)
+ on_error_tbl_ok = false;
+ break;
+ case 4:
+ if (attForm->atttypid != INT8OID ||
+ strcmp(NameStr(attForm->attname), "lineno") != 0)
+ on_error_tbl_ok = false;
+ break;
+ case 5:
+ if (attForm->atttypid != TEXTOID ||
+ strcmp(NameStr(attForm->attname), "line") != 0)
+ on_error_tbl_ok = false;
+ break;
+ case 6:
+ if (attForm->atttypid != TEXTOID ||
+ strcmp(NameStr(attForm->attname), "colname") != 0)
+ on_error_tbl_ok = false;
+ break;
+ case 7:
+ if (attForm->atttypid != TEXTOID ||
+ strcmp(NameStr(attForm->attname), "raw_field_value") != 0)
+ on_error_tbl_ok = false;
+ break;
+ case 8:
+ if (attForm->atttypid != TEXTOID ||
+ strcmp(NameStr(attForm->attname), "err_message") != 0)
+ on_error_tbl_ok = false;
+ break;
+ case 9:
+ if (attForm->atttypid != TEXTOID ||
+ strcmp(NameStr(attForm->attname), "err_detail") != 0)
+ on_error_tbl_ok = false;
+ break;
+ case 10:
+ if (attForm->atttypid != TEXTOID ||
+ strcmp(NameStr(attForm->attname), "errorcode") != 0)
+ on_error_tbl_ok = false;
+ break;
+ default:
+ on_error_tbl_ok = false;
+ break;
+ }
+ }
+ systable_endscan(scan);
+ table_close(pg_attribute, AccessShareLock);
+
+ if (attcnt != ERROR_TBL_COLUMNS || !on_error_tbl_ok)
+ ereport(ERROR,
+ errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("table \"%s\" cannot be used for COPY error saving",
+ RelationGetRelationName(cstate->error_saving_rel)),
+ errdetail("Table \"%s\" data definition is not suitable for error saving",
+ RelationGetRelationName(cstate->error_saving_rel)));
+ }
+
/* 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 f5fc346e201..34acacf8660 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -62,6 +62,7 @@
#include <unistd.h>
#include <sys/stat.h>
+#include "access/heapam.h"
#include "commands/copyapi.h"
#include "commands/copyfrom_internal.h"
#include "commands/progress.h"
@@ -78,6 +79,8 @@
#define ISOCTAL(c) (((c) >= '0') && ((c) <= '7'))
#define OCTVALUE(c) ((c) - '0')
+#define ERROR_TBL_COLUMNS 10
+
/*
* These macros centralize code used to process line_buf and input_buf buffers.
* They are macros because they often do continue/break control and to avoid
@@ -1035,9 +1038,54 @@ CopyFromTextLikeOneRow(CopyFromState cstate, ExprContext *econtext,
{
Assert(cstate->opts.on_error != COPY_ON_ERROR_STOP);
+ if (cstate->opts.on_error == COPY_ON_ERROR_TABLE)
+ {
+ /*
+ * We use ErrorSaveContext to form a tuple and insert it to the
+ * error saving table. A RowExclusiveLock on error_saving_rel
+ * was already acquired in BeginCopyFrom.
+ */
+ HeapTuple tuple;
+ TupleDesc tupdesc;
+ char *err_detail;
+ char *err_code;
+ Datum values[ERROR_TBL_COLUMNS] = {0};
+ bool isnull[ERROR_TBL_COLUMNS] = {0};
+ int j = 0;
+
+ Assert(cstate->rel != NULL);
+ Assert(cstate->escontext->error_occurred);
+
+ values[j++] = ObjectIdGetDatum(GetUserId());
+ values[j++] = ObjectIdGetDatum(cstate->rel->rd_rel->oid);
+ values[j++] = CStringGetTextDatum(cstate->filename ? cstate->filename : "STDIN");
+ values[j++] = Int64GetDatum((long long) cstate->cur_lineno);
+ values[j++] = CStringGetTextDatum(cstate->line_buf.data);
+ values[j++] = CStringGetTextDatum(cstate->cur_attname);
+ values[j++] = CStringGetTextDatum(string);
+ 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;
+ values[j] = err_detail ? CStringGetTextDatum(err_detail) : (Datum) 0;
+ isnull[j++] = err_detail ? false : true;
+
+ err_code = unpack_sql_state(cstate->escontext->error_data->sqlerrcode);
+ values[j++] = CStringGetTextDatum(err_code);
+
+ Assert(j == ERROR_TBL_COLUMNS);
+
+ tupdesc = RelationGetDescr(cstate->error_saving_rel);
+ tuple = heap_form_tuple(tupdesc, values, isnull);
+ simple_heap_insert(cstate->error_saving_rel, tuple);
+ }
+
cstate->num_errors++;
- if (cstate->opts.log_verbosity == COPY_LOG_VERBOSITY_VERBOSE)
+ if (cstate->opts.log_verbosity == COPY_LOG_VERBOSITY_VERBOSE &&
+ cstate->opts.on_error == COPY_ON_ERROR_IGNORE)
{
/*
* Since we emit line number and column info in the below
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 3c4268b271a..0a45305c4b4 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -3577,6 +3577,7 @@ copy_generic_opt_arg:
| NumericOnly { $$ = (Node *) $1; }
| '*' { $$ = (Node *) makeNode(A_Star); }
| DEFAULT { $$ = (Node *) makeString("default"); }
+ | TABLE { $$ = (Node *) makeString("table"); }
| '(' copy_generic_opt_arg_list ')' { $$ = (Node *) $2; }
| /* EMPTY */ { $$ = NULL; }
;
diff --git a/src/include/commands/copy.h b/src/include/commands/copy.h
index 06dfdfef721..0c7a2defc51 100644
--- a/src/include/commands/copy.h
+++ b/src/include/commands/copy.h
@@ -38,8 +38,15 @@ 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;
+/*
+ * used for (COPY on_error 'table'); the error saving table have only 10
+ * columns.
+*/
+#define ERROR_TBL_COLUMNS 10
+
/*
* Represents verbosity of logged messages by COPY command.
*/
@@ -86,6 +93,7 @@ typedef struct CopyFormatOptions
CopyOnErrorChoice on_error; /* what to do when error happened */
CopyLogVerbosityChoice log_verbosity; /* verbosity of logged messages */
int64 reject_limit; /* maximum tolerable number of errors */
+ char *on_error_tbl; /* on error, save error info to the table, table name */
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 c8b22af22d8..c974311f6bf 100644
--- a/src/include/commands/copyfrom_internal.h
+++ b/src/include/commands/copyfrom_internal.h
@@ -73,6 +73,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 64ea33aeae8..67c15d8849d 100644
--- a/src/test/regress/expected/copy2.out
+++ b/src/test/regress/expected/copy2.out
@@ -813,6 +813,103 @@ ERROR: skipped more than REJECT_LIMIT (3) rows due to data type incompatibility
CONTEXT: COPY check_ign_err, line 5, column n: ""
COPY check_ign_err FROM STDIN WITH (on_error ignore, reject_limit 4);
NOTICE: 4 rows were skipped due to data type incompatibility
+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
+);
+--cannot use for error saving.
+create table err_tbl_1(
+userid oid, copy_tbl oid, filename text, lineno bigint, line text,
+colname text, raw_field_value text,
+err_message text,
+err_detail text);
+--cannot use for error saving.
+create table err_tbl_2(
+userid oid, copy_tbl oid, filename text, lineno bigint,line text,
+colname text, raw_field_value text, err_message text,
+err_detail text,
+errorcode text,
+errorcode1 text
+);
+create table t_copy_tbl(a int, b int, c int);
+create view s1 as select 1 as a;
+----invalid options, the below all should fails
+COPY err_tbl FROM STDIN WITH (DELIMITER ',', on_error table, table err_tbl);
+ERROR: cannot use relation "err_tbl" for COPY error saving while copying data to it
+COPY t_copy_tbl FROM STDIN WITH (on_error table);
+ERROR: cannot specify ON_ERROR option value to "TABLE" when TABLE is not specified
+HINT: You may need also specify "TABLE" option.
+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 (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(a,b) FROM STDIN WITH (on_error table, reject_limit 10, table err_tbl);
+ERROR: cannot specify REJECT_LIMIT option when ON_ERROR option is specified as "TABLE"
+COPY t_copy_tbl(a,b) FROM STDIN WITH (on_error table, table not_exists);
+ERROR: relation "not_exists" does not exist
+COPY t_copy_tbl(a) FROM STDIN WITH (on_error table, table s1);
+ERROR: cannot use relation "s1" for COPY error saving
+DETAIL: This operation is not supported for views.
+--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 "err_tbl_1" cannot be used for COPY error saving
+DETAIL: Table "err_tbl_1" data definition is not suitable for error saving
+--should fail. err_tbl_2 does not meet criteria
+COPY t_copy_tbl(a,b) FROM STDIN WITH (on_error table, table err_tbl_2);
+ERROR: table "err_tbl_2" cannot be used for COPY error saving
+DETAIL: Table "err_tbl_2" data definition is not suitable for error saving
+----invalid options, the above all should fails
+--should fail, copied data have 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, copied data have 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 saved to table "err_tbl" due to data type incompatibility
+--should fail. lack privilege
+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 "err_tbl" for COPY FROM error saving
+HINT: Ensure current user have enough privilege on "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 | 6 | 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
+(2 rows)
+
-- clean up
DROP TABLE forcetest;
DROP TABLE vistest;
@@ -831,6 +928,10 @@ 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 err_tbl_2;
+DROP TABLE t_copy_tbl CASCADE;
--
-- COPY FROM ... DEFAULT
--
diff --git a/src/test/regress/sql/copy2.sql b/src/test/regress/sql/copy2.sql
index 45273557ce0..a190432682c 100644
--- a/src/test/regress/sql/copy2.sql
+++ b/src/test/regress/sql/copy2.sql
@@ -588,6 +588,86 @@ a {7} 7
10 {10} 10
\.
+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
+);
+--cannot use for error saving.
+create table err_tbl_1(
+userid oid, copy_tbl oid, filename text, lineno bigint, line text,
+colname text, raw_field_value text,
+err_message text,
+err_detail text);
+
+--cannot use for error saving.
+create table err_tbl_2(
+userid oid, copy_tbl oid, filename text, lineno bigint,line text,
+colname text, raw_field_value text, err_message text,
+err_detail text,
+errorcode text,
+errorcode1 text
+);
+create table t_copy_tbl(a int, b int, c int);
+create view s1 as select 1 as a;
+
+----invalid options, the below all should fails
+COPY err_tbl FROM STDIN WITH (DELIMITER ',', on_error table, table err_tbl);
+COPY t_copy_tbl FROM STDIN WITH (on_error table);
+COPY t_copy_tbl FROM STDIN WITH (table err_tbl);
+COPY t_copy_tbl TO STDIN WITH (on_error table);
+COPY t_copy_tbl(a,b) FROM STDIN WITH (on_error table, reject_limit 10, table err_tbl);
+COPY t_copy_tbl(a,b) FROM STDIN WITH (on_error table, table not_exists);
+COPY t_copy_tbl(a) FROM STDIN WITH (on_error table, table 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. err_tbl_2 does not meet criteria
+COPY t_copy_tbl(a,b) FROM STDIN WITH (on_error table, table err_tbl_2);
+----invalid options, the above all should fails
+
+--should fail, copied data have extra columns
+COPY t_copy_tbl(a,b) FROM STDIN WITH (DELIMITER ',', on_error table, table err_tbl);
+1,2,3,4
+\.
+
+--should fail, copied data have 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
+1,11,4238679732489879879
+\.
+
+--should fail. lack privilege
+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;
@@ -606,6 +686,10 @@ 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 err_tbl_2;
+DROP TABLE t_copy_tbl CASCADE;
--
-- COPY FROM ... DEFAULT
--
2.34.1
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: on_error table, saving error info to a table
2024-02-03 06:22 on_error table, saving error info to a table jian he <[email protected]>
2024-07-15 05:42 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
2024-08-20 00:00 ` Re: on_error table, saving error info to a table jian he <[email protected]>
2024-11-05 10:30 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
2024-12-03 04:28 ` Re: on_error table, saving error info to a table jian he <[email protected]>
2024-12-03 10:28 ` Re: on_error table, saving error info to a table Kirill Reshke <[email protected]>
2024-12-11 11:41 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
2024-12-13 08:26 ` Re: on_error table, saving error info to a table jian he <[email protected]>
2024-12-16 11:50 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
2024-12-17 04:31 ` Re: on_error table, saving error info to a table Kirill Reshke <[email protected]>
2025-04-25 13:46 ` Re: on_error table, saving error info to a table jian he <[email protected]>
@ 2026-05-25 08:13 ` jian he <[email protected]>
2026-05-28 22:41 ` Re: on_error table, saving error info to a table Zsolt Parragi <[email protected]>
0 siblings, 1 reply; 13+ messages in thread
From: jian he @ 2026-05-25 08:13 UTC (permalink / raw)
To: Zsolt Parragi <[email protected]>; +Cc: [email protected]
On Fri, May 15, 2026 at 7:32 PM Zsolt Parragi <[email protected]> wrote:
>
> > I am not sure.
> > Should we produce the NOTICE message below for the above test case?
> > +NOTICE: 5 rows were saved to table "err_tbl2" due to data type incompatibility
> > but because of the trigger, err_tbl2 has zero rows.
>
> I am also not sure about the exact details, however silently dropping
> rows from both the target and error table seems wrong to me, so I
> would definitely add some output about this happening.
>
With v13, I have added:
+ <para>
+ Triggers on <replaceable
class="parameter">error_saving_table</replaceable>
+ will be fired. Therefore, the <literal>NOTICE</literal> message regarding
+ rows inserted into <replaceable
class="parameter">error_saving_table</replaceable>
+ may differ from what is actually being inserted.
+ </para>
> + /* Prepare to build the result tuple */
> + TupleTableSlot *myslot = ExecGetReturningSlot(estate,
> + mtstate->resultRelInfo);
>
>
> Is reusing the returning slot for this the proper way to do it? I'm
> not saying that it's wrong, but I'm unsure about this.
Searching the codebase for ExecGetReturningSlot or ri_ReturningSlot shows that
this is its only call site for CopyFromState->error_rel
(Note that in ExecInsert, resultRelInfo->ri_projectReturning is NULL
for the CopyFromState->error_rel).
In ExecInsert, we also have these comments:
---------
* Using ExecGetReturningSlot() to store the tuple for the
* recheck isn't that pretty, but we can't trivially use
* the input slot, because it might not be of a compatible
* type. As there's no conflicting usage of
* ExecGetReturningSlot() in the DO NOTHING case...
---------
Therefore I think it's safe to reuse ResultRelInfo->ri_ReturningSlot.
Regarding the trigger behavior, with v13:
Statement-level and row-level triggers will fire for every error
record insertion, which behaves very similarly to
ExecForPortionOfLeftovers.
I have attached version 13, which should resolve the rest of the
issues you raised.
--
jian
https://www.enterprisedb.com/
Attachments:
[text/x-patch] v13-0002-COPY-FROM-on_error-table-error_table-errtbl.patch (56.8K, 2-v13-0002-COPY-FROM-on_error-table-error_table-errtbl.patch)
download | inline diff:
From c66b7484315ece47a905efae33ba88ce9ffa2e81 Mon Sep 17 00:00:00 2001
From: jian he <[email protected]>
Date: Mon, 25 May 2026 16:05:52 +0800
Subject: [PATCH v13 2/2] COPY FROM (on_error table, error_table errtbl)
The COPY syntax is {on_error table, error_table 'your_defined_error_saving_tbl'}.
The error_saving_tbl must be a typed table, based on internal type
pg_catalog.copy_error_saving. A preliminary lock check is also performed on the
error-saving table to ensure that inserts into it will not block.
When an error occurs, we record the error metadata and insert it into the
error_saving_tbl, then proceed to the next row. Although the error_saving_tbl
may not capture information for every invalid column in each row, it retains the
raw_field_value, which can be used for further investigation.
the build-in type: pg_catalog.copy_error_saving definition is
CREATE TYPE copy_error_saving AS
(
userid oid,
copy_tbl oid,
filename text COLLATE "C",
lineno bigint,
line text COLLATE "C",
colname text COLLATE "C",
raw_field_value text COLLATE "C",
err_message text COLLATE "C",
err_detail text COLLATE "C",
errorcode text COLLATE "C"
);
It's declared in src/backend/catalog/system_functions.sql.
If it's going to change, which may cause potential upgrade issue, to avoid that
we may need be sure that this will unlikely to change in the future.
TODO: Should we also add field (starttime timestamptz) to copy_error_saving to
indicate the time when this error record was inserted.
reference: https://postgr.es/m/CACJufxHi53OpGYPAe6SdCb4m=-+H8L+7LDbUWvTiJp=V4YYEqA@mail.gmail.com
reference: https://postgr.es/m/752672.1699474336%40sss.pgh.pa.us
discussion: https://postgr.es/m/CACJufxH_OJpVra=0c4ow8fbxHj7heMcVaTNEPa5vAurSeNA-6Q@mail.gmail.com
commitfest entry: https://commitfest.postgresql.org/patch/4817
---
doc/src/sgml/datatype.sgml | 104 ++++++++
doc/src/sgml/ref/copy.sgml | 50 +++-
src/backend/catalog/system_functions.sql | 13 +
src/backend/commands/copy.c | 23 ++
src/backend/commands/copyfrom.c | 306 ++++++++++++++++++++++-
src/backend/commands/copyfromparse.c | 105 ++++++++
src/backend/parser/gram.y | 1 +
src/include/commands/copy.h | 2 +
src/include/commands/copyfrom_internal.h | 9 +
src/test/regress/expected/copy.out | 6 +
src/test/regress/expected/copy2.out | 201 +++++++++++++++
src/test/regress/sql/copy.sql | 9 +
src/test/regress/sql/copy2.sql | 150 +++++++++++
13 files changed, 968 insertions(+), 11 deletions(-)
diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml
index d8d91678e86..356b7485288 100644
--- a/doc/src/sgml/datatype.sgml
+++ b/doc/src/sgml/datatype.sgml
@@ -5116,6 +5116,110 @@ WHERE ...
</para>
</sect1>
+ <sect1 id="datatype-copy_error_saving">
+ <title><type>copy_error_saving</type> Type</title>
+ <indexterm zone="datatype-copy_error_saving">
+ <primary>copy_error_saving</primary>
+ </indexterm>
+
+ <para>
+ The built-in composite type <type>copy_error_saving</type> is used by the
+ <link linkend="sql-copy"><command>COPY FROM</command></link> command.
+ It contains the following fields, which are used to store information when <command>COPY FROM</command>
+ encounters an error converting a column’s input value to its data type.
+ </para>
+
+ <para>
+<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 <command>COPY FROM</command> operation user.
+ 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.
+ 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 primary error message,
+ see <link linkend="error-message-reporting">ereport</link></entry>
+ </row>
+
+ <row>
+ <entry> <literal>err_detail</literal> </entry>
+ <entry><type>text</type></entry>
+ <entry>The optionally detailed error message,
+ see <link linkend="error-message-reporting">ereport</link> </entry>
+ </row>
+
+ <row>
+ <entry> <literal>errorcode </literal> </entry>
+ <entry><type>text</type></entry>
+ <entry>The SQLSTATE error identifier code for the error condition,
+ see <link linkend="errcodes-appendix">Error Codes</link>
+ </entry>
+ </row>
+
+ </tbody>
+ </tgroup>
+ </informaltable>
+ </para>
+ </sect1>
+
+
<sect1 id="datatype-pg-lsn">
<title><type>pg_lsn</type> Type</title>
diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml
index 4706c9a4410..138c2d64248 100644
--- a/doc/src/sgml/ref/copy.sgml
+++ b/doc/src/sgml/ref/copy.sgml
@@ -46,6 +46,7 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
FORCE_NULL { ( <replaceable class="parameter">column_name</replaceable> [, ...] ) | * }
ON_ERROR <replaceable class="parameter">error_action</replaceable>
REJECT_LIMIT <replaceable class="parameter">maxerror</replaceable>
+ ERROR_TABLE <replaceable class="parameter">error_saving_table</replaceable>
ENCODING '<replaceable class="parameter">encoding_name</replaceable>'
LOG_VERBOSITY <replaceable class="parameter">verbosity</replaceable>
</synopsis>
@@ -450,11 +451,13 @@ COPY (SELECT j FROM (VALUES ('null'::json), (NULL::json)) v(j))
<literal>stop</literal> means fail the command, while
<literal>ignore</literal> means discard the input row and continue with the next one,
and <literal>set_null</literal> means replace the field containing the invalid
- input value with a null value and continue to the next field.
+ input value with a null value and continue to the next field,
+ and <literal>table</literal> means save error information to
+ <replaceable class="parameter">error_saving_table</replaceable> and continue with the next one.
The default is <literal>stop</literal>.
</para>
<para>
- The <literal>ignore</literal> and <literal>set_null</literal>
+ The <literal>ignore</literal>, <literal>set_null</literal> and <literal>table</literal>
options are applicable only for <command>COPY FROM</command>
when the <literal>FORMAT</literal> is <literal>text</literal> or <literal>csv</literal>.
</para>
@@ -463,19 +466,35 @@ COPY (SELECT j FROM (VALUES ('null'::json), (NULL::json)) v(j))
<literal>set_null</literal>, a <literal>NOTICE</literal> message is emitted at the end of the
<command>COPY FROM</command> command containing the count of rows that were ignored or
changed, if at least one row was affected.
+ If <literal>ON_ERROR</literal> option is set to <literal>table</literal>,
+ a <literal>NOTICE</literal> message containing the row count inserted to
+ <replaceable class="parameter">error_saving_table</replaceable> is
+ emitted at the end of the <command>COPY FROM</command>.
</para>
<para>
When <literal>LOG_VERBOSITY</literal> option is set to <literal>verbose</literal>,
- for <literal>ignore</literal> option, a <literal>NOTICE</literal> message
+ for <literal>ignore</literal> or <literal>table</literal> option, a <literal>NOTICE</literal> message
containing the line of the input file and the column name whose input
conversion has failed is emitted for each discarded row;
for <literal>set_null</literal> option, a <literal>NOTICE</literal>
message containing the line of the input file and the column name where
value was replaced with <literal>NULL</literal> for each input conversion
- failure.
+ failure,
+ for <literal>table</literal> option, a <literal>NOTICE</literal>
+ message containing the line of the input file and the column name whose input
+ conversion has failed is emitted for each row that inserted to
+ <replaceable class="parameter">error_saving_table</replaceable>.
When it is set to <literal>silent</literal>, no message is emitted
regarding input conversion failed rows.
</para>
+
+ <para>
+ Triggers on <replaceable class="parameter">error_saving_table</replaceable>
+ will be fired. Therefore, the <literal>NOTICE</literal> message regarding
+ rows inserted into <replaceable class="parameter">error_saving_table</replaceable>
+ may differ from what is actually being inserted.
+ </para>
+
</listitem>
</varlistentry>
@@ -497,6 +516,25 @@ COPY (SELECT j FROM (VALUES ('null'::json), (NULL::json)) v(j))
</listitem>
</varlistentry>
+ <varlistentry id="sql-copy-params-error-table">
+ <term><literal>ERROR_TABLE</literal></term>
+ <listitem>
+ <para>
+ Insert error context details to the table <replaceable class="parameter">error_saving_table</replaceable>.
+ This option is allowed only in <command>COPY FROM</command> and
+ <literal>ON_ERROR</literal> is specified as <literal>TABLE</literal>.
+ The <replaceable class="parameter">error_saving_table</replaceable> must
+ be a typed table based on the system composite type
+ <link linkend="datatype-copy_error_saving"><command>copy_error_saving</command></link>,
+ and the user running <command>COPY FROM</command> requires <literal>INSERT</literal>
+ privileges on it. During the error records inseration,
+ <literal>NOT NULL</literal> and <literal>CHECK</literal> constraints are enforced,
+ and both row-level and statement-level triggers will be fired.
+ </para>
+ </listitem>
+ </varlistentry>
+
+
<varlistentry id="sql-copy-params-encoding">
<term><literal>ENCODING</literal></term>
<listitem>
@@ -522,8 +560,8 @@ COPY (SELECT j FROM (VALUES ('null'::json), (NULL::json)) v(j))
</para>
<para>
This is currently used in <command>COPY FROM</command> command when
- <literal>ON_ERROR</literal> option is set to <literal>ignore</literal>
- or <literal>set_null</literal>.
+ <literal>ON_ERROR</literal> option is set to <literal>ignore</literal>,
+ <literal>set_null</literal> or <literal>table</literal>.
</para>
</listitem>
</varlistentry>
diff --git a/src/backend/catalog/system_functions.sql b/src/backend/catalog/system_functions.sql
index c3c0a6e84ed..62a4a6d98aa 100644
--- a/src/backend/catalog/system_functions.sql
+++ b/src/backend/catalog/system_functions.sql
@@ -366,3 +366,16 @@ CREATE OR REPLACE FUNCTION ts_debug(document text,
BEGIN ATOMIC
SELECT * FROM ts_debug(get_current_ts_config(), $1);
END;
+
+CREATE TYPE copy_error_saving AS(
+ userid oid,
+ copy_tbl oid,
+ filename text COLLATE "C",
+ lineno bigint,
+ line text COLLATE "C",
+ colname text COLLATE "C",
+ raw_field_value text COLLATE "C",
+ err_message text COLLATE "C",
+ err_detail text COLLATE "C",
+ errorcode text COLLATE "C"
+);
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 003b70852bb..2c1c4478bad 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -494,6 +494,8 @@ defGetCopyOnErrorChoice(DefElem *def, ParseState *pstate, bool is_from)
return COPY_ON_ERROR_IGNORE;
if (pg_strcasecmp(sval, "set_null") == 0)
return COPY_ON_ERROR_SET_NULL;
+ if (pg_strcasecmp(sval, "table") == 0)
+ return COPY_ON_ERROR_TABLE;
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
@@ -587,6 +589,7 @@ ProcessCopyOptions(ParseState *pstate,
bool freeze_specified = false;
bool header_specified = false;
bool on_error_specified = false;
+ bool error_rel_specified = false;
bool log_verbosity_specified = false;
bool reject_limit_specified = false;
bool force_array_specified = false;
@@ -774,6 +777,13 @@ ProcessCopyOptions(ParseState *pstate,
reject_limit_specified = true;
opts_out->reject_limit = defGetCopyRejectLimitOption(defel);
}
+ else if (strcmp(defel->defname, "error_table") == 0)
+ {
+ if (error_rel_specified)
+ errorConflictingDefElem(defel, pstate);
+ error_rel_specified = true;
+ opts_out->error_table = defGetString(defel);
+ }
else
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
@@ -782,6 +792,19 @@ ProcessCopyOptions(ParseState *pstate,
parser_errposition(pstate, defel->location)));
}
+ if (opts_out->on_error == COPY_ON_ERROR_TABLE)
+ {
+ if (opts_out->error_table == NULL)
+ ereport(ERROR,
+ errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("cannot set option %s to \"%s\" when \"%s\" is not specified", "ON_ERROR", "TABLE", "ERROR_TABLE"),
+ errhint("\"%s\" option is required", "ERROR_TABLE"));
+ }
+ else if (opts_out->error_table != NULL)
+ ereport(ERROR,
+ errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("COPY %s can only be used when option %s is set to \"%s\"", "ERROR_TABLE", "ON_ERROR", "TABLE"));
+
/*
* Check for incompatible options (must do these three before inserting
* defaults)
diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c
index 64ac3063c61..9d7e4ff8ca7 100644
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -29,6 +29,7 @@
#include "access/tupconvert.h"
#include "access/xact.h"
#include "catalog/namespace.h"
+#include "catalog/pg_namespace.h"
#include "commands/copyapi.h"
#include "commands/copyfrom_internal.h"
#include "commands/progress.h"
@@ -42,16 +43,20 @@
#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"
#include "tcop/tcopprot.h"
+#include "utils/builtins.h"
#include "utils/lsyscache.h"
#include "utils/memutils.h"
#include "utils/portal.h"
#include "utils/rel.h"
+#include "utils/regproc.h"
#include "utils/snapmgr.h"
#include "utils/typcache.h"
+#include "utils/syscache.h"
/*
* No more than this many tuples per CopyMultiInsertBuffer
@@ -120,6 +125,10 @@ static void CopyFromBinaryInFunc(CopyFromState cstate, Oid atttypid,
FmgrInfo *finfo, Oid *typioparam);
static void CopyFromBinaryStart(CopyFromState cstate, TupleDesc tupDesc);
static void CopyFromBinaryEnd(CopyFromState cstate);
+static void RangeVarCallbackForCopyErrorTable(const RangeVar *rv, Oid relid, Oid oldrelid,
+ void *arg);
+static void CopyFromErrorTableInit(CopyFromState cstate);
+static void CopyFromErrorTablePermissionCheck(ParseState *pstate, Relation rel);
/*
@@ -982,6 +991,11 @@ CopyFrom(CopyFromState cstate)
if (cstate->rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
proute = ExecSetupPartitionTupleRouting(estate, cstate->rel);
+ if (cstate->opts.on_error == COPY_ON_ERROR_TABLE)
+ CopyFromErrorTableInit(cstate);
+ else
+ cstate->mtcontext = NULL;
+
if (cstate->whereClause)
cstate->qualexpr = ExecInitQual(castNode(List, cstate->whereClause),
&mtstate->ps);
@@ -1151,22 +1165,26 @@ CopyFrom(CopyFromState cstate)
if (!NextCopyFrom(cstate, econtext, myslot->tts_values, myslot->tts_isnull))
break;
- if (cstate->opts.on_error == COPY_ON_ERROR_IGNORE &&
+ if ((cstate->opts.on_error == COPY_ON_ERROR_IGNORE ||
+ cstate->opts.on_error == COPY_ON_ERROR_TABLE) &&
cstate->escontext->error_occurred)
{
/*
* Soft error occurred, skip this tuple and just make
- * ErrorSaveContext ready for the next NextCopyFrom. Since we
- * don't set details_wanted and error_data is not to be filled,
- * just resetting error_occurred is enough.
+ * ErrorSaveContext ready for the next NextCopyFrom.
*/
cstate->escontext->error_occurred = false;
+ /* Reset ErrorSaveContext->error_data */
+ if (cstate->opts.on_error == COPY_ON_ERROR_TABLE)
+ memset(cstate->escontext->error_data, 0, sizeof(ErrorData));
+
/* Report that this tuple was skipped by the ON_ERROR clause */
pgstat_progress_update_param(PROGRESS_COPY_TUPLES_SKIPPED,
cstate->num_errors);
- if (cstate->opts.reject_limit > 0 &&
+ if (cstate->opts.on_error == COPY_ON_ERROR_IGNORE &&
+ cstate->opts.reject_limit > 0 &&
cstate->num_errors > cstate->opts.reject_limit)
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
@@ -1480,6 +1498,13 @@ CopyFrom(CopyFromState cstate)
"in %" PRIu64 " rows, columns were set to null due to data type incompatibility",
cstate->num_errors,
cstate->num_errors));
+ else if (cstate->opts.on_error == COPY_ON_ERROR_TABLE)
+ ereport(NOTICE,
+ errmsg_plural("%" PRIu64 " row was saved to table \"%s\" due to data type incompatibility",
+ "%" PRIu64 " rows were saved to table \"%s\" due to data type incompatibility",
+ cstate->num_errors,
+ cstate->num_errors,
+ RelationGetRelationName(cstate->error_rel)));
}
if (bistate != NULL)
@@ -1487,6 +1512,39 @@ CopyFrom(CopyFromState cstate)
MemoryContextSwitchTo(oldcontext);
+ /*
+ * This should be aligned with the resource release/destruction performed
+ * by ExecutorFinish and ExecutorEnd on the EState.
+ */
+ if (cstate->opts.on_error == COPY_ON_ERROR_TABLE)
+ {
+ MemoryContext tmpcontext;
+ ModifyTableState *on_error_mtstate;
+
+ tmpcontext =
+ MemoryContextSwitchTo(cstate->mtcontext->estate->es_query_cxt);
+
+ on_error_mtstate = cstate->mtcontext->mtstate;
+ on_error_mtstate->mt_done = true;
+ cstate->mtcontext->estate->es_finished = true;
+
+ /* Release resources associated with error_table */
+ ExecResetTupleTable(cstate->mtcontext->estate->es_tupleTable, false);
+ ExecCloseResultRelations(cstate->mtcontext->estate);
+ ExecCloseRangeTableRelations(cstate->mtcontext->estate);
+
+ /* Do away with our snapshots */
+ UnregisterSnapshot(cstate->mtcontext->estate->es_snapshot);
+ UnregisterSnapshot(cstate->mtcontext->estate->es_crosscheck_snapshot);
+
+ /*
+ * Must switch out of context before destroying it
+ */
+ MemoryContextSwitchTo(tmpcontext);
+
+ FreeExecutorState(cstate->mtcontext->estate);
+ }
+
/* Execute AFTER STATEMENT insertion triggers */
ExecASInsertTriggers(estate, target_resultRelInfo, cstate->transition_capture);
@@ -1630,6 +1688,15 @@ BeginCopyFrom(ParseState *pstate,
if (cstate->opts.on_error == COPY_ON_ERROR_IGNORE ||
cstate->opts.on_error == COPY_ON_ERROR_SET_NULL)
cstate->escontext->details_wanted = false;
+ else if (cstate->opts.on_error == COPY_ON_ERROR_TABLE)
+ {
+ /*
+ * For ON_ERROR = TABLE, we must set details_wanted to true. This
+ * ensures that ErrorData is populated when the next error occurs,
+ * allowing us to capture error metadata.
+ */
+ cstate->escontext->details_wanted = true;
+ }
}
else
cstate->escontext = NULL;
@@ -1657,6 +1724,85 @@ BeginCopyFrom(ParseState *pstate,
cstate->domain_with_constraint[i] = DomainHasConstraints(att->atttypid, NULL);
}
}
+ else if (cstate->opts.on_error == COPY_ON_ERROR_TABLE)
+ {
+ /* Set up COPY FROM (ON_ERROR TABLE) */
+ RangeVar *relvar;
+ List *relname_list;
+ HeapTuple tp;
+ Oid err_relOid,
+ typoid;
+ Oid reloftype = InvalidOid;
+
+ Assert(cstate->opts.error_table != NULL);
+
+ relname_list = stringToQualifiedNameList(cstate->opts.error_table,
+ NULL);
+ relvar = makeRangeVarFromNameList(relname_list);
+
+ /*
+ * We may insert tuples into error_table later. To avoid a deadlock or
+ * long hang during COPY, verify that the table is not already locked;
+ * otherwise, report a lock conflict error.
+ */
+ err_relOid = RangeVarGetRelidExtended(relvar,
+ RowExclusiveLock,
+ RVR_NOWAIT,
+ RangeVarCallbackForCopyErrorTable,
+ NULL);
+
+ if (RelationGetRelid(cstate->rel) == err_relOid)
+ ereport(ERROR,
+ errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("cannot use relation \"%s\" for COPY error saving while copying data to it",
+ cstate->opts.error_table));
+
+ cstate->error_rel = table_open(err_relOid, NoLock);
+
+ /*
+ * The error-saving table must be a plain table. It cannot have
+ * rewrite rules or any enabled row security policies.
+ */
+ if (cstate->error_rel->rd_rel->relrowsecurity || cstate->error_rel->rd_rules)
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot use relation \"%s\" for COPY error saving",
+ RelationGetRelationName(cstate->error_rel)),
+ cstate->error_rel->rd_rel->relrowsecurity
+ ? errdetail("The error saving table cannot have row-level security policies.")
+ : errdetail("The error saving table cannot have rules."));
+
+ typoid = GetSysCacheOid2(TYPENAMENSP, Anum_pg_type_oid,
+ CStringGetDatum("copy_error_saving"),
+ ObjectIdGetDatum(PG_CATALOG_NAMESPACE));
+ if (!OidIsValid(typoid))
+ elog(ERROR, "cache lookup failed for catalog type %s", "copy_error_saving");
+
+ tp = SearchSysCache1(RELOID, ObjectIdGetDatum(err_relOid));
+ if (!HeapTupleIsValid(tp))
+ elog(ERROR, "cache lookup failed for relation %u", err_relOid);
+ else
+ {
+ Form_pg_class reltup = (Form_pg_class) GETSTRUCT(tp);
+
+ reloftype = reltup->reloftype;
+
+ if (reloftype != typoid)
+ ereport(ERROR,
+ errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot use relation \"%s\" for COPY error saving",
+ RelationGetRelationName(cstate->error_rel)),
+ OidIsValid(reloftype)
+ ? errdetail("Relation \"%s\" is a typed table based on type \"%s\".",
+ RelationGetRelationName(cstate->error_rel),
+ format_type_be_qualified(reloftype))
+ : 0,
+ errhint("The COPY error saving table must be a typed table based on type \"%s\".",
+ format_type_be_qualified(typoid)));
+
+ ReleaseSysCache(tp);
+ }
+ }
/* Convert FORCE_NULL name list to per-column flags, check validity */
cstate->opts.force_null_flags = (bool *) palloc0(num_phys_attrs * sizeof(bool));
@@ -1960,6 +2106,9 @@ EndCopyFrom(CopyFromState cstate)
pgstat_progress_end_command();
+ if (cstate->error_rel)
+ table_close(cstate->error_rel, NoLock);
+
MemoryContextDelete(cstate->copycontext);
pfree(cstate);
}
@@ -1998,3 +2147,150 @@ ClosePipeFromProgram(CopyFromState cstate)
errdetail_internal("%s", wait_result_to_str(pclose_rc))));
}
}
+
+/*
+ * Perform permission and other checks for error_table, and initialize
+ * cstate->mtcontext.
+ */
+static void
+CopyFromErrorTableInit(CopyFromState cstate)
+{
+ ModifyTableState *mtstate;
+ ModifyTable *node;
+ MemoryContext tmpcontext;
+ ParseState *pstate = make_parsestate(NULL);
+ EState *estate = CreateExecutorState();
+
+ cstate->mtcontext = palloc0_object(ModifyTableContext);
+
+ Assert(cstate->opts.on_error == COPY_ON_ERROR_TABLE);
+
+ tmpcontext = MemoryContextSwitchTo(estate->es_query_cxt);
+
+ estate->es_output_cid = GetCurrentCommandId(true);
+ estate->es_snapshot = RegisterSnapshot(GetActiveSnapshot());
+ estate->es_crosscheck_snapshot = RegisterSnapshot(InvalidSnapshot);
+
+ /* Do the error saving table permission check */
+ CopyFromErrorTablePermissionCheck(pstate, cstate->error_rel);
+
+ /*
+ * We need a ResultRelInfo so we can use the regular executor's
+ * index-entry-making machinery.
+ */
+ ExecInitRangeTable(estate, pstate->p_rtable, pstate->p_rteperminfos,
+ bms_make_singleton(1));
+
+ node = makeNode(ModifyTable);
+ node->operation = CMD_INSERT;
+ node->canSetTag = false;
+ node->rootRelation = 0;
+ node->resultRelations = list_make1_int(1);
+ node->onConflictAction = ONCONFLICT_NONE;
+
+ /*
+ * Populate ModifyTableState for inserting record to error saving table.
+ */
+ mtstate = makeNode(ModifyTableState);
+ mtstate->ps.plan = (Plan *) node;
+ mtstate->ps.state = estate;
+
+ mtstate->operation = CMD_INSERT;
+ mtstate->canSetTag = false;
+ mtstate->mt_done = false;
+
+ mtstate->mt_nrels = 1;
+ mtstate->resultRelInfo = palloc_array(ResultRelInfo, 1);
+
+ mtstate->rootResultRelInfo = mtstate->resultRelInfo;
+ ExecInitResultRelation(estate, mtstate->resultRelInfo,
+ linitial_int(node->resultRelations));
+
+ /* Verify the named relation is a valid target for INSERT */
+ CheckValidResultRel(mtstate->resultRelInfo, node->operation,
+ node->onConflictAction, NIL);
+
+ /*
+ * Open the table's indexes, if we have not done so already, so that we
+ * can add new index entries for the inserted tuple.
+ */
+ if (cstate->error_rel->rd_rel->relhasindex &&
+ mtstate->resultRelInfo->ri_IndexRelationDescs == NULL)
+ ExecOpenIndices(mtstate->resultRelInfo,
+ node->onConflictAction != ONCONFLICT_NONE);
+
+ MemoryContextSwitchTo(tmpcontext);
+
+ cstate->mtcontext->mtstate = mtstate;
+ cstate->mtcontext->estate = estate;
+}
+
+/*
+ * Callback to RangeVarGetRelidExtended().
+ *
+ * Checks the following:
+ * - the relation specified is a table.
+ * - the table is not a system table.
+ *
+ * If any of these checks fails then an error is raised.
+ */
+static void
+RangeVarCallbackForCopyErrorTable(const RangeVar *rv, Oid relid, Oid oldrelid,
+ void *arg)
+{
+ HeapTuple tuple;
+ Form_pg_class classform;
+ char relkind;
+
+ tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
+ if (!HeapTupleIsValid(tuple))
+ return;
+
+ classform = (Form_pg_class) GETSTRUCT(tuple);
+ relkind = classform->relkind;
+
+ if (!allowSystemTableMods && IsSystemClass(relid, classform))
+ ereport(ERROR,
+ errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("permission denied: \"%s\" is a system catalog",
+ rv->relname));
+
+ /*
+ * Currently, the error-saving table must be a regular relation.
+ *
+ * TODO: Allow cstate->error_rel to be a partitioned table. This should be
+ * not difficult, but requires proper handling of constraints and triggers
+ * on the partitioned table.
+ */
+ if (relkind != RELKIND_RELATION)
+ ereport(ERROR,
+ errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot use relation \"%s\" for COPY error saving",
+ rv->relname),
+ errdetail_relkind_not_supported(relkind));
+
+ ReleaseSysCache(tuple);
+}
+
+/*
+ * COPY (ON_ERROR TABLE) inserts COPY FROM error details to error_table,
+ * therefore, the current user must have INSERT privileges on error_table.
+ */
+static void
+CopyFromErrorTablePermissionCheck(ParseState *pstate, Relation rel)
+{
+ LOCKMODE lockmode = RowExclusiveLock;
+ AclResult aclresult;
+
+ /* Must have INSERT privilege on the table */
+ aclresult = pg_class_aclcheck(RelationGetRelid(rel),
+ GetUserId(),
+ ACL_INSERT);
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult,
+ get_relkind_objtype(get_rel_relkind(RelationGetRelid(rel))),
+ RelationGetRelationName(rel));
+
+ addRangeTableEntryForRelation(pstate, rel, lockmode,
+ NULL, false, false);
+}
diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 65fd5a0ab4f..f7b456bd8bc 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1101,6 +1101,96 @@ CopyFromTextLikeOneRow(CopyFromState cstate, ExprContext *econtext,
cstate->num_errors++;
}
}
+ else if (cstate->opts.on_error == COPY_ON_ERROR_TABLE)
+ {
+ char *err_code;
+ Datum *newvalues;
+ bool *newnulls;
+ ModifyTableState *mtstate = cstate->mtcontext->mtstate;
+ EState *estate = mtstate->ps.state;
+ MemoryContext tmpcontext;
+
+ /* Prepare to build the result tuple */
+ TupleTableSlot *myslot = ExecGetReturningSlot(estate,
+ mtstate->resultRelInfo);
+
+ ExecClearTuple(myslot);
+
+ newvalues = myslot->tts_values;
+ newnulls = myslot->tts_isnull;
+
+ Assert(RelationGetDescr(cstate->error_rel)->natts == 10);
+
+ for (int i = 0; i < RelationGetDescr(cstate->error_rel)->natts; i++)
+ newnulls[i] = false;
+
+ newvalues[0] = ObjectIdGetDatum(GetUserId());
+ newvalues[1] = ObjectIdGetDatum(cstate->rel->rd_rel->oid);
+ newvalues[2] = CStringGetTextDatum(cstate->filename ? cstate->filename : "STDIN");
+ newvalues[3] = Int64GetDatum((int64) cstate->cur_lineno);
+ newvalues[4] = CStringGetTextDatum(cstate->line_buf.data);
+ newvalues[5] = CStringGetTextDatum(cstate->cur_attname);
+
+ if (string)
+ newvalues[6] = CStringGetTextDatum(string);
+ else
+ {
+ newvalues[6] = (Datum) 0;
+ newnulls[6] = true;
+ }
+
+ if (cstate->escontext->error_data->message)
+ newvalues[7] =
+ CStringGetTextDatum(cstate->escontext->error_data->message);
+ else
+ {
+ newvalues[7] = (Datum) 0;
+ newnulls[7] = true;
+ }
+
+ if (cstate->escontext->error_data->detail)
+ newvalues[8] =
+ CStringGetTextDatum(cstate->escontext->error_data->detail);
+ else
+ {
+ newvalues[8] = (Datum) 0;
+ newnulls[8] = true;
+ }
+
+ err_code =
+ unpack_sql_state(cstate->escontext->error_data->sqlerrcode);
+ newvalues[9] = CStringGetTextDatum(err_code);
+
+ /* Build the virtual tuple. */
+ ExecStoreVirtualTuple(myslot);
+
+ tmpcontext =
+ MemoryContextSwitchTo(cstate->mtcontext->estate->es_query_cxt);
+
+ AfterTriggerBeginQuery();
+ mtstate->mt_transition_capture =
+ MakeTransitionCaptureState(cstate->error_rel->trigdesc,
+ RelationGetRelid(cstate->error_rel),
+ CMD_INSERT);
+ /* Execute BEFORE STATEMENT insertion triggers */
+ ExecBSInsertTriggers(cstate->mtcontext->estate,
+ cstate->mtcontext->mtstate->rootResultRelInfo);
+ ExecInsert(cstate->mtcontext,
+ cstate->mtcontext->mtstate->resultRelInfo,
+ myslot,
+ false,
+ NULL,
+ NULL);
+ /* Execute AFTER STATEMENT insertion triggers */
+ ExecASInsertTriggers(cstate->mtcontext->estate,
+ mtstate->rootResultRelInfo,
+ mtstate->mt_transition_capture);
+ AfterTriggerEndQuery(estate);
+
+ MemoryContextSwitchTo(tmpcontext);
+
+ cstate->num_errors++;
+ }
if (cstate->opts.log_verbosity == COPY_LOG_VERBOSITY_VERBOSE)
{
@@ -1130,6 +1220,13 @@ CopyFromTextLikeOneRow(CopyFromState cstate, ExprContext *econtext,
cstate->cur_lineno,
cstate->cur_attname,
attval));
+ else if (cstate->opts.on_error == COPY_ON_ERROR_TABLE)
+ ereport(NOTICE,
+ errmsg("saving error information to table \"%s\" row due to data type incompatibility at line %" PRIu64 " for column \"%s\": \"%s\"",
+ RelationGetRelationName(cstate->error_rel),
+ cstate->cur_lineno,
+ cstate->cur_attname,
+ attval));
pfree(attval);
}
else
@@ -1139,6 +1236,12 @@ CopyFromTextLikeOneRow(CopyFromState cstate, ExprContext *econtext,
errmsg("skipping row due to data type incompatibility at line %" PRIu64 " for column \"%s\": null input",
cstate->cur_lineno,
cstate->cur_attname));
+ else if (cstate->opts.on_error == COPY_ON_ERROR_TABLE)
+ ereport(NOTICE,
+ errmsg("saving error information to table \"%s\" row due to data type incompatibility at line %" PRIu64 " for column \"%s\": null input",
+ RelationGetRelationName(cstate->error_rel),
+ cstate->cur_lineno,
+ cstate->cur_attname));
}
/* reset relname_only */
cstate->relname_only = false;
@@ -1148,6 +1251,8 @@ CopyFromTextLikeOneRow(CopyFromState cstate, ExprContext *econtext,
return true;
else if (cstate->opts.on_error == COPY_ON_ERROR_SET_NULL)
continue;
+ else if (cstate->opts.on_error == COPY_ON_ERROR_TABLE)
+ return true;
}
cstate->cur_attname = NULL;
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index ff4e1388c55..2854f2a884f 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -3755,6 +3755,7 @@ copy_generic_opt_arg:
| NumericOnly { $$ = (Node *) $1; }
| '*' { $$ = (Node *) makeNode(A_Star); }
| DEFAULT { $$ = (Node *) makeString("default"); }
+ | TABLE { $$ = (Node *) makeString("table"); }
| '(' copy_generic_opt_arg_list ')' { $$ = (Node *) $2; }
| /* EMPTY */ { $$ = NULL; }
;
diff --git a/src/include/commands/copy.h b/src/include/commands/copy.h
index abecfe51098..3745342d0f9 100644
--- a/src/include/commands/copy.h
+++ b/src/include/commands/copy.h
@@ -36,6 +36,7 @@ typedef enum CopyOnErrorChoice
COPY_ON_ERROR_STOP = 0, /* immediately throw errors, default */
COPY_ON_ERROR_IGNORE, /* ignore errors */
COPY_ON_ERROR_SET_NULL, /* set error field to null */
+ COPY_ON_ERROR_TABLE, /* save input conversion errors info to table */
} CopyOnErrorChoice;
/*
@@ -96,6 +97,7 @@ typedef struct CopyFormatOptions
CopyOnErrorChoice on_error; /* what to do when error happened */
CopyLogVerbosityChoice log_verbosity; /* verbosity of logged messages */
int64 reject_limit; /* maximum tolerable number of errors */
+ char *error_table; /* on error, save error info to the table, table name */
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 9d3e244ee55..64d0b204090 100644
--- a/src/include/commands/copyfrom_internal.h
+++ b/src/include/commands/copyfrom_internal.h
@@ -16,6 +16,7 @@
#include "commands/copy.h"
#include "commands/trigger.h"
+#include "executor/nodeModifyTable.h"
#include "nodes/miscnodes.h"
/*
@@ -73,6 +74,7 @@ typedef struct CopyFromStateData
/* parameters from the COPY command */
Relation rel; /* relation to copy from */
+ Relation error_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? */
@@ -189,6 +191,13 @@ typedef struct CopyFromStateData
#define RAW_BUF_BYTES(cstate) ((cstate)->raw_buf_len - (cstate)->raw_buf_index)
uint64 bytes_processed; /* number of bytes processed so far */
+
+ /*
+ * INSERT operation context for inserting COPY FROM input conversion
+ * failure error information to error_table. Populated only when ON_ERROR
+ * is specified as 'TABLE'.
+ */
+ ModifyTableContext *mtcontext;
} CopyFromStateData;
extern void ReceiveCopyBegin(CopyFromState cstate);
diff --git a/src/test/regress/expected/copy.out b/src/test/regress/expected/copy.out
index 37498cdd6e7..530a83a31fa 100644
--- a/src/test/regress/expected/copy.out
+++ b/src/test/regress/expected/copy.out
@@ -433,6 +433,12 @@ INFO: progress: {"type": "FILE", "command": "COPY FROM", "relname": "tab_progre
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}
+begin;
+create temp table err_tbl OF copy_error_saving;
+copy tab_progress_reporting from stdin(on_error table, error_table err_tbl);
+NOTICE: 2 rows were saved to table "err_tbl" 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}
+rollback;
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/expected/copy2.out b/src/test/regress/expected/copy2.out
index 7600e5239d2..4fbce478f9f 100644
--- a/src/test/regress/expected/copy2.out
+++ b/src/test/regress/expected/copy2.out
@@ -884,7 +884,208 @@ ERROR: skipped more than REJECT_LIMIT (3) rows due to data type incompatibility
CONTEXT: COPY check_ign_err, line 5, column n: ""
COPY check_ign_err FROM STDIN WITH (on_error ignore, reject_limit 4);
NOTICE: 4 rows were skipped due to data type incompatibility
+-- Tests for on_error table, save COPY FROM input data type conversion error
+-- information to a user-defined table
+-- Direct modification of system catalog composite type copy_error_saving is
+-- not allowed
+ALTER TYPE copy_error_saving ADD ATTRIBUTE b text;
+ERROR: permission denied: "copy_error_saving" is a system catalog
+ALTER TYPE copy_error_saving DROP ATTRIBUTE userid;
+ERROR: permission denied: "copy_error_saving" is a system catalog
+ALTER TYPE copy_error_saving RENAME ATTRIBUTE userid to userid1;
+ERROR: permission denied: "copy_error_saving" is a system catalog
+ALTER TYPE copy_error_saving ALTER ATTRIBUTE userid SET DATA TYPE OID8;
+ERROR: permission denied: "copy_error_saving" is a system catalog
+CREATE TABLE t_on_error_table(a jsonb, b int, c int, d dcheck_ign_err2);
+CREATE TABLE err_tbl OF copy_error_saving;
+CREATE TABLE err_tbl1 OF copy_error_saving;
+CREATE TABLE err_tbl2 OF copy_error_saving PARTITION BY RANGE (lineno);
+CREATE UNIQUE INDEX err_tbl_idx ON err_tbl(colname);
+CREATE TEMP TABLE err_tbl3 AS SELECT * FROM err_tbl;
+CREATE TYPE t_copy_typ AS (a int, b int, c int);
+CREATE TABLE t_copy_tbl1 OF t_copy_typ;
+CREATE TEMP VIEW t_copy_v1 AS SELECT * FROM t_on_error_table;
+-- all of the following should fail
+COPY t_on_error_table FROM STDIN WITH (format binary, on_error table, error_table err_tbl);
+ERROR: only ON_ERROR STOP is allowed in BINARY mode
+COPY t_on_error_table FROM STDIN WITH (on_error table, error_table t_on_error_table);
+ERROR: cannot use relation "t_on_error_table" for COPY error saving while copying data to it
+COPY t_on_error_table FROM STDIN WITH (on_error table, error_table t_copy_tbl1);
+ERROR: cannot use relation "t_copy_tbl1" for COPY error saving
+DETAIL: Relation "t_copy_tbl1" is a typed table based on type "public.t_copy_typ".
+HINT: The COPY error saving table must be a typed table based on type "pg_catalog.copy_error_saving".
+COPY t_on_error_table FROM STDIN WITH (on_error table, error_table t_copy_v1);
+ERROR: cannot use relation "t_copy_v1" for COPY error saving
+DETAIL: This operation is not supported for views.
+COPY t_on_error_table FROM STDIN WITH (on_error table, reject_limit 10, error_table err_tbl);
+ERROR: COPY REJECT_LIMIT requires ON_ERROR to be set to IGNORE
+COPY t_on_error_table FROM STDIN WITH (on_error table, error_table not_existsx);
+ERROR: relation "not_existsx" does not exist
+COPY t_on_error_table FROM STDIN WITH (on_error table);
+ERROR: cannot set option ON_ERROR to "TABLE" when "ERROR_TABLE" is not specified
+HINT: "ERROR_TABLE" option is required
+COPY t_on_error_table FROM STDIN WITH (error_table err_tbl);
+ERROR: COPY ERROR_TABLE can only be used when option ON_ERROR is set to "TABLE"
+COPY t_on_error_table TO STDIN WITH (on_error table);
+ERROR: COPY ON_ERROR cannot be used with COPY TO
+LINE 1: COPY t_on_error_table TO STDIN WITH (on_error table);
+ ^
+-- all of the above should fail
+-- error, error_table cannot be partitioned table
+COPY t_on_error_table(a,b) FROM STDIN WITH (on_error table, error_table err_tbl2);
+ERROR: cannot use relation "err_tbl2" for COPY error saving
+DETAIL: This operation is not supported for partitioned tables.
+CREATE RULE regtest_test_rule AS ON INSERT TO err_tbl1 DO ALSO NOTHING;
+-- error, error_table cannot be have rules
+COPY t_on_error_table(a,b) FROM STDIN WITH (on_error table, error_table err_tbl1);
+ERROR: cannot use relation "err_tbl1" for COPY error saving
+DETAIL: The error saving table cannot have rules.
+DROP RULE regtest_test_rule ON err_tbl1;
+CREATE POLICY p1 ON err_tbl1 FOR SELECT USING (true);
+ALTER TABLE err_tbl1 ENABLE ROW LEVEL SECURITY;
+ALTER TABLE err_tbl1 FORCE ROW LEVEL SECURITY;
+-- error, error_table cannot be have RLS policies
+COPY t_on_error_table(a,b) FROM STDIN WITH (on_error table, error_table err_tbl1);
+ERROR: cannot use relation "err_tbl1" for COPY error saving
+DETAIL: The error saving table cannot have row-level security policies.
+DROP POLICY IF EXISTS p1 ON err_tbl1;
+ALTER TABLE err_tbl1 DISABLE ROW LEVEL SECURITY;
+ALTER TABLE err_tbl ADD CONSTRAINT cc2 CHECK(lineno > 0);
+ALTER TABLE err_tbl ADD CONSTRAINT cc3 NOT NULL userid;
+-- ok, constraints on table err_tbl1 will be verified
+COPY t_on_error_table(b, a) FROM STDIN WITH (DELIMITER ',', on_error table, error_table err_tbl1);
+NOTICE: 1 row was saved to table "err_tbl1" due to data type incompatibility
+ALTER TABLE err_tbl DROP CONSTRAINT cc2;
+ALTER TABLE err_tbl DROP CONSTRAINT cc3;
+TRUNCATE err_tbl;
+-- fail, copied data have extra columns
+COPY t_on_error_table(a,b) FROM STDIN WITH (DELIMITER ',', on_error table, error_table err_tbl);
+ERROR: extra data after last expected column
+CONTEXT: COPY t_on_error_table, line 1: "1,2,3,4,5"
+-- fail, copied data have less columns
+COPY t_on_error_table(a,b) FROM STDIN WITH (DELIMITER ',', on_error table, error_table err_tbl);
+ERROR: extra data after last expected column
+CONTEXT: COPY t_on_error_table, line 1: "1,2,3"
+-- permission check
+BEGIN;
+CREATE USER regress_user30;
+GRANT INSERT(userid, copy_tbl, filename, lineno, line, colname, raw_field_value, err_message, err_detail)
+ ON TABLE err_tbl TO regress_user30;
+GRANT INSERT ON TABLE t_on_error_table TO regress_user30;
+GRANT SELECT ON TABLE err_tbl TO regress_user30;
+SAVEPOINT s1;
+SET ROLE regress_user30;
+COPY t_on_error_table FROM STDIN WITH (on_error table, error_table err_tbl); -- error, insufficient privilege
+ERROR: permission denied for table err_tbl
+ROLLBACK TO SAVEPOINT s1;
+RESET ROLE;
+GRANT INSERT on TABLE err_tbl to regress_user30;
+GRANT INSERT(errorcode) ON TABLE err_tbl TO regress_user30;
+SET ROLE regress_user30;
+COPY t_on_error_table FROM STDIN WITH (DELIMITER ',', on_error table, error_table err_tbl, log_verbosity verbose); -- ok
+NOTICE: saving error information to table "err_tbl" row due to data type incompatibility at line 1 for column "a": "a"
+NOTICE: 1 row was saved to table "err_tbl" due to data type incompatibility
+SELECT copy_tbl::regclass, filename, lineno, line, colname, raw_field_value, err_message, err_detail, errorcode
+FROM err_tbl;
+ copy_tbl | filename | lineno | line | colname | raw_field_value | err_message | err_detail | errorcode
+------------------+----------+--------+---------+---------+-----------------+------------------------------------+-----------------------+-----------
+ t_on_error_table | STDIN | 1 | a,b,3,4 | a | a | invalid input syntax for type json | Token "a" is invalid. | 22P02
+(1 row)
+
+-- error, unique constraint violation on table err_tbl
+COPY t_on_error_table FROM STDIN WITH (DELIMITER ',', on_error table, error_table err_tbl);
+ERROR: duplicate key value violates unique constraint "err_tbl_idx"
+DETAIL: Key (colname)=(a) already exists.
+CONTEXT: COPY t_on_error_table, line 1, column a: "a"
+ROLLBACK;
+DROP INDEX err_tbl_idx;
+CREATE FUNCTION trig_copy_error_saving_insert()
+RETURNS TRIGGER LANGUAGE plpgsql AS
+$$
+BEGIN
+ if (TG_LEVEL = 'STATEMENT' and TG_WHEN = 'AFTER') then
+ RAISE NOTICE E'trigger name: %, % % FOR EACH %\n', TG_NAME, TG_WHEN, TG_OP, TG_LEVEL;
+ else
+ RAISE NOTICE 'trigger name: %, % % FOR EACH %', TG_NAME, TG_WHEN, TG_OP, TG_LEVEL;
+ end if;
+ if TG_LEVEL = 'ROW' then
+ RAISE NOTICE 'NEW raw_field_value: %, err_message: %', NEW.raw_field_value, NEW.err_message;
+ end if;
+ RETURN NEW;
+END;
+$$;
+CREATE TRIGGER err_tbl_after_ins_row
+ AFTER INSERT ON err_tbl
+ FOR EACH ROW EXECUTE PROCEDURE trig_copy_error_saving_insert();
+CREATE TRIGGER err_tbl_before_ins_row
+ BEFORE INSERT ON err_tbl
+ FOR EACH ROW EXECUTE PROCEDURE trig_copy_error_saving_insert();
+CREATE TRIGGER err_tbl_bfore_ins_stmt
+ BEFORE INSERT ON err_tbl
+ FOR EACH STATEMENT EXECUTE PROCEDURE trig_copy_error_saving_insert();
+CREATE TRIGGER err_tbl_after_ins_stmt
+ AFTER INSERT ON err_tbl
+ REFERENCING NEW TABLE AS new_rows
+ FOR EACH STATEMENT EXECUTE PROCEDURE trig_copy_error_saving_insert();
+-- Each insert on the error_table invokes both the statement-level and row-level triggers.
+COPY t_on_error_table FROM STDIN WITH (DELIMITER ',', on_error table, error_table err_tbl);
+NOTICE: trigger name: err_tbl_bfore_ins_stmt, BEFORE INSERT FOR EACH STATEMENT
+NOTICE: trigger name: err_tbl_before_ins_row, BEFORE INSERT FOR EACH ROW
+NOTICE: NEW raw_field_value: a, err_message: invalid input syntax for type integer: "a"
+NOTICE: trigger name: err_tbl_after_ins_row, AFTER INSERT FOR EACH ROW
+NOTICE: NEW raw_field_value: a, err_message: invalid input syntax for type integer: "a"
+NOTICE: trigger name: err_tbl_after_ins_stmt, AFTER INSERT FOR EACH STATEMENT
+
+NOTICE: trigger name: err_tbl_bfore_ins_stmt, BEFORE INSERT FOR EACH STATEMENT
+NOTICE: trigger name: err_tbl_before_ins_row, BEFORE INSERT FOR EACH ROW
+NOTICE: NEW raw_field_value: <NULL>, err_message: domain dcheck_ign_err2 does not allow null values
+NOTICE: trigger name: err_tbl_after_ins_row, AFTER INSERT FOR EACH ROW
+NOTICE: NEW raw_field_value: <NULL>, err_message: domain dcheck_ign_err2 does not allow null values
+NOTICE: trigger name: err_tbl_after_ins_stmt, AFTER INSERT FOR EACH STATEMENT
+
+NOTICE: trigger name: err_tbl_bfore_ins_stmt, BEFORE INSERT FOR EACH STATEMENT
+NOTICE: trigger name: err_tbl_before_ins_row, BEFORE INSERT FOR EACH ROW
+NOTICE: NEW raw_field_value: _junk, err_message: invalid input syntax for type integer: "_junk"
+NOTICE: trigger name: err_tbl_after_ins_row, AFTER INSERT FOR EACH ROW
+NOTICE: NEW raw_field_value: _junk, err_message: invalid input syntax for type integer: "_junk"
+NOTICE: trigger name: err_tbl_after_ins_stmt, AFTER INSERT FOR EACH STATEMENT
+
+NOTICE: trigger name: err_tbl_bfore_ins_stmt, BEFORE INSERT FOR EACH STATEMENT
+NOTICE: trigger name: err_tbl_before_ins_row, BEFORE INSERT FOR EACH ROW
+NOTICE: NEW raw_field_value: cola, err_message: invalid input syntax for type json
+NOTICE: trigger name: err_tbl_after_ins_row, AFTER INSERT FOR EACH ROW
+NOTICE: NEW raw_field_value: cola, err_message: invalid input syntax for type json
+NOTICE: trigger name: err_tbl_after_ins_stmt, AFTER INSERT FOR EACH STATEMENT
+
+NOTICE: trigger name: err_tbl_bfore_ins_stmt, BEFORE INSERT FOR EACH STATEMENT
+NOTICE: trigger name: err_tbl_before_ins_row, BEFORE INSERT FOR EACH ROW
+NOTICE: NEW raw_field_value: 4238679732489879879, err_message: value "4238679732489879879" is out of range for type integer
+NOTICE: trigger name: err_tbl_after_ins_row, AFTER INSERT FOR EACH ROW
+NOTICE: NEW raw_field_value: 4238679732489879879, err_message: value "4238679732489879879" is out of range for type integer
+NOTICE: trigger name: err_tbl_after_ins_stmt, AFTER INSERT FOR EACH STATEMENT
+
+NOTICE: 5 rows were saved to table "err_tbl" due to data type incompatibility
+SELECT copy_tbl::regclass, filename, lineno, line, colname ,raw_field_value, err_message, err_detail, errorcode
+FROM err_tbl;
+ copy_tbl | filename | lineno | line | colname | raw_field_value | err_message | err_detail | errorcode
+------------------+----------+--------+----------------------------+---------+---------------------+--------------------------------------------------------------+--------------------------+-----------
+ t_on_error_table | STDIN | 1 | 1,2,a,1 | c | a | invalid input syntax for type integer: "a" | | 22P02
+ t_on_error_table | STDIN | 2 | 1,2,3,\N | d | | domain dcheck_ign_err2 does not allow null values | | 23502
+ t_on_error_table | STDIN | 3 | 1,_junk,test,11 | b | _junk | invalid input syntax for type integer: "_junk" | | 22P02
+ t_on_error_table | STDIN | 4 | cola,colb,colc,12 | a | cola | invalid input syntax for type json | Token "cola" is invalid. | 22P02
+ t_on_error_table | STDIN | 6 | 1,11,4238679732489879879,2 | c | 4238679732489879879 | value "4238679732489879879" is out of range for type integer | | 22003
+(5 rows)
+
-- clean up
+DROP TABLE err_tbl;
+DROP TABLE err_tbl1;
+DROP TABLE err_tbl2;
+DROP TABLE err_tbl3;
+DROP VIEW t_copy_v1;
+DROP TABLE t_on_error_table;
+DROP TABLE t_copy_tbl1;
+DROP TYPE t_copy_typ;
+DROP FUNCTION trig_copy_error_saving_insert();
DROP TABLE forcetest;
DROP TABLE vistest;
DROP FUNCTION truncate_in_subxact();
diff --git a/src/test/regress/sql/copy.sql b/src/test/regress/sql/copy.sql
index 094fd76c12b..7627a5a09ca 100644
--- a/src/test/regress/sql/copy.sql
+++ b/src/test/regress/sql/copy.sql
@@ -376,6 +376,15 @@ sharon 25 (15,12) 1000 sam
sharon y (15,12) x sam
\.
+begin;
+create temp table err_tbl OF copy_error_saving;
+copy tab_progress_reporting from stdin(on_error table, error_table err_tbl);
+sharon x (15,12) x sam
+sharon 25 (15,12) 1000 sam
+sharon y (15,12) x sam
+\.
+rollback;
+
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/copy2.sql b/src/test/regress/sql/copy2.sql
index e0810109473..f93af03c996 100644
--- a/src/test/regress/sql/copy2.sql
+++ b/src/test/regress/sql/copy2.sql
@@ -636,7 +636,157 @@ a {7} 7
10 {10} 10
\.
+-- Tests for on_error table, save COPY FROM input data type conversion error
+-- information to a user-defined table
+
+-- Direct modification of system catalog composite type copy_error_saving is
+-- not allowed
+ALTER TYPE copy_error_saving ADD ATTRIBUTE b text;
+ALTER TYPE copy_error_saving DROP ATTRIBUTE userid;
+ALTER TYPE copy_error_saving RENAME ATTRIBUTE userid to userid1;
+ALTER TYPE copy_error_saving ALTER ATTRIBUTE userid SET DATA TYPE OID8;
+
+CREATE TABLE t_on_error_table(a jsonb, b int, c int, d dcheck_ign_err2);
+CREATE TABLE err_tbl OF copy_error_saving;
+CREATE TABLE err_tbl1 OF copy_error_saving;
+CREATE TABLE err_tbl2 OF copy_error_saving PARTITION BY RANGE (lineno);
+CREATE UNIQUE INDEX err_tbl_idx ON err_tbl(colname);
+CREATE TEMP TABLE err_tbl3 AS SELECT * FROM err_tbl;
+CREATE TYPE t_copy_typ AS (a int, b int, c int);
+CREATE TABLE t_copy_tbl1 OF t_copy_typ;
+CREATE TEMP VIEW t_copy_v1 AS SELECT * FROM t_on_error_table;
+
+-- all of the following should fail
+COPY t_on_error_table FROM STDIN WITH (format binary, on_error table, error_table err_tbl);
+COPY t_on_error_table FROM STDIN WITH (on_error table, error_table t_on_error_table);
+COPY t_on_error_table FROM STDIN WITH (on_error table, error_table t_copy_tbl1);
+COPY t_on_error_table FROM STDIN WITH (on_error table, error_table t_copy_v1);
+COPY t_on_error_table FROM STDIN WITH (on_error table, reject_limit 10, error_table err_tbl);
+COPY t_on_error_table FROM STDIN WITH (on_error table, error_table not_existsx);
+COPY t_on_error_table FROM STDIN WITH (on_error table);
+COPY t_on_error_table FROM STDIN WITH (error_table err_tbl);
+COPY t_on_error_table TO STDIN WITH (on_error table);
+-- all of the above should fail
+
+-- error, error_table cannot be partitioned table
+COPY t_on_error_table(a,b) FROM STDIN WITH (on_error table, error_table err_tbl2);
+CREATE RULE regtest_test_rule AS ON INSERT TO err_tbl1 DO ALSO NOTHING;
+-- error, error_table cannot be have rules
+COPY t_on_error_table(a,b) FROM STDIN WITH (on_error table, error_table err_tbl1);
+DROP RULE regtest_test_rule ON err_tbl1;
+
+CREATE POLICY p1 ON err_tbl1 FOR SELECT USING (true);
+ALTER TABLE err_tbl1 ENABLE ROW LEVEL SECURITY;
+ALTER TABLE err_tbl1 FORCE ROW LEVEL SECURITY;
+-- error, error_table cannot be have RLS policies
+COPY t_on_error_table(a,b) FROM STDIN WITH (on_error table, error_table err_tbl1);
+DROP POLICY IF EXISTS p1 ON err_tbl1;
+ALTER TABLE err_tbl1 DISABLE ROW LEVEL SECURITY;
+
+ALTER TABLE err_tbl ADD CONSTRAINT cc2 CHECK(lineno > 0);
+ALTER TABLE err_tbl ADD CONSTRAINT cc3 NOT NULL userid;
+-- ok, constraints on table err_tbl1 will be verified
+COPY t_on_error_table(b, a) FROM STDIN WITH (DELIMITER ',', on_error table, error_table err_tbl1);
+a,b
+\.
+ALTER TABLE err_tbl DROP CONSTRAINT cc2;
+ALTER TABLE err_tbl DROP CONSTRAINT cc3;
+TRUNCATE err_tbl;
+
+-- fail, copied data have extra columns
+COPY t_on_error_table(a,b) FROM STDIN WITH (DELIMITER ',', on_error table, error_table err_tbl);
+1,2,3,4,5
+\.
+
+-- fail, copied data have less columns
+COPY t_on_error_table(a,b) FROM STDIN WITH (DELIMITER ',', on_error table, error_table err_tbl);
+1,2,3
+\.
+
+-- permission check
+BEGIN;
+CREATE USER regress_user30;
+GRANT INSERT(userid, copy_tbl, filename, lineno, line, colname, raw_field_value, err_message, err_detail)
+ ON TABLE err_tbl TO regress_user30;
+GRANT INSERT ON TABLE t_on_error_table TO regress_user30;
+GRANT SELECT ON TABLE err_tbl TO regress_user30;
+SAVEPOINT s1;
+
+SET ROLE regress_user30;
+COPY t_on_error_table FROM STDIN WITH (on_error table, error_table err_tbl); -- error, insufficient privilege
+\.
+ROLLBACK TO SAVEPOINT s1;
+
+RESET ROLE;
+GRANT INSERT on TABLE err_tbl to regress_user30;
+GRANT INSERT(errorcode) ON TABLE err_tbl TO regress_user30;
+SET ROLE regress_user30;
+COPY t_on_error_table FROM STDIN WITH (DELIMITER ',', on_error table, error_table err_tbl, log_verbosity verbose); -- ok
+a,b,3,4
+\.
+
+SELECT copy_tbl::regclass, filename, lineno, line, colname, raw_field_value, err_message, err_detail, errorcode
+FROM err_tbl;
+
+-- error, unique constraint violation on table err_tbl
+COPY t_on_error_table FROM STDIN WITH (DELIMITER ',', on_error table, error_table err_tbl);
+a,b,3,4
+\.
+ROLLBACK;
+DROP INDEX err_tbl_idx;
+
+CREATE FUNCTION trig_copy_error_saving_insert()
+RETURNS TRIGGER LANGUAGE plpgsql AS
+$$
+BEGIN
+ if (TG_LEVEL = 'STATEMENT' and TG_WHEN = 'AFTER') then
+ RAISE NOTICE E'trigger name: %, % % FOR EACH %\n', TG_NAME, TG_WHEN, TG_OP, TG_LEVEL;
+ else
+ RAISE NOTICE 'trigger name: %, % % FOR EACH %', TG_NAME, TG_WHEN, TG_OP, TG_LEVEL;
+ end if;
+ if TG_LEVEL = 'ROW' then
+ RAISE NOTICE 'NEW raw_field_value: %, err_message: %', NEW.raw_field_value, NEW.err_message;
+ end if;
+ RETURN NEW;
+END;
+$$;
+
+CREATE TRIGGER err_tbl_after_ins_row
+ AFTER INSERT ON err_tbl
+ FOR EACH ROW EXECUTE PROCEDURE trig_copy_error_saving_insert();
+CREATE TRIGGER err_tbl_before_ins_row
+ BEFORE INSERT ON err_tbl
+ FOR EACH ROW EXECUTE PROCEDURE trig_copy_error_saving_insert();
+CREATE TRIGGER err_tbl_bfore_ins_stmt
+ BEFORE INSERT ON err_tbl
+ FOR EACH STATEMENT EXECUTE PROCEDURE trig_copy_error_saving_insert();
+CREATE TRIGGER err_tbl_after_ins_stmt
+ AFTER INSERT ON err_tbl
+ REFERENCING NEW TABLE AS new_rows
+ FOR EACH STATEMENT EXECUTE PROCEDURE trig_copy_error_saving_insert();
+
+-- Each insert on the error_table invokes both the statement-level and row-level triggers.
+COPY t_on_error_table FROM STDIN WITH (DELIMITER ',', on_error table, error_table err_tbl);
+1,2,a,1
+1,2,3,\N
+1,_junk,test,11
+cola,colb,colc,12
+4,5,6,1111
+1,11,4238679732489879879,2
+\.
+SELECT copy_tbl::regclass, filename, lineno, line, colname ,raw_field_value, err_message, err_detail, errorcode
+FROM err_tbl;
+
-- clean up
+DROP TABLE err_tbl;
+DROP TABLE err_tbl1;
+DROP TABLE err_tbl2;
+DROP TABLE err_tbl3;
+DROP VIEW t_copy_v1;
+DROP TABLE t_on_error_table;
+DROP TABLE t_copy_tbl1;
+DROP TYPE t_copy_typ;
+DROP FUNCTION trig_copy_error_saving_insert();
DROP TABLE forcetest;
DROP TABLE vistest;
DROP FUNCTION truncate_in_subxact();
--
2.34.1
[text/x-patch] v13-0001-export-ExecInsert.patch (4.8K, 3-v13-0001-export-ExecInsert.patch)
download | inline diff:
From 99a5560b5f78959c34b3d981eeb1b5fa26c54a33 Mon Sep 17 00:00:00 2001
From: jian he <[email protected]>
Date: Sun, 10 May 2026 11:54:30 +0800
Subject: [PATCH v13 1/2] export ExecInsert
The ExecInsert function encapsulates core logic for the insertion pipeline,
including partition routing, BEFORE ROW triggers, INSTEAD OF triggers, and AFTER
ROW triggers and others.
reference: https://postgr.es/m/CACJufxHi53OpGYPAe6SdCb4m=-+H8L+7LDbUWvTiJp=V4YYEqA@mail.gmail.com
reference: https://postgr.es/m/752672.1699474336%40sss.pgh.pa.us
discussion: https://postgr.es/m/CACJufxH_OJpVra=0c4ow8fbxHj7heMcVaTNEPa5vAurSeNA-6Q@mail.gmail.com
commitfest entry: https://commitfest.postgresql.org/patch/4817
---
src/backend/executor/nodeModifyTable.c | 40 +----------------------
src/include/executor/nodeModifyTable.h | 45 ++++++++++++++++++++++++++
2 files changed, 46 insertions(+), 39 deletions(-)
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 478cb01783c..85f3df7c09a 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -84,44 +84,6 @@ typedef struct MTTargetRelLookup
int relationIndex; /* rel's index in resultRelInfo[] array */
} MTTargetRelLookup;
-/*
- * Context struct for a ModifyTable operation, containing basic execution
- * state and some output variables populated by ExecUpdateAct() and
- * ExecDeleteAct() to report the result of their actions to callers.
- */
-typedef struct ModifyTableContext
-{
- /* Operation state */
- ModifyTableState *mtstate;
- EPQState *epqstate;
- EState *estate;
-
- /*
- * Slot containing tuple obtained from ModifyTable's subplan. Used to
- * access "junk" columns that are not going to be stored.
- */
- TupleTableSlot *planSlot;
-
- /*
- * Information about the changes that were made concurrently to a tuple
- * being updated or deleted
- */
- TM_FailureData tmfd;
-
- /*
- * The tuple deleted when doing a cross-partition UPDATE with a RETURNING
- * clause that refers to OLD columns (converted to the root's tuple
- * descriptor).
- */
- TupleTableSlot *cpDeletedSlot;
-
- /*
- * The tuple projected by the INSERT's RETURNING clause, when doing a
- * cross-partition UPDATE
- */
- TupleTableSlot *cpUpdateReturningSlot;
-} ModifyTableContext;
-
/*
* Context struct containing output data specific to UPDATE operations.
*/
@@ -868,7 +830,7 @@ ExecGetUpdateNewTuple(ResultRelInfo *relinfo,
* save the previous value to avoid losing track of it.
* ----------------------------------------------------------------
*/
-static TupleTableSlot *
+TupleTableSlot *
ExecInsert(ModifyTableContext *context,
ResultRelInfo *resultRelInfo,
TupleTableSlot *slot,
diff --git a/src/include/executor/nodeModifyTable.h b/src/include/executor/nodeModifyTable.h
index f6070e1cdf3..250bd64ad15 100644
--- a/src/include/executor/nodeModifyTable.h
+++ b/src/include/executor/nodeModifyTable.h
@@ -13,8 +13,47 @@
#ifndef NODEMODIFYTABLE_H
#define NODEMODIFYTABLE_H
+#include "access/tableam.h"
#include "nodes/execnodes.h"
+/*
+ * Context struct for a ModifyTable operation, containing basic execution
+ * state and some output variables populated by ExecUpdateAct() and
+ * ExecDeleteAct() to report the result of their actions to callers.
+ */
+typedef struct ModifyTableContext
+{
+ /* Operation state */
+ ModifyTableState *mtstate;
+ EPQState *epqstate;
+ EState *estate;
+
+ /*
+ * Slot containing tuple obtained from ModifyTable's subplan. Used to
+ * access "junk" columns that are not going to be stored.
+ */
+ TupleTableSlot *planSlot;
+
+ /*
+ * Information about the changes that were made concurrently to a tuple
+ * being updated or deleted
+ */
+ TM_FailureData tmfd;
+
+ /*
+ * The tuple deleted when doing a cross-partition UPDATE with a RETURNING
+ * clause that refers to OLD columns (converted to the root's tuple
+ * descriptor).
+ */
+ TupleTableSlot *cpDeletedSlot;
+
+ /*
+ * The tuple projected by the INSERT's RETURNING clause, when doing a
+ * cross-partition UPDATE
+ */
+ TupleTableSlot *cpUpdateReturningSlot;
+} ModifyTableContext;
+
extern void ExecInitGenerated(ResultRelInfo *resultRelInfo,
EState *estate,
CmdType cmdtype);
@@ -24,6 +63,12 @@ extern void ExecComputeStoredGenerated(ResultRelInfo *resultRelInfo,
CmdType cmdtype);
extern ModifyTableState *ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags);
+extern TupleTableSlot *ExecInsert(ModifyTableContext *context,
+ ResultRelInfo *resultRelInfo,
+ TupleTableSlot *slot,
+ bool canSetTag,
+ TupleTableSlot **inserted_tuple,
+ ResultRelInfo **insert_destrel);
extern void ExecEndModifyTable(ModifyTableState *node);
extern void ExecReScanModifyTable(ModifyTableState *node);
--
2.34.1
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: on_error table, saving error info to a table
2024-02-03 06:22 on_error table, saving error info to a table jian he <[email protected]>
2024-07-15 05:42 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
2024-08-20 00:00 ` Re: on_error table, saving error info to a table jian he <[email protected]>
2024-11-05 10:30 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
2024-12-03 04:28 ` Re: on_error table, saving error info to a table jian he <[email protected]>
2024-12-03 10:28 ` Re: on_error table, saving error info to a table Kirill Reshke <[email protected]>
2024-12-11 11:41 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
2024-12-13 08:26 ` Re: on_error table, saving error info to a table jian he <[email protected]>
2024-12-16 11:50 ` Re: on_error table, saving error info to a table Nishant Sharma <[email protected]>
2024-12-17 04:31 ` Re: on_error table, saving error info to a table Kirill Reshke <[email protected]>
2025-04-25 13:46 ` Re: on_error table, saving error info to a table jian he <[email protected]>
2026-05-25 08:13 ` Re: on_error table, saving error info to a table jian he <[email protected]>
@ 2026-05-28 22:41 ` Zsolt Parragi <[email protected]>
0 siblings, 0 replies; 13+ messages in thread
From: Zsolt Parragi @ 2026-05-28 22:41 UTC (permalink / raw)
To: [email protected]
Generally looks good to me, I only found a few typos:
+ errmsg("saving error information to table \"%s\" row due to
data type incompatibility at line %" PRIu64 " for column \"%s\":
\"%s\"",
Is row needed there?
+ * TODO: Allow cstate->error_rel to be a partitioned table. This should be
+ * not difficult, but requires proper handling of constraints and triggers
should not be difficult
+ privileges on it. During the error records inseration,
+ <literal>NOT NULL</literal> and <literal>CHECK</literal>
constraints are enforced,
+ and both row-level and statement-level triggers will be fired.
record's insertion
Maybe this could explicitly mention that failure to insert into the
error table will fail the copy statement? Or some better wording of
that, as it is allowed behavior with triggers.
^ permalink raw reply [nested|flat] 13+ messages in thread
end of thread, other threads:[~2026-05-28 22:41 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-02-03 06:22 on_error table, saving error info to a table jian he <[email protected]>
2024-07-15 05:42 ` Nishant Sharma <[email protected]>
2024-08-20 00:00 ` jian he <[email protected]>
2024-11-05 10:30 ` Nishant Sharma <[email protected]>
2024-12-03 04:28 ` jian he <[email protected]>
2024-12-03 10:28 ` Kirill Reshke <[email protected]>
2024-12-11 11:41 ` Nishant Sharma <[email protected]>
2024-12-13 08:26 ` jian he <[email protected]>
2024-12-16 11:50 ` Nishant Sharma <[email protected]>
2024-12-17 04:31 ` Kirill Reshke <[email protected]>
2025-04-25 13:46 ` jian he <[email protected]>
2026-05-25 08:13 ` jian he <[email protected]>
2026-05-28 22:41 ` Zsolt Parragi <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox