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

From: jian he <jian(dot)universality(at)gmail(dot)com>
To: Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>
Cc: Jim Jones <jim(dot)jones(at)uni-muenster(dot)de>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: COPY ON_CONFLICT TABLE; save duplicated record to another table.
Date: 2026-05-12 08:15:19
Message-ID: CACJufxE9L5vVkBxjRZRjbpsGJcSLXM1oyayr_u10TA_ZfyFvmg@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, May 12, 2026 at 4:40 AM Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)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/

Attachment Content-Type Size
v3-0001-export-ExecInsert.patch text/x-patch 4.7 KB
v3-0002-COPY-ON_CONFLICT-TABLE.patch text/x-patch 63.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Maksim.Melnikov 2026-05-12 08:30:55 Re: Incorrect checksum in control file with pg_rewind test
Previous Message Alexander Pyhalov 2026-05-12 08:09:23 Re: Function scan FDW pushdown