Re: COPY-able sql log outputs

FAST PostgreSQL wrote:
>> Am Dienstag, 3. April 2007 20:33 schrieb FAST PostgreSQL:
>>> Attached is the completed patch for the COPY-able sql log outputs.
>> Could you please remove random whitespace changes from this patch?
>
> Done and attached.

A couple of comments.

First one that's not at all about the code - but your emails appear to
be dated several days into the future. This one, for example, is
datestamped 2007-04-04. AFAIK timezones only go 13 (or is it 14?) hours
:) You'll want to look into that. (otherwise, it'll look like you missed
feature freeze..)


Looking at the code, I was first going to ask what happens when you
enable both redirect_stderr and this one - would the data be put in
random order in the same file. But looking at the code, it seems you're
appending ".sql" to the filename for this log. Two comments about that:

1) This is not mentioned anywhere in the docs or comments. It needs to be.

2) Not sure if .sql is such a great appendix to use, given that it's not
actual SQL. (I will also echo the other comment that was posted about
the whole name being sqllog, when it's not SQL. I think a better name is
needed since the data is csv. Goes for all parameters and function names)



Isn't the data included in the loglines in need of quoting/escaping?
What if there are weird charactersin the database name, for example? You
seem to only be escaping the error message itself. I think all
user-controlled data needs to be.


Then, it may be just me, but I find code like this:
!         sqllogFile = fopen(strcat(filename, ".sql"), "a");

very hard to read. Took me a while to realize that's how it dealt with
the different filenames. I think it's better to do the filename catting
as a separate step.


//Magnus