Re: on_error table, saving error info to a table

On Fri, May 15, 2026 at 1:14 AM Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> wrote:
>
> SET debug_discard_caches = 1;
> CREATE TABLE src_tbl (a int, b int);
> CREATE TABLE err_tbl OF copy_error_saving;
> CREATE INDEX src_idx ON src_tbl(a);
> COPY src_tbl FROM STDIN (FORMAT csv, ON_ERROR table, ERROR_TABLE err_tbl);
> 1,2
> xx,3
> 3,4
> \.
> -- ERROR: relation with OID 0 does not exist
>
Yech. We need do this in EndCopyFrom
+ if (cstate->error_rel)
+ table_close(cstate->error_rel, NoLock);

>
> + /* Handle queued AFTER triggers */
> + AfterTriggerEndQuery(cstate->mtcontext->estate);
>
> Is the order of this correct? See the following snippet that crashes the server:
>
> CREATE TABLE target_tbl (id int, val int);
> CREATE TABLE err_tbl OF copy_error_saving;
>
> CREATE OR REPLACE FUNCTION err_stmt_trans_fn() RETURNS trigger AS $$
> BEGIN
> END;
> $$ LANGUAGE plpgsql;
>
> CREATE TRIGGER err_stmt_trans
> AFTER INSERT ON err_tbl
> REFERENCING NEW TABLE AS new_rows
> FOR EACH STATEMENT EXECUTE FUNCTION err_stmt_trans_fn();
>
> \echo === COPY ===
> COPY target_tbl FROM stdin WITH (on_error 'table', error_table 'err_tbl');
> 1 100
> bad 200
> 3 notanumber
> 4 400
> \.
>

We need do AfterTriggerEndQuery(cstate->mtcontext->estate);
first then
AfterTriggerEndQuery(estate);

ExecForPortionOfLeftovers() handles this in the same way.

``static AfterTriggersData afterTriggers;``
and ExecASInsertTriggers, AfterTriggerEndQuery will change the value
of afterTriggers constantly,
therefore, the position of these functions is important!

Therefore in copyfrom.c the order should be:
ExecBSInsertTriggers(estate, resultRelInfo);
ExecBSInsertTriggers(cstate->mtcontext->mtstate->ps.state,
cstate->mtcontext->mtstate->rootResultRelInfo);
ExecASInsertTriggers(cstate->mtcontext->estate,
on_error_mtstate->rootResultRelInfo,
on_error_mtstate->mt_transition_capture);
ExecASInsertTriggers(estate, target_resultRelInfo, cstate->transition_capture);

> +
> + cstate->num_errors = cstate->num_errors + estate->es_processed;
>
> Counting seems to miss if a before trigger returns null:
>
> \set ON_ERROR_STOP 0
> CREATE TABLE t2 (a int, b int, c int);
> CREATE TABLE err_tbl2 OF copy_error_saving;
> CREATE FUNCTION drop_all() RETURNS TRIGGER LANGUAGE plpgsql AS $$
> BEGIN
> RETURN NULL;
> END;
> $$;
> CREATE TRIGGER drop_all_t BEFORE INSERT ON err_tbl2 FOR EACH ROW
> EXECUTE FUNCTION drop_all();
> COPY t2 FROM STDIN WITH (FORMAT csv, ON_ERROR table, ERROR_TABLE err_tbl2);
> 1,2,a
> 3,4,b
> 5,6,c
> 7,8,d
> 9,10,e
> \.
>
> SELECT count(*) AS n FROM t2;
> SELECT count(*) AS n FROM err_tbl2;
>
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.

>
> + typoid = GetSysCacheOid2(TYPENAMENSP, Anum_pg_type_oid,
> + PointerGetDatum("copy_error_saving"),
> + ObjectIdGetDatum(PG_CATALOG_NAMESPACE));
> ...
> + if (reloftype != typoid)
> + ereport(ERROR,
> ...
> + errhint("The COPY error saving table must be a
> typed table based on type \"%s\".",
> + format_type_be_qualified(typoid)));
>
> Isn't an if (!OidIsValid(typoid)) check missing between the two?

Sure, I have added a
+ if (!OidIsValid(typoid))
+ elog(ERROR, "cache lookup failed for catalog type %s",
"copy_error_saving");

--
jian
https://www.enterprisedb.com/