On 25 July 2018 at 19:24, Cynthia Shang <cynthia.sh...@crunchydata.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
text_header_v4.patch
Description: Binary data
