Re: Fix DEFAULT marker handling in COPY FROM ... HEADER MATCH

From: Ayoub Kazar <ayoub(dot)kazar(at)data-bene(dot)io>
To: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Fix DEFAULT marker handling in COPY FROM ... HEADER MATCH
Date: 2026-11-04 08:57:56
Message-ID: eb352a72-b490-453d-bd0e-48c5948460cd@data-bene.io
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,
Thanks for the review!

On 04/11/2026 07:54, Chao Li wrote:
> ERROR: column name mismatch in header line field 2: got null value ("a"), expected "a"
> CONTEXT: COPY t, line 1: "id,a"
> ```
>
> Here, a is a valid column name in the header line and should not be rejected simply because it matches the NULL marker.
>
> Adding an is_header parameter allows us to disable both NULL and DEFAULT marker recognition when parsing header lines, addressing both issues with the same approach. It also avoids temporarily modifying the COPY options.
>
> So, I’m proposing v2 with this approach. I kept the regression tests you added in v1 and added tests for NULL markers and the text format. Would you please review v2?
I have mixed feelings about this:

CREATE TABLE t (id int, "N" text);

If user inputs this in COPY:
id   \N
1    \N

same is in diffed existing regression test for it:

|-ERROR: column name mismatch in header line field 3: got null value
("\N"), expected "c" +ERROR: column name mismatch in header line field
3: got "N", expected "c"|

IMO, i prefer having this error out for NULL marker reason in a column
name, rather than reading '\N' as 'N' in header, and this might be the
intention of who wrote it... maybe.
I agree this should be data rows only, but the documentation doesn't
specify that either, at least not as expected as the case of DEFAULT
marker. So i'm not sure...

CSV case:
In CSV, CopyReadAttributesCSV() already checks `!saw_quote`. If a user has a
column named "a" and sets `null 'a'`, quoting the column name in the
header (`id,"a"`) already prevents it from being treated as NULL.

I have an extra worry about performance regressions, previously from
[1], i saw that very minimal changes effect the binary layout and
branching (without going into much details), therefore having 1-3%
performance regressions or improvements that appear just like that (this
applies everywhere in Postgres really...), CopyReadAttributesCSV() is
very sensitive, it runs millions of times very easily and its a high
registers-pressured code.

A not too serious benchmark on v2 already shows 5% regressions.

Regards,
Ayoub Kazar

[1]
https://www.postgresql.org/message-id/CAOzEurSW8cNr6TPKsjrstnPfhf4QyQqB4tnPXGGe8N4e_v7Jig%40mail.gmail.com

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2026-11-04 09:08:29 Re: Add compiler option -fstrict-flex-arrays=1
Previous Message Rahila Syed 2026-11-04 08:43:25 Re: Stale hash entries when reusing an auxiliary resource owner