On Thu, Aug 09, 2018 at 10:37:28AM -0400, Cynthia Shang wrote: > This patch looks good. I realized I should have changed the status > back while we were discussing all this. It is now (and still is) ready > for committer.
I have some comments. -ERROR: COPY HEADER available only in CSV mode +ERROR: cannot specify HEADER in BINARY mode This should read "COPY HEADER not available in BINARY mode" perhaps? +copy copytest3 from stdin csv header; +copy copytest3 to stdout csv header; It would be more interesting to first export the data into the file with a header, truncate the relation, and import it back with again header specified. The data of the original should match the new, for both text and csv format. CopyStateData defines header_line, which still assumes that only CSV is supported. Why are there no additional tests for file_fdw? The point about the header matching mentioned upthread is quite interesting as it could make the proposed feature way more useful, and it has not really been discussed. As far as I can see this adds more sanity checks in NextCopyFromRawFields(). I'd like to think that this should be a completely different option, say CHECK_HEADER, as CSV simply skips the header in COPY FROM if specified on HEAD. -- Michael
signature.asc
Description: PGP signature
