On Sat, 31 Mar 2007 14:09, you wrote:
> 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..)
Sorry.. Fixed.
>
>
> 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.
Agreed. As I mentioned in the previous mail, preferably alongwith the
recommended table structure to COPY the logs into. But where? Is adding a new
paragraph under the log_destination section of 'Where to Log' chapter ok ?
>
> 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)
'csvlog' is another option. I can change that. Anyone have other preferences ?
>
>
>
> 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.
I will fix that too.
Rgds,
Arul Shaji
>
>
> //Magnus