Re: on_error table, saving error info to a table

From: jian he <jian(dot)universality(at)gmail(dot)com>
To: Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: on_error table, saving error info to a table
Date: 2026-05-25 08:13:44
Message-ID: CACJufxFd1pbip3SXcwTshWN21fCWOknKQHZ5c8tVYfPGWPQ-7g@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, May 15, 2026 at 7:32 PM Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> 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/

Attachment Content-Type Size
v13-0002-COPY-FROM-on_error-table-error_table-errtbl.patch text/x-patch 56.8 KB
v13-0001-export-ExecInsert.patch text/x-patch 4.8 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2026-05-25 08:21:49 Re: NULL pointer dereference in syslogger with load_libraries() and -DEXEC_BACKEND at startup
Previous Message Shinya Kato 2026-05-25 08:09:47 Re: Use pg_current_xact_id() instead of deprecated txid_current()