2018-03-08 11:17 GMT+01:00 Pavel Stehule <pavel.steh...@gmail.com>: > > > 2018-03-08 11:05 GMT+01:00 Fabien COELHO <coe...@cri.ensmp.fr>: > >> >> Hello Daniel, >> >> PFA a v3 patch that implements that, along with regression tests this >>> time. >>> >> >> My 0.02 €: >> >> Patch applies cleanly, compiles, make check ok, doc generation ok. >> >> I'm in favor of having a simple psql way to generate a convenient and >> compliant CSV output for export/import. >> >> I also think that a short option brings little value, and "--csv" is good >> enough for the purpose, so I would agree to remove the "-C" binding. >> >> About "fieldsep_csv", I do not like much the principle of having >> different output variables to represent the same concept depending on the >> format. I would rather have reused fieldsep as in your previous submission >> and set it to "," when under --csv. This is not a strong opinion and other >> people may differ: the committer opinion is the one to follow:-) >> > Looks like complex rule. But maybe it is acceptable. If the format is csv, and fieldsep is not defined, then use ','. If it defined, then use defined field sep. I am not sure, if is possible distinct between default and explicitly defined field separator.
If not, then I prefer fieldsep_csv. > >> The "\n" eol style is hardcoded. Should it use "recordsep"? For instance, >> https://tools.ietf.org/html/rfc4180 seems to specify CRLF end of lines. >> The definition is evolving, eg https://www.w3.org/TR/tabular-data-model/ >> accepts both "\r" and "\r\n". I do not like using windows eol, but I think >> that it should be possible to do it, which is not the case with this >> version. >> > > In this case recordsep is not problem - default is ok. > > >> >> The "\pset format" error message in "do_pset" shows values in seemingly >> random order. The situation is pre-existing but not really satisfactory. >> I'd suggest to put all values in alphabetical order. >> >> In csv_print_field & csv_print_text, you are not consistent when putting >> braces on blocks with only one instruction. I'd suggest not to put braces >> in that case. >> >> I'd suggest that tests should include more types, not just strings. I >> would suggest int, float, timestamp, bytea, an array (which uses , as a >> separator), json (which uses both " and ,)... >> >> -- >> Fabien. > > >
