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

> On Nov 4, 2026, at 16:57, Ayoub Kazar <ayoub(dot)kazar(at)data-bene(dot)io> wrote:
>
> 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
>

Good point. I hadn’t considered this case. This would incorrectly allow COPY to succeed, even though \N in the header line should cause it to fail.

I believe this can be resolved if we pursue v2’s approach, but some refactoring will likely be needed.

> 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.
>

That’s true, but I think v2 would remove this restriction, so users would no longer have to quote column names in the header line just because they match the NULL marker. I consider that an improvement.

>
> 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.

Could you share how you ran the benchmark and measured the 5% regression?

I ran a brief benchmark with assertions disabled and -O2. The test copies 1 million rows into a table with 32 text columns. The results are:
```
| Version | Format | Runs | Average time | Change vs master |
|---|---|---:|---:|---:|
| Master (`590788ad0bf`) | CSV | 6 | 625.382 ms | — |
| Master | Text | 5 | 627.275 ms | — |
| Master | Binary | 5 | 795.974 ms | — |
| v2 | CSV | 5 | 641.186 ms | +2.53% |
| v2 | Text | 5 | 607.293 ms | −3.19% |
| v2 | Binary | 6 | 785.810 ms | −1.28% |
```

These results don’t show a clear performance regression to me, though this was only a brief benchmark.

Overall, I agree that point 1 has to be addressed, and doing so would require more work beyond the scope of this patch. Let’s drop v2 for now; I’ll propose it in a separate thread.

The attached v3 restores the v1 fix. I only adjusted the commit message formatting and added author/reviewer annotations.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/