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
- v13-0002-COPY-FROM-on_error-table-error_table-errtbl.patch (text/x-patch)
- v13-0001-export-ExecInsert.patch (text/x-patch)
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/