Re: [HACKERS] Add support for logging the current role

2011-03-11 Thread Bruce Momjian
Is this something for the next commit-fest? --- Stephen Frost wrote: -- Start of PGP signed section. * Tom Lane (t...@sss.pgh.pa.us) wrote: Robert Haas robertmh...@gmail.com writes: It seems there's at least one more

Re: [HACKERS] Add support for logging the current role

2011-03-11 Thread Stephen Frost
* Bruce Momjian (br...@momjian.us) wrote: Is this something for the next commit-fest? I already moved it there.. Thanks, Stephen signature.asc Description: Digital signature

Re: [HACKERS] Add support for logging the current role

2011-02-18 Thread Robert Haas
On Thu, Feb 17, 2011 at 10:40 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Thu, Feb 17, 2011 at 4:53 PM, Tom Lane t...@sss.pgh.pa.us wrote: In short, add a bit of overhead at SetUserId time in order to make this cheap (and accurate) in elog.c. As

Re: [HACKERS] Add support for logging the current role

2011-02-17 Thread Robert Haas
On Wed, Feb 16, 2011 at 9:52 PM, Stephen Frost sfr...@snowman.net wrote: * Robert Haas (robertmh...@gmail.com) wrote: Ah, so it does.  Sounds like you win.  Have we a patch implementing the sounds-like-its-agreed change, then? Patch attached, rebased to current master. Ugg, wait a minute.

Re: [HACKERS] Add support for logging the current role

2011-02-17 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: Ugg, wait a minute. This not only adds %U; it also changes the behavior of %u, which I don't think we've agreed on. Also, emitting 'none' when not SET ROLE has been done is pretty ugly. I'm back to thinking we need to push this out to 9.2 and take

Re: [HACKERS] Add support for logging the current role

2011-02-17 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote: Ugg, wait a minute. This not only adds %U; it also changes the behavior of %u, which I don't think we've agreed on. Also, emitting 'none' when not SET ROLE has been done is pretty ugly. I'm back to thinking we need to push this out to 9.2 and

Re: [HACKERS] Add support for logging the current role

2011-02-17 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote: Robert Haas robertmh...@gmail.com writes: Ugg, wait a minute. This not only adds %U; it also changes the behavior of %u, which I don't think we've agreed on. Also, emitting 'none' when not SET ROLE has been done is pretty ugly. I'm back to

Re: [HACKERS] Add support for logging the current role

2011-02-17 Thread Tom Lane
Stephen Frost sfr...@snowman.net writes: * Tom Lane (t...@sss.pgh.pa.us) wrote: Yeah, I thought what was supposed to be emitted was the value of current_user, not SQL's weird definition of what SET ROLE means. current_user uses GetUserNameFromId() and goes through the cache lookups to get

Re: [HACKERS] Add support for logging the current role

2011-02-17 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote: Well, that just doesn't seem useful to me in the real world. If I were using this, I would expect it to emit a real user name that matches the currently applied permissions checking. All the time. I wouldn't have ever thought to use %U w/o %u, to be

Re: [HACKERS] Add support for logging the current role

2011-02-17 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote: Ugg, wait a minute. This not only adds %U; it also changes the behavior of %u, which I don't think we've agreed on. Also, emitting 'none' when not SET ROLE has been done is pretty ugly. I'm back to thinking we need to push this out to 9.2 and

Re: [HACKERS] Add support for logging the current role

2011-02-17 Thread Robert Haas
On Thu, Feb 17, 2011 at 11:45 AM, Stephen Frost sfr...@snowman.net wrote: Robert, if you say this has to be punted to 9.2 again, I'm giving up. ;) Frankly, this patch has already consumed more than its fair share of my attention. Having said that, I've just spent some more time on it. I

Re: [HACKERS] Add support for logging the current role

2011-02-17 Thread Josh Berkus
Robert, It seems there's at least one more thing to worry about here, which is the overhead of this computation when CSV logging is in use. If no SET ROLE or SET SESSION AUTHORIZATION commands are in use, the code will call show_role(), which will return none. We'll then strcmp() that

Re: [HACKERS] Add support for logging the current role

2011-02-17 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: It seems there's at least one more thing to worry about here, which is the overhead of this computation when CSV logging is in use. If no SET ROLE or SET SESSION AUTHORIZATION commands are in use, the code will call show_role(), which will return

Re: [HACKERS] Add support for logging the current role

2011-02-17 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote: Robert Haas robertmh...@gmail.com writes: It seems there's at least one more thing to worry about here, which is the overhead of this computation when CSV logging is in use. If no SET ROLE or SET SESSION AUTHORIZATION commands are in use, the code

Re: [HACKERS] Add support for logging the current role

2011-02-17 Thread Robert Haas
On Thu, Feb 17, 2011 at 4:53 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: It seems there's at least one more thing to worry about here, which is the overhead of this computation when CSV logging is in use.  If no SET ROLE or SET SESSION AUTHORIZATION

Re: [HACKERS] Add support for logging the current role

2011-02-17 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Thu, Feb 17, 2011 at 4:53 PM, Tom Lane t...@sss.pgh.pa.us wrote: In short, add a bit of overhead at SetUserId time in order to make this cheap (and accurate) in elog.c. As Stephen says, I think this is utterly impractical; those routines can't

Re: [HACKERS] Add support for logging the current role

2011-02-16 Thread Robert Haas
On Tue, Feb 15, 2011 at 1:02 PM, Stephen Frost sfr...@snowman.net wrote: * Andrew Dunstan (and...@dunslane.net) wrote: On 02/15/2011 11:13 AM, Stephen Frost wrote: Think I suggested that at one point.  I'm all for doing that on a major version change like this one, but I think we already had

Re: [HACKERS] Add support for logging the current role

2011-02-16 Thread Andrew Dunstan
On 02/16/2011 04:24 PM, Robert Haas wrote: On Tue, Feb 15, 2011 at 1:02 PM, Stephen Frostsfr...@snowman.net wrote: * Andrew Dunstan (and...@dunslane.net) wrote: On 02/15/2011 11:13 AM, Stephen Frost wrote: Think I suggested that at one point. I'm all for doing that on a major version

Re: [HACKERS] Add support for logging the current role

2011-02-16 Thread Robert Haas
On Wed, Feb 16, 2011 at 5:55 PM, Andrew Dunstan and...@dunslane.net wrote: On 02/16/2011 04:24 PM, Robert Haas wrote: On Tue, Feb 15, 2011 at 1:02 PM, Stephen Frostsfr...@snowman.net  wrote: * Andrew Dunstan (and...@dunslane.net) wrote: On 02/15/2011 11:13 AM, Stephen Frost wrote: Think I

Re: [HACKERS] Add support for logging the current role

2011-02-16 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Tue, Feb 15, 2011 at 1:02 PM, Stephen Frostsfr...@snowman.net  wrote: I believe the suggestion that Robert and I were talking about above was to just unilatterally change the CSV log file output format to include current_role.  No header lines, no

Re: [HACKERS] Add support for logging the current role

2011-02-16 Thread Robert Haas
On Wed, Feb 16, 2011 at 7:42 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Tue, Feb 15, 2011 at 1:02 PM, Stephen Frostsfr...@snowman.net  wrote: I believe the suggestion that Robert and I were talking about above was to just unilatterally change the CSV

Re: [HACKERS] Add support for logging the current role

2011-02-16 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote: I can't remember at the moment: have we changed the CSV format in any releases since it was first created? And if so, did anyone complain? It was changed between 8.4 and 9.0 (application_name was added). I've looked around a bit in the archives w/ google

Re: [HACKERS] Add support for logging the current role

2011-02-16 Thread Andrew Dunstan
On 02/16/2011 08:38 PM, Stephen Frost wrote: * Tom Lane (t...@sss.pgh.pa.us) wrote: I can't remember at the moment: have we changed the CSV format in any releases since it was first created? And if so, did anyone complain? It was changed between 8.4 and 9.0 (application_name was added).

Re: [HACKERS] Add support for logging the current role

2011-02-16 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote: CSV log files were introduced in 8.3.0 by commit fd801f4faa8e0f00bc314b16549e3d8e8aa1b653. There are several follow-on commits making adjustments, but they all appear to be 8.3-vintage: 230e8962f3a47cae4729ad7c017410d28caf1370

Re: [HACKERS] Add support for logging the current role

2011-02-16 Thread Robert Haas
On Wed, Feb 16, 2011 at 8:58 PM, Stephen Frost sfr...@snowman.net wrote: * Robert Haas (robertmh...@gmail.com) wrote: CSV log files were introduced in 8.3.0 by commit fd801f4faa8e0f00bc314b16549e3d8e8aa1b653.  There are several follow-on commits making adjustments, but they all appear to be

Re: [HACKERS] Add support for logging the current role

2011-02-16 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote: On Wed, Feb 16, 2011 at 8:58 PM, Stephen Frost sfr...@snowman.net wrote: This list appears to miss out on 8217cfbd991856d25d73b0f7afcf43d99f90b653 ..? Ah, so it does. Sounds like you win. Have we a patch implementing the

Re: [HACKERS] Add support for logging the current role

2011-02-16 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote: Ah, so it does. Sounds like you win. Have we a patch implementing the sounds-like-its-agreed change, then? Patch attached, rebased to current master. Full git log: Thanks, Stephen commit

Re: [HACKERS] Add support for logging the current role

2011-02-15 Thread Robert Haas
On Tue, Feb 15, 2011 at 12:46 AM, Itagaki Takahiro itagaki.takah...@gmail.com wrote: On Mon, Feb 14, 2011 at 23:30, Stephen Frost sfr...@snowman.net wrote: * In assign_csvlog_fields(), we need to cleanup memory and memory context before return on error. Fixed this and a couple of similar

Re: [HACKERS] Add support for logging the current role

2011-02-15 Thread Robert Haas
On Fri, Feb 11, 2011 at 6:20 PM, Kevin Grittner kevin.gritt...@wicourts.gov wrote: I wrote: Patch attached. This time with src/backend/utils/misc/postgresql.conf.sample fixed. Committed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via

Re: [HACKERS] Add support for logging the current role

2011-02-15 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote: The payoff (getting %U) seems quite out of proportion to the potential downsides of making a change of this type at this late date. I'd be happy to go back to the original patch/idea of just the simple addition of %U as an option for

Re: [HACKERS] Add support for logging the current role

2011-02-15 Thread Stephen Frost
* Itagaki Takahiro (itagaki.takah...@gmail.com) wrote: On Mon, Feb 14, 2011 at 23:30, Stephen Frost sfr...@snowman.net wrote: * In assign_csvlog_fields(), we need to cleanup memory and memory context before return on error. Fixed this and a couple of similar issues. Not yet fixed.

Re: [HACKERS] Add support for logging the current role

2011-02-15 Thread Stephen Frost
* Stephen Frost (sfr...@snowman.net) wrote: Ugh, sorry about that, I should have realized that needed to be done. Updated patch attached. Errr, for real this time. Thanks, Stephen commit 25e94dcb390f56502bc46e683b438c20d2dc74e0 Author: Stephen Frost sfr...@snowman.net

Re: [HACKERS] Add support for logging the current role

2011-02-15 Thread Stephen Frost
* Stephen Frost (sfr...@snowman.net) wrote: I'd be happy to go back to the original patch/idea of just the simple addition of %U as an option for log_line_prefix. Updated patch attached which just adds %U support to log_line_prefix. Will work on adding CSV support for this in 9.2, along with

Re: [HACKERS] Add support for logging the current role

2011-02-15 Thread Robert Haas
On Tue, Feb 15, 2011 at 10:26 AM, Stephen Frost sfr...@snowman.net wrote: * Stephen Frost (sfr...@snowman.net) wrote: I'd be happy to go back to the original patch/idea of just the simple addition of %U as an option for log_line_prefix. Updated patch attached which just adds %U support to

Re: [HACKERS] Add support for logging the current role

2011-02-15 Thread Tom Lane
Stephen Frost sfr...@snowman.net writes: * Robert Haas (robertmh...@gmail.com) wrote: The payoff (getting %U) seems quite out of proportion to the potential downsides of making a change of this type at this late date. I'd be happy to go back to the original patch/idea of just the simple

Re: [HACKERS] Add support for logging the current role

2011-02-15 Thread Robert Haas
On Tue, Feb 15, 2011 at 10:37 AM, Tom Lane t...@sss.pgh.pa.us wrote: Stephen Frost sfr...@snowman.net writes: * Robert Haas (robertmh...@gmail.com) wrote: The payoff (getting %U) seems quite out of proportion to the potential downsides of making a change of this type at this late date. I'd

Re: [HACKERS] Add support for logging the current role

2011-02-15 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: Something along these lines would be OK with me (I haven't yet validated every detail), but there were previous objections to adding any new fields to log_line_prefix until we had a flexible CSV format. I think that's raising the bar a bit too high,

Re: [HACKERS] Add support for logging the current role

2011-02-15 Thread Robert Haas
On Tue, Feb 15, 2011 at 10:57 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Something along these lines would be OK with me (I haven't yet validated every detail), but there were previous objections to adding any new fields to log_line_prefix until we had a

Re: [HACKERS] Add support for logging the current role

2011-02-15 Thread Stephen Frost
Tom, * Tom Lane (t...@sss.pgh.pa.us) wrote: Given that this has been like this right along, I don't see why it's all that urgent to force a half-baked solution into 9.1. I'm also concerned that if we do do that, you'll lose motivation to work on cleaning it up for 9.2 ;-) The addition to

Re: [HACKERS] Add support for logging the current role

2011-02-15 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote: Well, I guess the other option is to just add it to the format, full stop. But as someone pointed out previously, that's not a terribly scalable solution, but perhaps it could be judged adequate for this particular case. Think I suggested that at

Re: [HACKERS] Add support for logging the current role

2011-02-15 Thread Robert Haas
On Tue, Feb 15, 2011 at 11:13 AM, Stephen Frost sfr...@snowman.net wrote: * Robert Haas (robertmh...@gmail.com) wrote: Well, I guess the other option is to just add it to the format, full stop.  But as someone pointed out previously, that's not a terribly scalable solution, but perhaps it

Re: [HACKERS] Add support for logging the current role

2011-02-15 Thread Andrew Dunstan
On 02/15/2011 11:13 AM, Stephen Frost wrote: * Robert Haas (robertmh...@gmail.com) wrote: Well, I guess the other option is to just add it to the format, full stop. But as someone pointed out previously, that's not a terribly scalable solution, but perhaps it could be judged adequate for

Re: [HACKERS] Add support for logging the current role

2011-02-15 Thread Stephen Frost
* Andrew Dunstan (and...@dunslane.net) wrote: On 02/15/2011 11:13 AM, Stephen Frost wrote: Think I suggested that at one point. I'm all for doing that on a major version change like this one, but I think we already had some concerns about that on this thread (Andrew maybe?). I could live

Re: [HACKERS] Add support for logging the current role

2011-02-14 Thread Itagaki Takahiro
On Mon, Feb 14, 2011 at 11:51, Stephen Frost sfr...@snowman.net wrote: It would be awfully nice if we could inject something into the csvlog output format that would let client programs know which fields are present.  This would be especially useful for people writing tools that are intended

Re: [HACKERS] Add support for logging the current role

2011-02-14 Thread Stephen Frost
Itagaki, * Itagaki Takahiro (itagaki.takah...@gmail.com) wrote: We need to design csvlog_header more carefully. csvlog_header won't work if log_filename is low-resolution, ex. log-per-day. This isn't any different a problem to the issue of someone changing the csvlog_fields GUC but not

Re: [HACKERS] Add support for logging the current role

2011-02-14 Thread Itagaki Takahiro
On Mon, Feb 14, 2011 at 23:30, Stephen Frost sfr...@snowman.net wrote: * In assign_csvlog_fields(), we need to cleanup memory and memory context before return on error. Fixed this and a couple of similar issues. Not yet fixed. Switched memory context is not restored on error. Updated patch

Re: [HACKERS] Add support for logging the current role

2011-02-13 Thread Robert Haas
On Fri, Feb 11, 2011 at 11:23 AM, Stephen Frost sfr...@snowman.net wrote: Updated patch attached, full git log below. The documentation for csvlog_fields should probably use literal around the default value. The sentence that begins For details on what these fields are should hyperlink the

Re: [HACKERS] Add support for logging the current role

2011-02-13 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote: The documentation for csvlog_fields should probably use literal around the default value. The sentence that begins For details on what these fields are should hyperlink the referenced sections of the documentation. The function prototype you

Re: [HACKERS] Add support for logging the current role

2011-02-13 Thread Andrew Dunstan
On 02/13/2011 08:26 AM, Stephen Frost wrote: This would be called a 'header' in most typical CSV scenarios. Unfortunately, last I checked (maybe it's changed?), COPY w/ HEADER just throws the header away instead of doing anything useful with it. If it actually used the header to build the

Re: [HACKERS] Add support for logging the current role

2011-02-13 Thread Stephen Frost
Robert, * Robert Haas (robertmh...@gmail.com) wrote: On Fri, Feb 11, 2011 at 11:23 AM, Stephen Frost sfr...@snowman.net wrote: Updated patch attached, full git log below. Thanks again for the review. Updated patch attached. The documentation for csvlog_fields should probably use literal

Re: [HACKERS] Add support for logging the current role

2011-02-13 Thread Stephen Frost
Andrew, * Andrew Dunstan (and...@dunslane.net) wrote: See the discussion back around the the 8.1 release (IIRC) when we added the HEADER option. I recall seeing it and not agreeing with it then. :) If I wanted something to throw away the first record of a file before loading it, I'd use

Re: [HACKERS] Add support for logging the current role

2011-02-11 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote: I don't think it's entirely stupid to worry about this completely screwing up the output of SHOW ALL on people using 80-character terminal windows. I haven't checked, but if it renders the output totally unreadable then I think we should try to

Re: [HACKERS] Add support for logging the current role

2011-02-11 Thread Kevin Grittner
Stephen Frost sfr...@snowman.net wrote: That, plus the length of max_predicate_locks_per_transaction, would make 'show all;' go beyond 80 characters even if we took out the description Should we abbreviate something there? max_pred_locks_per_tran, maybe? -Kevin -- Sent via

Re: [HACKERS] Add support for logging the current role

2011-02-11 Thread Robert Haas
On Fri, Feb 11, 2011 at 10:34 AM, Kevin Grittner kevin.gritt...@wicourts.gov wrote: Stephen Frost sfr...@snowman.net wrote: That, plus the length of max_predicate_locks_per_transaction, would make 'show all;' go beyond 80 characters even if we took out the description Should we abbreviate

Re: [HACKERS] Add support for logging the current role

2011-02-11 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote: On Fri, Feb 11, 2011 at 10:34 AM, Kevin Grittner Should we abbreviate something there?  max_pred_locks_per_tran, maybe? If we're going to abbreviate transaction, I'd vote for txn over tran, but I think Stephen's point that this is already a

Re: [HACKERS] Add support for logging the current role

2011-02-11 Thread Robert Haas
On Fri, Feb 11, 2011 at 10:52 AM, Stephen Frost sfr...@snowman.net wrote: * Robert Haas (robertmh...@gmail.com) wrote: On Fri, Feb 11, 2011 at 10:34 AM, Kevin Grittner Should we abbreviate something there?  max_pred_locks_per_tran, maybe? If we're going to abbreviate transaction, I'd vote

Re: [HACKERS] Add support for logging the current role

2011-02-11 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote: Yeah. The root cause of this problem is that the way psql handles tabular output with a few very wide rows stinks. True, but it would be kinda nice to support multi-line configuration variables. I still vote for that being not required to get this

Re: [HACKERS] Add support for logging the current role

2011-02-11 Thread Stephen Frost
* Itagaki Takahiro (itagaki.takah...@gmail.com) wrote: On Mon, Feb 7, 2011 at 04:10, Stephen Frost sfr...@snowman.net wrote: Yeah, doesn't seem to work for me (missing '/bin/collateindex.pl', apparently..). You might need yum install openjade stylesheets or similar packages and

Re: [HACKERS] Add support for logging the current role

2011-02-11 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Fri, Feb 11, 2011 at 10:34 AM, Kevin Grittner kevin.gritt...@wicourts.gov wrote: Should we abbreviate something there?  max_pred_locks_per_tran, maybe? If we're going to abbreviate transaction, I'd vote for txn over tran, but I think Stephen's

Re: [HACKERS] Add support for logging the current role

2011-02-11 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote: Robert Haas robertmh...@gmail.com writes: If we're going to abbreviate transaction, I'd vote for txn over tran, but I think Stephen's point that this is already a lost cause may have some validity. Not sure what other people think. I agree w/

Re: [HACKERS] Add support for logging the current role

2011-02-11 Thread Tom Lane
Stephen Frost sfr...@snowman.net writes: * Itagaki Takahiro (itagaki.takah...@gmail.com) wrote: You might need yum install openjade stylesheets or similar packages and re-configure. I've got openjade, etc, installed, but I'm on Debian and it doesn't appear to include that collateindex.pl

Re: [HACKERS] Add support for logging the current role

2011-02-11 Thread Alvaro Herrera
Excerpts from Tom Lane's message of vie feb 11 13:49:33 -0300 2011: Stephen Frost sfr...@snowman.net writes: * Itagaki Takahiro (itagaki.takah...@gmail.com) wrote: You might need yum install openjade stylesheets or similar packages and re-configure. I've got openjade, etc, installed,

Re: [HACKERS] Add support for logging the current role

2011-02-11 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote: $ which collateindex.pl /usr/bin/collateindex.pl $ rpm -qf /usr/bin/collateindex.pl docbook-style-dsssl-1.79-11.fc13.noarch Ah-hah, thanks for that! Apparently on Debian it's docbook-dsssl that contains it, and yes, it gets installed into /usr/bin (not

Re: [HACKERS] Add support for logging the current role

2011-02-11 Thread Kevin Grittner
Stephen Frost sfr...@snowman.net wrote: Tom Lane (t...@sss.pgh.pa.us) wrote: I'd be happy with max_pred_locks_per_transaction which gets the worst case down without being too obviously at variance with max_locks_per_transaction. Sounds good to me. The header length for show all would

Re: [HACKERS] Add support for logging the current role

2011-02-11 Thread Kevin Grittner
I wrote: Patch attached. This time with src/backend/utils/misc/postgresql.conf.sample fixed. -Kevin *** a/doc/src/sgml/config.sgml --- b/doc/src/sgml/config.sgml *** *** 5188,5202 dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir' /listitem

Re: [HACKERS] Add support for logging the current role

2011-02-10 Thread Itagaki Takahiro
On Mon, Feb 7, 2011 at 04:10, Stephen Frost sfr...@snowman.net wrote: Yeah, doesn't seem to work for me (missing '/bin/collateindex.pl', apparently..). You might need yum install openjade stylesheets or similar packages and re-configure. Ok, I've cleaned up that part of the documentation to

Re: [HACKERS] Add support for logging the current role

2011-02-10 Thread Noah Misch
On Thu, Feb 10, 2011 at 06:56:15PM +0900, Itagaki Takahiro wrote: On Mon, Feb 7, 2011 at 04:10, Stephen Frost sfr...@snowman.net wrote: I agree that it's logically good design, but we could not accept it as long as it breaks tools in the real world... If it does, I think it's pretty clear

Re: [HACKERS] Add support for logging the current role

2011-02-10 Thread Robert Haas
On Thu, Feb 10, 2011 at 6:27 AM, Noah Misch n...@leadboat.com wrote: FWIW, a 330 byte boot_val doesn't seem like a big deal to me.  If it were over _POSIX2_LINE_MAX (2048), that might be another matter. I don't think it's entirely stupid to worry about this completely screwing up the output of

Re: [HACKERS] Add support for logging the current role

2011-02-06 Thread Stephen Frost
* Itagaki Takahiro (itagaki.takah...@gmail.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..

Re: [HACKERS] Add support for logging the current role

2011-02-06 Thread Itagaki Takahiro
On Sun, Feb 6, 2011 at 23:31, Stephen Frost sfr...@snowman.net wrote: * Itagaki Takahiro (itagaki.takah...@gmail.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

Re: [HACKERS] Add support for logging the current role

2011-02-06 Thread Stephen Frost
* Itagaki Takahiro (itagaki.takah...@gmail.com) wrote: I agree that it's logically good design, but we could not accept it as long as it breaks tools in the real world... If it does, I think it's pretty clear that those tools are themselves broken.. Will it break SHOW ALL and SELECT * FROM

Re: [HACKERS] Add support for logging the current role

2011-02-06 Thread Stephen Frost
* Itagaki Takahiro (itagaki.takah...@gmail.com) wrote: Could you try to make html in the doc directory? Yeah, doesn't seem to work for me (missing '/bin/collateindex.pl', apparently..). Your new decumentation after | These columns may be included in the CSV output: will be unaligned plain

Re: [HACKERS] Add support for logging the current role

2011-02-01 Thread Itagaki Takahiro
Updated patch attached. 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. #2. Hide the variable from the default conf. #3. Use short %x mnemonic both in

Re: [HACKERS] Add support for logging the current role

2011-01-28 Thread Stephen Frost
Itagaki, * Itagaki Takahiro (itagaki.takah...@gmail.com) wrote: Discussions * How about csvlog_fields rather than log_csv_fields? Since we have variables with syslog_ prefix, csvlog_ prefix seems to be better. Sure, not a big deal, to be honest, as long as we can actually agree

Re: [HACKERS] Add support for logging the current role

2011-01-24 Thread Itagaki Takahiro
On Wed, Jan 19, 2011 at 14:36, Stephen Frost sfr...@snowman.net wrote: Alright, here's the latest on this patch.  I've added a log_csv_fields GUC along with the associated magic to make it work (at least for me). Also added 'role_name' and '%U' options.  Requires postmaster restart to change,

Re: [HACKERS] Add support for logging the current role

2011-01-18 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote: Stephen Frost sfr...@snowman.net writes: Point being, until we get Andrew's jagged-csv-import magic committed to core, we won't be able to import a log file that a user has changed the field list for mid-stream (following the add-a-new-column use-case

Re: [HACKERS] Add support for logging the current role

2011-01-17 Thread Alvaro Herrera
Excerpts from Tom Lane's message of sáb ene 15 00:34:40 -0300 2011: Stephen Frost sfr...@snowman.net writes: What about something other than version_x_y? I could maybe see having a 'default' and an 'all' instead.. Then have the default be what we have currently and 'all' be the full

Re: [HACKERS] Add support for logging the current role

2011-01-17 Thread Andrew Dunstan
On 01/17/2011 11:44 AM, Alvaro Herrera wrote: Excerpts from Tom Lane's message of sáb ene 15 00:34:40 -0300 2011: Stephen Frostsfr...@snowman.net writes: What about something other than version_x_y? I could maybe see having a 'default' and an 'all' instead.. Then have the default be what

Re: [HACKERS] Add support for logging the current role

2011-01-15 Thread Robert Haas
On Fri, Jan 14, 2011 at 9:24 PM, Andrew Dunstan and...@dunslane.net wrote: On 01/14/2011 08:41 PM, Robert Haas wrote: I think we're in the process of designing a manned mission to Mars to solve the problem that our shoelaces are untied. What's your suggestion, then? If there's a practical way

Re: [HACKERS] Add support for logging the current role

2011-01-15 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Fri, Jan 14, 2011 at 9:24 PM, Andrew Dunstan and...@dunslane.net wrote: What's your suggestion, then? If there's a practical way to add the requested escape, add it to the text format and leave reengineering the CSV format for another day. Yeah, I

Re: [HACKERS] Add support for logging the current role

2011-01-15 Thread Andrew Dunstan
On 01/15/2011 11:08 AM, Tom Lane wrote: Robert Haasrobertmh...@gmail.com writes: On Fri, Jan 14, 2011 at 9:24 PM, Andrew Dunstanand...@dunslane.net wrote: What's your suggestion, then? If there's a practical way to add the requested escape, add it to the text format and leave

Re: [HACKERS] Add support for logging the current role

2011-01-14 Thread Dimitri Fontaine
Tom Lane t...@sss.pgh.pa.us writes: Another little problem with the quick and dirty solution is that stuff that's important enough to warrant a log_line_prefix escape is generally thought to be important enough to warrant inclusion in CSV logs. That would imply adding a column and taking the

Re: [HACKERS] Add support for logging the current role

2011-01-14 Thread Andrew Dunstan
On 01/12/2011 08:59 PM, Tom Lane wrote: I'm not actually concerned about adding a few extra cycles to SET ROLE for this. What bothered me more was the cost of adding another output column to CSV log mode. That's not something you're going to be able to finesse such that only people who care

Re: [HACKERS] Add support for logging the current role

2011-01-14 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote: I seem to recall that the assign hook for role stores the string form of the role name anyway. Indeed it does, and it's already exposed through show_role() since it's needed in guc.c. Based on my review and understanding of the comments and calls, it

Re: [HACKERS] Add support for logging the current role

2011-01-14 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes: On 01/12/2011 08:59 PM, Tom Lane wrote: I'm not actually concerned about adding a few extra cycles to SET ROLE for this. What bothered me more was the cost of adding another output column to CSV log mode. That's not something you're going to be able

Re: [HACKERS] Add support for logging the current role

2011-01-14 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote: Andrew Dunstan and...@dunslane.net writes: I think it's time to revisit the design of CSV logs again, now we have two or three releases worth of experience with it. It needs some flexibility and refinement. It would definitely be nice to support

Re: [HACKERS] Add support for logging the current role

2011-01-14 Thread Andrew Dunstan
On 01/14/2011 11:48 AM, Stephen Frost wrote: * Tom Lane (t...@sss.pgh.pa.us) wrote: Andrew Dunstanand...@dunslane.net writes: I think it's time to revisit the design of CSV logs again, now we have two or three releases worth of experience with it. It needs some flexibility and refinement.

Re: [HACKERS] Add support for logging the current role

2011-01-14 Thread Aidan Van Dyk
On Fri, Jan 14, 2011 at 4:56 PM, Andrew Dunstan and...@dunslane.net wrote: I'm not sure I really want to make it that flexible :-) To deal with the issue Tom's referring to, I think it would be sufficient if we just allowed users to suppress production of certain columns (as long as we never

Re: [HACKERS] Add support for logging the current role

2011-01-14 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes: On 01/14/2011 11:48 AM, Stephen Frost wrote: My first thought would be to have a 'log_csv_format' GUC that's very similar to 'log_line_prefix' (and uses the same variables if possible..). We could then ship a default in postgresql.conf that matches

Re: [HACKERS] Add support for logging the current role

2011-01-14 Thread Andrew Dunstan
On 01/14/2011 05:04 PM, Aidan Van Dyk wrote: If there is going to be any change, how about using fixed columns (an possibly allowing them to be empty for stuff that's expensive to create/write), but adding a 1st column that contains a version identifyer. And to make it easy, maybe the PG

Re: [HACKERS] Add support for logging the current role

2011-01-14 Thread Tom Lane
Aidan Van Dyk ai...@highrise.ca writes: If there is going to be any change, how about using fixed columns (an possibly allowing them to be empty for stuff that's expensive to create/write), but adding a 1st column that contains a version identifyer. And to make it easy, maybe the PG major

Re: [HACKERS] Add support for logging the current role

2011-01-14 Thread Andrew Dunstan
On 01/14/2011 05:08 PM, Tom Lane wrote: Andrew Dunstanand...@dunslane.net writes: On 01/14/2011 11:48 AM, Stephen Frost wrote: My first thought would be to have a 'log_csv_format' GUC that's very similar to 'log_line_prefix' (and uses the same variables if possible..). We could then ship a

Re: [HACKERS] Add support for logging the current role

2011-01-14 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes: On 01/14/2011 05:08 PM, Tom Lane wrote: It actually sounded like a pretty good idea to me. If you have a format string, what do you want to do with the bits of the format that aren't field references? I was thinking of it as being strictly a field

Re: [HACKERS] Add support for logging the current role

2011-01-14 Thread Robert Haas
On Fri, Jan 14, 2011 at 8:00 PM, Tom Lane t...@sss.pgh.pa.us wrote: Andrew Dunstan and...@dunslane.net writes: On 01/14/2011 05:08 PM, Tom Lane wrote: It actually sounded like a pretty good idea to me. If you have a format string, what do you want to do with the bits of the format that

Re: [HACKERS] Add support for logging the current role

2011-01-14 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote: Andrew Dunstan and...@dunslane.net writes: If you have a format string, what do you want to do with the bits of the format that aren't field references? I was thinking of it as being strictly a field list. I don't know whether it's really practical

Re: [HACKERS] Add support for logging the current role

2011-01-14 Thread Tom Lane
Stephen Frost sfr...@snowman.net writes: * Tom Lane (t...@sss.pgh.pa.us) wrote: I was thinking of it as being strictly a field list. I don't know whether it's really practical to borrow log_line_prefix's one-character names for the fields (for one thing, there would need to be names for all

Re: [HACKERS] Add support for logging the current role

2011-01-14 Thread Andrew Dunstan
On 01/14/2011 08:41 PM, Robert Haas wrote: I think we're in the process of designing a manned mission to Mars to solve the problem that our shoelaces are untied. What's your suggestion, then? cheers andre -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make

Re: [HACKERS] Add support for logging the current role

2011-01-14 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Fri, Jan 14, 2011 at 8:00 PM, Tom Lane t...@sss.pgh.pa.us wrote: I was thinking of it as being strictly a field list. I think we're in the process of designing a manned mission to Mars to solve the problem that our shoelaces are untied. How so?

Re: [HACKERS] Add support for logging the current role

2011-01-14 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote: A user-settable column list seems pretty on-target for solving those problems to me. I'm looking into implementing this. An interesting initial question is- should the users be able to control the *order* of the columns? My gut feeling, if we're giving

Re: [HACKERS] Add support for logging the current role

2011-01-14 Thread Tom Lane
Stephen Frost sfr...@snowman.net writes: * Tom Lane (t...@sss.pgh.pa.us) wrote: A user-settable column list seems pretty on-target for solving those problems to me. I'm looking into implementing this. An interesting initial question is- should the users be able to control the *order* of

  1   2   >