Thread

1. Re: COPY ON_CONFLICT TABLE; save duplicated record to another table.

   Jim Jones <jim.jones@uni-muenster.de> — 2026-05-05T10:06:44Z

   Hi Jian
   
   On 25/04/2026 06:12, jian he wrote:
   > Comments are welcome!
   
   Thanks for the patch!
   
   A few comments:
   
   == double defGet in function name ==
   
   defGetdefGetCopyOnConflictChoice should probably be 
   defGetCopyOnConflictChoice
   
   == identical error message in ProcessCopyOptions() ==
   
   The same error message is raised for conflict_tbl_specified and 
   !conflict_tbl_specified
   
   ereport(ERROR,
      errcode(ERRCODE_INVALID_PARAMETER_VALUE),
      errmsg("COPY %s requires %s option", "CONFLICT_TABLE", "ON_CONFLICT"));
   
   == unused enum in CopyOnErrorChoice ==
   
   The enum COPY_ON_ERROR_TABLE is introduced, but is never used anywhere.
   
   == unconditional ExecOpenIndices() ==
   
   ExecOpenIndices(resultRelInfo, true);
   
   I'm not very familiar with this part of the code, but it looks like that 
   this change would affect other COPY FROM operations. If I'm mistaken, a 
   comment would add some value here. Or perhaps something like:
   
   ExecOpenIndices(resultRelInfo, cstate->opts.on_conflict != ONCONFLICT_NONE);
   
   == ON_CONFLICT TABLE not rejected in COPY TO ==
   
   CONFLICT_TABLE is silently ignored, even if the table does not exist:
   
   postgres=# COPY t TO '/dev/null' (ON_CONFLICT TABLE, CONFLICT_TABLE 
   table_does_not_exist);
   COPY 1
   
   I guess adding a is_from to defGetdefGetCopyOnConflictChoice() is the 
   way to go.
   
   == redundant condition in CopyFrom() ==
   
   The second condition seems unnecessary, as the previous if already tests 
   for cstate->opts.on_conflict == ONCONFLICT_NONE:
   
   else if (resultRelInfo->ri_NumIndices > 0 &&
      cstate->opts.on_conflict != ONCONFLICT_NONE)
   
   == typos ==
   regular realtion > regular relation
   vertification > verification
   resouces > resources
   unqiue > unique
   
   == unnecessary pnstrdup (?) ==
   
   newvalues[i] = CStringGetTextDatum(pnstrdup(cstate->line_buf.data,
                                                cstate->line_buf.len));
   
   Is the duplication really necessary? Wouldn't it suffice to use 
   cstring_to_text_with_len() instead? Something like:
   
   newvalues[i] = 
   PointerGetDatum(cstring_to_text_with_len(cstate->line_buf.data, 
   cstate->line_buf.len));
   
   or even
   
   newvalues[i] = CStringGetTextDatum(cstate->line_buf.data)
   
   I'll check the documentation after we get more feedback on the syntax.
   
   Thanks!
   Best, Jim
   
   
   
   
   

2. Re: COPY ON_CONFLICT TABLE; save duplicated record to another table.

   Zsolt Parragi <zsolt.parragi@percona.com> — 2026-05-06T22:17:35Z

   Hello!
   
   I tried the patch and found a few issues.
   
   1. Two of them are null pointer dereference crashes, one with
   partitioned tables:
   
   CREATE TABLE part_t (a int PRIMARY KEY, b text) PARTITION BY RANGE (a);
   CREATE TABLE part_t_p1 PARTITION OF part_t FOR VALUES FROM (0) TO (1000);
   CREATE TABLE conflict_log (
       rel        oid,
       file_name  text,
       line_no    bigint,
       raw_line   text
   );
   
   INSERT INTO part_t VALUES (1, 'pre-existing');
   COPY part_t (a, b) FROM stdin WITH (on_conflict 'table',
   conflict_table 'conflict_log');
   2 row-two
   1 dup
   3 row-three
   \.
   
   
   2. And another with repeateable reads:
   
   CREATE TABLE t_rr (a int PRIMARY KEY, b text);
   CREATE TABLE conflict_log (rel oid, fname text, ln bigint, raw text);
   INSERT INTO t_rr VALUES (1, 'pre-committed');
   
   BEGIN ISOLATION LEVEL REPEATABLE READ;
   COPY t_rr FROM stdin WITH (on_conflict 'table', conflict_table 'conflict_log');
   1 dup-row
   \.
   
   
   3. There's also a possible data loss scenario, reports 3 copied 0 actual:
   
   CREATE TABLE conf_log (
       relname     oid,
       fname       text,
       lineno      bigint,
       rawline     text
   );
   
   CREATE TABLE no_idx_tgt (id int, payload text);
   
   CREATE FUNCTION noop_trig() RETURNS trigger LANGUAGE plpgsql AS $$
   BEGIN
       RETURN NEW;
   END;
   $$;
   
   CREATE TRIGGER noop_before BEFORE INSERT ON no_idx_tgt
       FOR EACH ROW EXECUTE FUNCTION noop_trig();
   
   COPY no_idx_tgt (id, payload) FROM STDIN
       WITH (ON_CONFLICT TABLE, CONFLICT_TABLE conf_log);
   1 alpha
   2 beta
   3 gamma
   \.
   
   SELECT 'A: no_idx_tgt count' AS scenario, count(*) AS rows FROM no_idx_tgt;
   SELECT 'A: conf_log count'  AS scenario, count(*) AS rows FROM conf_log;
   SELECT * FROM no_idx_tgt ORDER BY id;
   
   
   4. Shouldn't the following error out?
   
   CREATE TABLE t (a int PRIMARY KEY, b text);
   COPY t TO '/dev/null' (ON_CONFLICT TABLE, CONFLICT_TABLE no_such_table);
   
   
   
   

3. Re: COPY ON_CONFLICT TABLE; save duplicated record to another table.

   jian he <jian.universality@gmail.com> — 2026-05-11T03:13:15Z

   Hi.
   
   The attached patch should address most, if not all, of the issues you
   both raised.
   
   As explained in [1], we can export ExecInsert to let it perform the
   main insertion work.
   To allow ExecInsert to handle the remaining tasks, we need to carefuly manage
   the lifecycle of constructed CopyFromStateData->ModifyTableContext (including
   ModifyTableContext->EState): populate it, use it, and then release it.
   
   Since ExecInsert already contains the necessary infrastructure for INSERT ON
   CONFLICT DO NOTHING/SELECT, exporting it avoids duplicating that logic in
   src/backend/commands/copyfrom.c (which is what v1 of the patch did).
   
   [1]: https://postgr.es/m/CACJufxH_NbPuA+O5YR7xP4xDZ+iHkO2VFkddhrhBz+4-EUTp7w@mail.gmail.com
   
   The exclusion unique constraint issue is still not resolved.... but,
   overall v2 is better than v1, IMHO.
   
   
   
   --
   jian
   https://www.enterprisedb.com/
   

4. Re: COPY ON_CONFLICT TABLE; save duplicated record to another table.

   Jim Jones <jim.jones@uni-muenster.de> — 2026-05-11T09:25:47Z

   Hi Jian
   
   On 11/05/2026 05:13, jian he wrote:
   > The attached patch should address most, if not all, of the issues you
   > both raised.
   
   Thanks for the update. All my points were addressed.
   
   > As explained in [1], we can export ExecInsert to let it perform the
   > main insertion work.
   > To allow ExecInsert to handle the remaining tasks, we need to carefuly manage
   > the lifecycle of constructed CopyFromStateData->ModifyTableContext (including
   > ModifyTableContext->EState): populate it, use it, and then release it.
   > 
   > Since ExecInsert already contains the necessary infrastructure for INSERT ON
   > CONFLICT DO NOTHING/SELECT, exporting it avoids duplicating that logic in
   > src/backend/commands/copyfrom.c (which is what v1 of the patch did).
   > 
   > [1]: https://postgr.es/m/CACJufxH_NbPuA+O5YR7xP4xDZ+iHkO2VFkddhrhBz+4-
   > EUTp7w@mail.gmail.com
   > 
   > The exclusion unique constraint issue is still not resolved.... but,
   > overall v2 is better than v1, IMHO.
   
   
   One other thing I just noticed in BINARY mode. I believe we're missing a
   check in ProcessCopyOptions() with ON_CONFLICT TABLE to show a proper
   error message, e.g.
   
   /* Check on_conflict */
   if (opts_out->format == COPY_FORMAT_BINARY && opts_out->on_conflict !=
   ONCONFLICT_NONE)
     ereport(ERROR,
       (errcode(ERRCODE_SYNTAX_ERROR),
        errmsg("cannot specify %s in BINARY mode", "ON_CONFLICT TABLE")));
   
   postgres=# COPY t FROM STDIN (FORMAT binary, ON_CONFLICT TABLE,
   CONFLICT_TABLE ctbl);
   ERROR:  cannot specify ON_CONFLICT TABLE in BINARY mode
   
   Right now the error is rather vague:
   
   ERROR:  COPY file signature not recognized
   
   What do you think?
   
   Thanks!
   
   Best, Jim
   
   
   
   
   

5. Re: COPY ON_CONFLICT TABLE; save duplicated record to another table.

   Zsolt Parragi <zsolt.parragi@percona.com> — 2026-05-11T20:40:18Z

   Hello!
   
   > One other thing I just noticed in BINARY mode. I believe we're missing a
   > check in ProcessCopyOptions() with ON_CONFLICT TABLE to show a proper
   > error message, e.g.
   
   It is possible to crash the current patch with binary mode, that check
   is definitely needed.
   
   
   + MakeTransitionCaptureState(cstate->conflictRel->trigdesc,
   +    RelationGetRelid(cstate->conflictRel),
   +    CMD_INSERT);
   
   Shouldn't this update mtstate->mt_transition_capture?
   
   + if (cstate->opts.on_conflict == ONCONFLICT_TABLE)
   ...
   + if (conflict_mstate->fireBSTriggers)
   + {
   + ExecBSInsertTriggers(conflict_mstate->ps.state,
   conflict_mstate->rootResultRelInfo);
   +
   + conflict_mstate->fireBSTriggers = false;
   + }
   +
   
   and
   
   + if (cstate->num_conflicts > 0)
   + {
   ...
   + /* Execute AFTER STATEMENT insertion triggers */
   + ExecASInsertTriggers(cstate->mtcontext->estate,
   + on_conflict_mtstate->rootResultRelInfo,
   + on_conflict_mtstate->mt_transition_capture);
   
   
   * Doesn't statements typically fire triggers unconditionally? INSERT
   ON CONFLICT DO NOTHING; fires BS+AS triggers even if it doesn't insert
   any rows.
   * Isn't firing a before statement trigger after some before/after row
   triggers were already fired (for non conflicting rows) strange?
   
   + if (valid_col_count != 4)
   + errdetail_msg = _("The conflict_table is incomplete; exactly four
   columns are required.");
   
   if valid_col_count > 4, is it still incomplete, shouldn't the error
   message change in that case?
   
   
   
   

6. Re: COPY ON_CONFLICT TABLE; save duplicated record to another table.

   jian he <jian.universality@gmail.com> — 2026-05-12T08:15:19Z

   On Tue, May 12, 2026 at 4:40 AM Zsolt Parragi <zsolt.parragi@percona.com> wrote:
   >
   > Hello!
   >
   > > One other thing I just noticed in BINARY mode. I believe we're missing a
   > > check in ProcessCopyOptions() with ON_CONFLICT TABLE to show a proper
   > > error message, e.g.
   >
   > It is possible to crash the current patch with binary mode, that check
   > is definitely needed.
   >
   
   binary mode lacks the concept of a line number or the whole line string.
   Since cstate->line_buf is null in binary mode, it will segfault in
   ```CStringGetTextDatum(cstate->line_buf.data);```
   
   Supporting ON_CONFLICT in binary mode is not trivial.
   Since ON_ERROR IGNORE also cannot be used in binary mode, not
   supporting ON_CONFLICT in binary mode should be fine, IMHO.
   
   >
   > + MakeTransitionCaptureState(cstate->conflictRel->trigdesc,
   > +    RelationGetRelid(cstate->conflictRel),
   > +    CMD_INSERT);
   >
   > Shouldn't this update mtstate->mt_transition_capture?
   >
   
   Attached v3 fix this issue.
   
   > + if (cstate->opts.on_conflict == ONCONFLICT_TABLE)
   > ...
   > + if (conflict_mstate->fireBSTriggers)
   > + {
   > + ExecBSInsertTriggers(conflict_mstate->ps.state,
   > conflict_mstate->rootResultRelInfo);
   > +
   > + conflict_mstate->fireBSTriggers = false;
   > + }
   > +
   >
   > and
   >
   > + if (cstate->num_conflicts > 0)
   > + {
   > ...
   > + /* Execute AFTER STATEMENT insertion triggers */
   > + ExecASInsertTriggers(cstate->mtcontext->estate,
   > + on_conflict_mtstate->rootResultRelInfo,
   > + on_conflict_mtstate->mt_transition_capture);
   >
   >
   > * Doesn't statements typically fire triggers unconditionally? INSERT
   > ON CONFLICT DO NOTHING; fires BS+AS triggers even if it doesn't insert
   > any rows.
   > * Isn't firing a before statement trigger after some before/after row
   > triggers were already fired (for non conflicting rows) strange?
   >
   
   Ok. I changed to
   Statement-level triggers on the CONFLICT_TABLE are fired
   unconditionally, regardless of whether an error occurred or not.
   Each row inserted into the CONFLICT_TABLE will fire both the BEFORE
   INSERT FOR EACH ROW and AFTER INSERT FOR EACH ROW triggers.
   
   > + if (valid_col_count != 4)
   > + errdetail_msg = _("The conflict_table is incomplete; exactly four
   > columns are required.");
   >
   > if valid_col_count > 4, is it still incomplete, shouldn't the error
   > message change in that case?
   
   With v3, whether there are more or fewer columns on the
   conflict_table, the error message is now the same:
   
   +ERROR:  cannot use relation "err_tbl1" for COPY on_conflict error saving
   +DETAIL:  The conflict_table should only have four columns
   +HINT:  The conflict_table must contain exactly four columns with data
   types, in order: OID, TEXT, BIGINT, TEXT
   
   Regression tests for permission checks have also been added.
   
   
   
   --
   jian
   https://www.enterprisedb.com/
   

7. Re: COPY ON_CONFLICT TABLE; save duplicated record to another table.

   Jim Jones <jim.jones@uni-muenster.de> — 2026-05-12T13:27:14Z

   Hi Jian
   
   On 12/05/2026 10:15, jian he wrote:
   > Attached v3 fix this issue.
   
   == closing relation too early ==
   
   In noticed that in BeginCopyFrom() you open cstate->conflictRel and
   close it after the CopyFromConflictTableCheck() call
   
   cstate->conflictRel = table_open(conflictRelid, NoLock);
   CopyFromConflictTableCheck(cstate);
   table_close(cstate->conflictRel, NoLock);
   
   But if we take a look at DoCopy(), the function CopyFrom() is called
   immediately after a BeginCopyFrom() call, which now references to a
   closed relation in cstate->conflictRel. I guess we should close the
   relation in EndCopyFrom().
   
   == redundant if blocks ==
   
   These two if (cstate->opts.on_conflict == ONCONFLICT_TABLE) could be merged:
   
   if (cstate->opts.on_conflict == ONCONFLICT_TABLE)
   {
     node->onConflictAction = ONCONFLICT_NOTHING;
     node->canSetTag = true;
   }
   
   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_conflict == ONCONFLICT_TABLE)
     conflictslot = ExecInitExtraTupleSlot(estate,
       RelationGetDescr(cstate->conflictRel),
       &TTSOpsVirtual);
   
   == comments ==
   ON_CONFLCT > ON_CONFLICT
   unqiue > unique
   
   IIUC, it should be here (copy.h) "on conflict" instead of "on error", right?
   
   char	   *on_conflictRel; /* on error, save error info to the table,
   								 * table name */
   
   Thanks!
   Best, Jim
   
   
   
   
   

8. Re: COPY ON_CONFLICT TABLE; save duplicated record to another table.

   Zsolt Parragi <zsolt.parragi@percona.com> — 2026-05-15T11:56:46Z

   I also noticed the early relation close mentioned by Jim, which can
   crash the patch.
   
   +      This uses the same mechanism as <link
   linkend="sql-on-conflict"><command>INSERT ... ON
   CONFLICT</command></link>.
   +      However, exclusion constraints are not supported; only
   <literal>NOT DEFERRABLE</literal>
   +      unique constraints are checked for violations.
   
   EXCLUDE USING gist (... WITH =, ... WITH &&) seems to work fine?
   Except that the message mentions unique constraint violation.
   
   I also checked the same trigger behaviors as in the other thread[1],
   especially before triggers on the conflict table, and this patch
   behaves similarly, it silently drops rows.
   I think this could also use some more visibility/documentation about that.
   
   1: https://www.postgresql.org/message-id/CAN4CZFPoohFvQTSE0wC%2BwcrfYiZOxFmUdOq0%2B9TCVR6Hk8n6iw%40mail.gmail.com
   
   
   
   

9. Re: COPY ON_CONFLICT TABLE; save duplicated record to another table.

   jian he <jian.universality@gmail.com> — 2026-05-27T14:06:27Z

   On Fri, May 15, 2026 at 7:56 PM Zsolt Parragi <zsolt.parragi@percona.com> wrote:
   >
   > I also noticed the early relation close mentioned by Jim, which can
   > crash the patch.
   >
   fixed.
   
   > +      This uses the same mechanism as <link
   > linkend="sql-on-conflict"><command>INSERT ... ON
   > CONFLICT</command></link>.
   > +      However, exclusion constraints are not supported; only
   > <literal>NOT DEFERRABLE</literal>
   > +      unique constraints are checked for violations.
   >
   > EXCLUDE USING gist (... WITH =, ... WITH &&) seems to work fine?
   > Except that the message mentions unique constraint violation.
   >
   
   I double-checked ExecCheckIndexConstraints, ExecInsertIndexTuples and
   added some dummy regression tests to confirm
   that INSERT ON CONFLICT DO NOTHING works fine with exclusion constraints.
   
   > I also checked the same trigger behaviors as in the other thread[1],
   > especially before triggers on the conflict table, and this patch
   > behaves similarly, it silently drops rows.
   > I think this could also use some more visibility/documentation about that.
   >
   > 1: https://www.postgresql.org/message-id/CAN4CZFPoohFvQTSE0wC%2BwcrfYiZOxFmUdOq0%2B9TCVR6Hk8n6iw%40mail.gmail.com
   >
   With the attached v4, row-level and statement-level triggers are now
   fired for every insertion to conflict_table
   
   In v3, there was a performance regression when a table don't have any unique or
   exclusion constraint, but ON_CONFLICT was still specified as 'TABLE'.
   I have attached an SQL test script demonstrating this.
   With v4, this regression is now very very minimal for COPY operations where
   ON_CONFLICT is set to 'TABLE' on a target table without any unique or exclusion
   constraints.
   
   I also polished the documentation.
   
   Comments from Jim Jones also addressed.
   
   
   
   --
   jian
   https://www.enterprisedb.com/