From: | Alex Hunsaker <badalex(at)gmail(dot)com> |
---|---|
To: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Stephen Frost <sfrost(at)snowman(dot)net> |
Subject: | csvlog_fields review |
Date: | 2011-07-09 21:11:52 |
Message-ID: | CAFaPBrR3yN7qVmvdwZvC9fUyugD77VJ+TpCnMTuAgdjWwMs-pg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
It bit rotted a bit find a new version attached that includes the
following fixes:
- show_session_authorization() no longer exists, instead access the
session_authorization_guc directly (like we do for show_role in
commands/variable.c). I find it quite ugly tho...
- it changed %u to %U and %U to be %u... flipped it back so %u remains unchanged
- num_fields in write_csvlog was unused, removed it
- new_csv_fields would leak in an error path of assign_csvlog_fields
(which probably matters as we are in TopMemoryContext)
All of Itagaki-san's comments still stand:
> * csvlog_fields and csvlog_header won't work with non-default log_filename
when it doesn't contain seconds in the format. They expect they can always
open empty log files.
I think at the very least we should document this? Or maybe only write
out the header when its a zero length file?
> * The long default value for csvlog_fields leads long text line in
postgresql.conf, SHOW ALL, pg_settings view, but there were no better
alternative solutions in the past discussion.
I think it might be worth revisiting using the %X syntax
log_line_prefix uses instead of inventing our own long form names.
> * csvlog_fields is marked as PGC_POSTMASTER. It can protect mixed formats
in a csv file on default log_filename, but other similar GUC variables
are usually marked AS PGC_SIGHUP.
I don't really see this as a problem...
As it stands I think we still need to address the first two questions
before its ready for a -commiter.
Attachment | Content-Type | Size |
---|---|---|
csvlog_fields_ah.patch.gz | application/x-gzip | 9.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Cédric Villemain | 2011-07-09 21:45:14 | Re: Enhanced psql in core? |
Previous Message | Darren Duncan | 2011-07-09 20:07:25 | Re: [HACKERS] Creating temp tables inside read only transactions |