Re: Allow COPY's 'text' format to output a header

From: Simon Muller <samullers(at)gmail(dot)com>
To: Cynthia Shang <cynthia(dot)shang(at)crunchydata(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Allow COPY's 'text' format to output a header
Date: 2018-07-25 22:09:43
Message-ID: CAF1-J-04vbUk2L11NEUfUMQP=e5VX9B=-NYAEg+Dm46a8Nvq7w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 25 July 2018 at 19:24, Cynthia Shang <cynthia(dot)shang(at)crunchydata(dot)com>
wrote:

>
> I've reviewed this patch and feel this patch addresses the original ask. I
> tested it manually trying to break it and, as mentioned previously, it's
> behavior is the same as the CSV copy with regards to it's shortcomings.
> However, I feel
> 1) a "copy from" test is needed and
> 2) the current "copy to" test is (along with a few others) in the wrong
> file.
>
> With regards to #2, the copy.source tests are for things requiring
> replacement when running the tests. Given that these copy tests do not, I
> have moved the current last set of copy tests to the copy2.sql file and
> have provided an attached patch.
>
>
Thanks for reviewing the patch.

I agree that moving those previous and these new tests out of the .source
files seems to make more sense as they don't make use of the
preprocessing/replacement feature.

With regards to #1, the patch I have provided can then be used and the
> following added as the COPY TO/FROM tests (perhaps after line 426 of the
> attached copy2.sql file). Note that I moved the FROM test before the TO
> test and omitted the "(format text, header true)" in the FROM test since it
> is another way the command can be invoked.
>
> copy copytest3 from stdin header;
> this is just a line full of junk that would error out if parsed
> 11 a 1
> 22 b 2
> \.
>
> copy copytest3 to stdout with (format text, header true);
>
>
>
I've incorporated both your suggestions and included the patch you provided
in the attached patch. Hope it's as expected.

> As for the matching check of the header in the discussion of this patch, I
> feel that is a separate patch that can be added later since it would affect
> the general functionality of the copy command, not just the ability to have
> a text header.
>
> Best,
> - Cynthia Shang
>

P.S. I did receive the first attached patch, but on my Ubuntu I had to
apply it using "git apply --ignore-space-change --ignore-whitespace",
probably due to line ending differences.

--
Simon Muller

Attachment Content-Type Size
text_header_v4.patch application/octet-stream 6.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2018-07-25 22:11:13 Re: LLVM jit and matview
Previous Message Andres Freund 2018-07-25 21:47:02 Re: LLVM jit and matview