Re: csv format for psql

    Fabien COELHO wrote:

> 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.

It's not that accepting -C brings much value by itself, it's that
loosing the consistency across all options comes with a negative cost.
The point is that up to now all options have a short form and a long
form, so --csv would be a deliberate exception. I'm rather unconvinced
it's justified, but I seem to be alone in that case, so I'll comply.


> 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

The trouble with fieldsep is that it defaults to '|', which
both you and Pavel say you dislike. Fair enough, it's better
to have ',' by default, but the cleanest solution to that
is fieldsep_csv with its own default.
The solution to set fieldsep automatically to ',' with
\pset format csv is problematic.

For instance
 \pset format csv
 \pset fieldsep ';'

changes fieldsep to ';' as expected, but in the other order

 \pset fieldsep ';'
 \pset format csv

you get ',' while reasonably you'd expect ';'

Same problem on the command line. Options are evaluated left-to-right:

 $ psql --csv -F';'
would work as expected, but
 $ psql -F';' --csv
would not.

I don't feel good about solving these issues with ad-hoc rules,
or inventing the notion that a pset variable has been defined
but not user-redefined. This stuff has "inconsistent" written all over it
and I don't see a maintainer going along with that.

> 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.

Interesting point. The output stream is opened in text mode so printing
'\n' should generate LF on Unix, CR LF on Windows, and I think CR on MacOS.
I think that's for the best.

recordsep in the unaligned mode doesn't play the role of a line ending
because the last line is not finished by recordsep. According to the source
code, this is intended, see print_unaligned_text() in print.c:
  /*
   * The last record is terminated by a newline, independent of the set
   * record separator.    But when the record separator is a zero byte, we
   * use that (compatible with find -print0 and xargs).
   */


> 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.

ok

> 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.

ok

> 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 ,)...

I'll do but the printout code is type-agnostic so it's not supposed
to make a difference compared to mere literals.
Cases with NULLs are missing though, I'll go add some too.


Best regards,
--
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite