Re: Add header support to text format and matching feature

From: Rémi Lapeyre <remi(dot)lapeyre(at)lenstra(dot)fr>
To: Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>
Cc: Zhihong Yu <zyu(at)yugabyte(dot)com>, David Steele <david(at)pgmasters(dot)net>, Michael Paquier <michael(at)paquier(dot)xyz>, vignesh C <vignesh21(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Daniel Verite <daniel(at)manitou-mail(dot)org>
Subject: Re: Add header support to text format and matching feature
Date: 2022-01-30 22:56:57
Message-ID: 07891B9D-2140-4C06-AF96-C14F1E1B0E8E@lenstra.fr
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


> On 28 Jan 2022, at 09:57, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> wrote:
>
> On 31.12.21 18:36, Rémi Lapeyre wrote:
>> Here’s an updated version of the patch that takes into account the changes in d1029bb5a2. The actual code is the same as v10 which was already marked as ready for committer.
>
> I have committed the 0001 patch. I will work on the 0002 patch next.
>

Thanks!

> I notice in the 0002 patch that there is no test case for the error "wrong header for column \"%s\": got \"%s\"", which I think is really the core functionality of this patch. So please add that.
>

I added a test for it in this new version of the patch.

> I wonder whether the header matching should be a separate option from the HEADER option. The option parsing in this patch is quite complicated and could be simpler if there were two separate options. It appears this has been mentioned in the thread but not fully discussed.

I suppose a new option could be added but I’m not sure it would simplify things much with regard to the code and in my opinion it would be a bit weirder for users, right now it is just:

copy my_table from stdin with (header match);

with an additional option it could be:

copy my_table from stdin with (header true, match);

with potentially “header true” being implicit when “match” is given:

copy my_table from stdin with (match);

But I think we would still have to check for and return an error if the user inputs:

copy my_table from stdin with (header off, match);

Rather than complicating things, the current implementation seemed to be the best but I will update the patch if you think I should change it.

Best regards,
Rémi

Attachment Content-Type Size
v12-0001-Add-header-matching-mode-to-COPY-FROM.patch application/octet-stream 14.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Noah Misch 2022-01-30 23:14:32 Re: plperl on windows
Previous Message Andres Freund 2022-01-30 22:44:20 Re: pg_basebackup WAL streamer shutdown is bogus - leading to slow tests