Re: Add support for logging the current role

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Add support for logging the current role
Date: 2011-02-06 14:31:44
Message-ID: 20110206143144.GI4116@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

* Itagaki Takahiro (itagaki(dot)takahiro(at)gmail(dot)com) wrote:
> I think we need to improve postgresql.conf.sample a bit more, especially
> the long line for #log_csv_fields = '...'. 330 characters in it!
> #1. Leave the long line because it is needed.

It's needed to match what the current default is..

> #2. Hide the variable from the default conf.

I don't like that idea.

> #3. Use short %x mnemonic both in log_line_prefix and log_csv_fields.
> (It might require many additional mnemonics.)

It would require a lot more, and wouldn't scale well either (this was
discussed previously..).

> I think it depends default log filename, that contains %S (seconds)
> suffix. We can remove %S from log_filename; if we use a log per-day,
> those log might contain different columns even after restart. If we
> cannot avoid zigged csv fields completely, SIGHUP seems reasonable for it.

This problem is bigger than SIGHUP, but at least with a restart
required, the default will work properly. The default configuration
wouldn't work w/ a change to the log line and a SIGHUP done. If you
change the log filename then it could generate jagged CSV files even on
restart. We'd have to move the old log file out of the way when/if we
detect that it had a different CSV format and that's really just not
practical. We don't want to cause problems for people, but I don't
think there's a way to prevent jagged CSVs if they're going to go out of
thier way to configure PG to create them.

> How about changing the type of csv_log_fields from List* to fixed
> array of LogCSVFields? If so, we can use an array-initializer
> instead of build_default_csvlog_list() ? The code will be simplified.
> Fixed length won't be a problem because it would be rare that the
> same field are specified many times.

I really don't think changing it to an array is necessary or even
particularly helpful, and I don't agree that the code would actually
be simpler- you still have to make sure the CSV fields are kept in
the order requested by the user. Also, you'd have to remember to update
the length of the static array every time a new log option was added or
risk writing past the end of the array..

> Could you try to "make html" in the doc directory?
> Your new decumentation after
> | These columns may be included in the CSV output:
> will be unaligned plain text without some tags.

Alright, I can take a look at improving the documentation situation,
though I certainly wouldn't complain if someone who has it all working
already were to help..

Thanks,

Stephen

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Itagaki Takahiro 2011-02-06 15:13:53 Re: Add support for logging the current role
Previous Message Robert Haas 2011-02-06 14:10:15 Re: Re: patch: fix performance problems with repated decomprimation of varlena values in plpgsql