Skip site navigation (1) Skip section navigation (2)

Re: COPY-able sql log outputs

From: Russell Smith <mr-russ(at)pws(dot)com(dot)au>
To: FAST PostgreSQL <fastpgs(at)fast(dot)fujitsu(dot)com(dot)au>
Cc: pgsql-patches(at)postgresql(dot)org
Subject: Re: COPY-able sql log outputs
Date: 2007-03-31 08:15:45
Message-ID: (view raw, whole thread or download thread mbox)
Lists: pgsql-patches
FAST PostgreSQL wrote:

I have attempted to do a quick review of the code in this patch, I've 
not installed or compiled it.  However I do have a couple of comments.
> Hi,
> Attached is the completed patch for the COPY-able sql log outputs. I have 
> modified my previous WIP patch, incorporating all the changes requested by 
> the community.  This patch has been tested both on windows and linux.
> Reiterating what has been done.
> The log is now output in COPY-able format as suggested. (Not INSERT-able as 
> was in the previous WIP patch.)
> log_destination now accepts 'sqllog' as a valid output destination. The log 
> output file will be determined by pg_log and log_filename variables. The sql 
> log output filename will be 'log_filename'.sql. The file rotation rules apply 
> to the sql log file output as well.
I'm not sure I'm sold on sqllog given it's not actually sql at all, it's 
just a CSV/text file.  But I don't have any better names at this time.
copylog, justifiedlog, csvlog  come to mind, but that all seem bad.
> The log output format is as follows.
> timestamp, username, database_name, sessionid, host_port, process_id, 
> command_tag, session_start_time, transaction_id, error_severity, 
> sql_state_code, statement
> The logs can be loaded into a table using the command
> COPY sqltable FROM 'filename.sql' WITH CSV;
I am concerned with the fact that no escaping is attempted on anything 
except the error message itself, is it known that it's not possible to 
have a ',' in the rest of the field types?
> There are only two minor issues I can think of
> 1. The sql log is currently output with newline and tab characters. It loads 
> into the table neatly. No problems. But when read back, atleast from psql in 
> windows, the tabs are replaced with some special characters.
> 2. I think it is better to document somewhere the table structure and the 
> COPY statement above. But where?
With regard to where to document the column structure, how about a 
header line when a new file is opened, that will allow users to use 
whatever table structure they like, and can import relevant columns from 
the log file.  You should also mention the format in the logging section 
under the sqllog parameter in the docs.
> [P.S. - In the wake of community's concerns regarding the legal disclaimer 
> getting attached to the end of mails we send to the community, we have got an 
> exemption from the disclaimer getting attached. As this is the first mail I 
> am sending after this approval, fingers crossed, it works. If for some reason 
> it gets attached, please ignore this mail and I will send the patch from some 
> other account.]
> Rgds,
> Arul Shaji
Other Patch comments;

Is there an overall line length you should be adhering too?  I think 
there is, so you will want to shorten those lines.

+         /* Win32 timezone names are too long so don't print them */
+ #ifndef WIN32
+              "%Y-%m-%d %H:%M:%S %Z",
+ #else
+              "%Y-%m-%d %H:%M:%S",
+ #endif

Do we actually care that the WIN32 format is long for this type of log?

+ /*
+  * Calculates and returns the timestamp
+  */
+ static void
+ get_timestamp(StringInfo buf)

"the timestamp" some words about the fact it gets the "current" 
timestamp might be helpful an that it appends to a string. 

!         sqllogFile = fopen(strcat(filename, ".sql"), "a");

I don't believe forcing .sql for a CSV file is a good idea, let the user 
decide what they want to call it, if you are forcing anything, at least 
make it ".csv".

+ #define LOG_DESTINATION_SQL    8

SQLLOG I would think for completness and consistency of naming.

Hope this helps get your patch into PostgreSQL cleanly and quickly.


Russell Smith

In response to

pgsql-patches by date

Next:From: Pavan DeolaseeDate: 2007-03-31 12:15:57
Subject: CIC and deadlocks
Previous:From: Tom LaneDate: 2007-03-31 05:48:17
Subject: Re: Fwd: Re: [pgsql-patches] pg_get_domaindef

Privacy Policy | About PostgreSQL
Copyright © 1996-2017 The PostgreSQL Global Development Group