Re: on_error table, saving error info to a table

jian he <jian.universality@gmail.com>

From: jian he <jian.universality@gmail.com>
To: Zsolt Parragi <zsolt.parragi@percona.com>
Cc: pgsql-hackers@lists.postgresql.org
Date: 2026-05-25T08:13:44Z
Lists: pgsql-hackers

Attachments

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