Re: COPY-able csv log outputs - Mailing list pgsql-patches
| From | FAST PostgreSQL |
|---|---|
| Subject | Re: COPY-able csv log outputs |
| Date | |
| Msg-id | 15633.14641176859445.fast.fujitsu.com.au@MHS Whole thread Raw |
| In response to | Re: COPY-able sql log outputs (Russell Smith <mr-russ@pws.com.au>) |
| Responses |
Re: COPY-able csv log outputs
(Greg Smith <gsmith@gregsmith.com>)
|
| List | pgsql-patches |
[Modified subject line from sql to csv] Attached is an updated patch for COPY-able log outputs. It incorporates all the comments received on my previous patch including improved commenting, documentation, escaping all user controlled data Rgds, Arul Shaji On Wed, 4 Apr 2007 18:02, Russell Smith wrote: > FAST PostgreSQL wrote: > > On Wed, 4 Apr 2007 02:26, Andrew Dunstan wrote: > > > > I am currently doing the following modifications to the patch as per the > > review comments. > > > > 1. Changing all references to 'sqllog' or 'sql' to 'csvlog' and 'csv' > > respectively. > > 2. Escaping the username and databasename > > 3. Constructing the csvlog filename using log_filename instead of strcat. > > 4. General code optimization. > > > > Any other changes required ? ? > > Function additions like get_timestamp need more comments, or better > comments "returns the timestamp", which timestamp? > > > Will soon submit the updated patch. > > > > Rgds, > > Arul Shaji > > > >> 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. > >> > >> Brief review of the CSV aspect only: > >> > >> a. username and databasename should probably be quoted and escaped in > >> case they contain dangerous characters (even though that's unlikely) > >> b. calling pg_get_client_encoding() inside the loop in > >> escape_string_literal seems suboptimal. At the very least it should be > >> moved outside the loop so it's only called once per string. > >> > >> cheers > >> > >> andrew > > > > ---------------------------(end of broadcast)--------------------------- > > TIP 6: explain analyze is your friend > <!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"> > <html> > <head> > <meta content="text/html;charset=ISO-8859-1" http-equiv="Content-Type"> > </head> > <body bgcolor="#ffffff" text="#000000"> > FAST PostgreSQL wrote: > <blockquote cite="mid26121.11031175649322.fast.fujitsu.com.au@MHS" > type="cite"> > <pre wrap="">On Wed, 4 Apr 2007 02:26, Andrew Dunstan wrote: > > I am currently doing the following modifications to the patch as per the > review comments. > > 1. Changing all references to 'sqllog' or 'sql' to 'csvlog' and 'csv' > respectively. > 2. Escaping the username and databasename > 3. Constructing the csvlog filename using log_filename instead of strcat. > 4. General code optimization. > > Any other changes required ? ? > </pre> > </blockquote> > Function additions like get_timestamp need more comments, or better > comments "returns the timestamp", which timestamp?<br> > <br> > <br> > <blockquote cite="mid26121.11031175649322.fast.fujitsu.com.au@MHS" > type="cite"> > <pre wrap=""> > Will soon submit the updated patch. > > Rgds, > Arul Shaji > > > </pre> > <blockquote type="cite"> > <pre wrap="">FAST PostgreSQL wrote: > </pre> > <blockquote type="cite"> > <blockquote type="cite"> > <pre wrap="">Am Dienstag, 3. April 2007 20:33 schrieb FAST > PostgreSQL: </pre> > <blockquote type="cite"> > <pre wrap="">Attached is the completed patch for the COPY-able > sql log outputs. </pre> > </blockquote> > <pre wrap="">Could you please remove random whitespace changes from > this patch? </pre> > </blockquote> > <pre wrap="">Done and attached. > </pre> > </blockquote> > <pre wrap="">Brief review of the CSV aspect only: > > a. username and databasename should probably be quoted and escaped in > case they contain dangerous characters (even though that's unlikely) > b. calling pg_get_client_encoding() inside the loop in > escape_string_literal seems suboptimal. At the very least it should be > moved outside the loop so it's only called once per string. > > cheers > > andrew > </pre> > </blockquote> > <pre wrap=""><!----> > > ---------------------------(end of broadcast)--------------------------- > TIP 6: explain analyze is your friend > > </pre> > </blockquote> > <br> > </body> > </html>
Attachment
pgsql-patches by date: