Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-14 Thread Andreas Pflug
Tom Lane wrote: Andreas Pflug <[EMAIL PROTECTED]> writes: Tom Lane wrote: I removed the separate pg_file_length() function, as it doesn't have any significant notational advantage anymore; you can do Please note that there are pg_file_length functions in use for 8.0 on probably >95 % of w

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-14 Thread Tom Lane
Andreas Pflug <[EMAIL PROTECTED]> writes: > Tom Lane wrote: >> I removed the separate pg_file_length() function, as it doesn't have any >> significant notational advantage anymore; you can do > Please note that there are pg_file_length functions in use for 8.0 on > probably >95 % of win32 install

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-14 Thread Andrew Dunstan
Andreas Pflug wrote: Tom Lane wrote: I removed the separate pg_file_length() function, as it doesn't have any significant notational advantage anymore; you can do Please note that there are pg_file_length functions in use for 8.0 on probably >95 % of win32 installations, so you're break

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-14 Thread Andreas Pflug
Tom Lane wrote: I removed the separate pg_file_length() function, as it doesn't have any significant notational advantage anymore; you can do Please note that there are pg_file_length functions in use for 8.0 on probably >95 % of win32 installations, so you're breaking backwards compatibilit

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-13 Thread Bruce Momjian
Tom Lane wrote: > I wrote: > > I'll see about installing an initdb-time kluge to make it use OUT > > parameters. > > Done: > > regression=# SELECT * FROM pg_stat_file('postgresql.conf'); > length | atime | mtime | ctime > | isdir > +---

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-13 Thread Tom Lane
I wrote: > I'll see about installing an initdb-time kluge to make it use OUT > parameters. Done: regression=# SELECT * FROM pg_stat_file('postgresql.conf'); length | atime | mtime | ctime | isdir ++--

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-13 Thread Tom Lane
Bruce Momjian writes: > Tom Lane wrote: >> I suppose as long it's just this one function at stake, we could imagine >> fixing the pg_proc row after-the-fact (later in the initdb sequence). >> Pretty klugy but something nicer could get done in the 8.2 time frame. > Yes, see my earlier email --- we

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-13 Thread Bruce Momjian
Tom Lane wrote: > Bruce Momjian writes: > > SELECT pg_ls_dir > > FROM( > > SELECT pg_ls_dir(t1.setting) > > FROM(SELECT setting FROM pg_settings > > WHERE NAME = 'log_directory') AS t1 > > ) AS t2, > >

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-13 Thread Bruce Momjian
Andreas Pflug wrote: > Bruce Momjian wrote: > > > > Also, do we have a way to return columns from a system-installed > > function? I really don't like that pg_stat_file() to returns a record > > rather than named columns. How do I even access the individual record > > values? > > As in pg_setti

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-13 Thread Tom Lane
Bruce Momjian writes: > SELECT pg_ls_dir > FROM( > SELECT pg_ls_dir(t1.setting) > FROM(SELECT setting FROM pg_settings > WHERE NAME = 'log_directory') AS t1 > ) AS t2, > (SELECT setti

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-13 Thread Andreas Pflug
Bruce Momjian wrote: Also, do we have a way to return columns from a system-installed function? I really don't like that pg_stat_file() to returns a record rather than named columns. How do I even access the individual record values? As in pg_settings: SELECT length, mtime FROM pg_file_stat

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-13 Thread Andreas Pflug
Tom Lane wrote: Andreas Pflug <[EMAIL PROTECTED]> writes: Bruce Momjian wrote: Well, if they mix log files and non-log files in the same directory, we would have to filter based on the log_filename directive in the application, or use LIKE in a query. .. which is what pg_logdir_ls does. An

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-13 Thread Bruce Momjian
Tom Lane wrote: > Bruce Momjian writes: > > What I can imagine making things very easy is a readonly GUC that returns > > the current log file name. > > ... which unfortunately is not going to happen since the backends can't > see inside the syslogger process to know what it's doing. That's a sh

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-13 Thread Tom Lane
Bruce Momjian writes: > What I can imagine making things very easy is a readonly GUC that returns > the current log file name. ... which unfortunately is not going to happen since the backends can't see inside the syslogger process to know what it's doing. ATM I think the best you can do is look

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-13 Thread Tom Lane
Andreas Pflug <[EMAIL PROTECTED]> writes: > Bruce Momjian wrote: >> Well, if they mix log files and non-log files in the same directory, we >> would have to filter based on the log_filename directive in the >> application, or use LIKE in a query. > .. which is what pg_logdir_ls does. And it's robu

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-13 Thread Andreas Pflug
Bruce Momjian wrote: True, but that is more for the application. I don't imagine a user looking at that from psql would have a problem. However, you asked for a query that looks like pg_ls_logdir() and here it is: SELECT pg_ls_dir FROM( SEL

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-12 Thread Bruce Momjian
Andreas Pflug wrote: > Bruce Momjian wrote: > > > > > > > I don't see how listing the log files relates to editing the confuration > > files. > > Both are remote administration. While we've seen the discussion that one > aspect (config file editing) should be performed in psql, you assume the

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-12 Thread Andreas Pflug
Bruce Momjian wrote: I don't see how listing the log files relates to editing the confuration files. Both are remote administration. While we've seen the discussion that one aspect (config file editing) should be performed in psql, you assume the other aspect (viewing the logfile) to be no

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-12 Thread Bruce Momjian
Andreas Pflug wrote: > Bruce Momjian wrote: > > > > > > > I don't assume people using psql will care about the current log files --- > > Hm. Probably because you think these users will have direct file access? > Which in turn means they can edit *.conf directly too and don't need an > interfa

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-12 Thread Andreas Pflug
Bruce Momjian wrote: I don't assume people using psql will care about the current log files --- Hm. Probably because you think these users will have direct file access? Which in turn means they can edit *.conf directly too and don't need an interface for that either. it would be somethi

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-12 Thread Bruce Momjian
Andreas Pflug wrote: > Bruce Momjian wrote: > > >> BTW, it surprised me that one of the functions (don't remember > >> which one) expected the log files to be named in a very specific > >> fashion. So there's no flexibility for changing the log_prefix. > >> Probably it's not so bad, but strang

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-12 Thread Tom Lane
Andreas Pflug <[EMAIL PROTECTED]> writes: > So how would your query to display all all available _logfiles_ > look like? I think it's perfectly reasonable to assume that all the files in the log directory are logfiles --- more so than assuming that the admin hasn't exercised his option to change t

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-12 Thread Andreas Pflug
Tom Lane wrote: Andreas Pflug <[EMAIL PROTECTED]> writes: So how would your query to display all all available _logfiles_ look like? I think it's perfectly reasonable to assume that all the files in the log directory are logfiles --- more so than assuming that the admin hasn't exercised his

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-12 Thread Andreas Pflug
Bruce Momjian wrote: BTW, it surprised me that one of the functions (don't remember which one) expected the log files to be named in a very specific fashion. So there's no flexibility for changing the log_prefix. Probably it's not so bad, but strange anyway. Is this for "security" reasons?

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-12 Thread Tom Lane
I wrote: > I'm not sure whether to export skip_drive from path.c or just duplicate > it. If we do export it, a different name would probably be a good idea, > as it seems too generic for a global symbol. On reflection, there's another possibility, which is to put the test code into path.c in the

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-12 Thread Tom Lane
Bruce Momjian writes: > Here is an updated patch I have just applied (catalog version updated). Actually, you forgot the catversion bump. I read over this and fixed most of the problems I could see, but there is still one left: /* *Prevent reference to the parent directory. *

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-12 Thread Bruce Momjian
Alvaro Herrera wrote: > On Fri, Aug 12, 2005 at 11:28:22AM +, Andreas Pflug wrote: > > Bruce Momjian wrote: > > > > > > > >I did not include pg_logdir_ls because that was basically pg_ls_dir with > > >logic to decode the log file name and convert it to a timestamp. That > > >seemed best done

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-12 Thread Alvaro Herrera
On Fri, Aug 12, 2005 at 11:28:22AM +, Andreas Pflug wrote: > Bruce Momjian wrote: > > > > >I did not include pg_logdir_ls because that was basically pg_ls_dir with > >logic to decode the log file name and convert it to a timestamp. That > >seemed best done in the client. > > IMHO omitting pg

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-12 Thread Andreas Pflug
Bruce Momjian wrote: I did not include pg_logdir_ls because that was basically pg_ls_dir with logic to decode the log file name and convert it to a timestamp. That seemed best done in the client. IMHO omitting pg_logdir_ls is a bad idea, because the function is intended to hide server inter

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-12 Thread Dave Page
> -Original Message- > From: Bruce Momjian [mailto:[EMAIL PROTECTED] > Sent: 12 August 2005 04:27 > To: PostgreSQL-patches > Cc: Dave Page; Andreas Pflug > Subject: Re: [PATCHES] [HACKERS] For review: Server > instrumentation patch > > Here is an update

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-11 Thread Bruce Momjian
Here is an updated patch I have just applied (catalog version updated). I have added these functions: pg_stat_file() pg_read_file() pg_ls_dir() pg_reload_conf() pg_rotate_logfile() I did not include pg_logdir_ls because that was basically pg_ls_dir with log

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-11 Thread Bruce Momjian
Tom Lane wrote: > Alvaro Herrera <[EMAIL PROTECTED]> writes: > > Interesting. I wonder why the function is not defined instead with OUT > > parameters. > > Because bootstrap mode isn't capable of dealing with array columns, > so you can't define stuff in pg_proc.h that sets up an array of OUT > p

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-11 Thread Tom Lane
Alvaro Herrera <[EMAIL PROTECTED]> writes: > Interesting. I wonder why the function is not defined instead with OUT > parameters. Because bootstrap mode isn't capable of dealing with array columns, so you can't define stuff in pg_proc.h that sets up an array of OUT parameter types. I tried to ap

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-11 Thread Bruce Momjian
pgman wrote: > Andreas Pflug wrote: > > Bruce Momjian wrote: > > > Dave Page wrote: > > > > > > > > > The only part I didn't like about the patch is the stat display: > > > > > > test=> select pg_file_stat('postgresql.conf'); > > > pg_file_stat > > > > >

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-11 Thread Alvaro Herrera
On Thu, Aug 11, 2005 at 02:26:16PM -0400, Bruce Momjian wrote: > Alvaro Herrera wrote: > > Interesting. I wonder why the function is not defined instead with OUT > > parameters. That way you don't have to declare the record at execution > > time. See my patch to pgstattuple for an example. > >

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-11 Thread Bruce Momjian
Alvaro Herrera wrote: > On Thu, Aug 11, 2005 at 11:34:43AM -0400, Bruce Momjian wrote: > > Andreas Pflug wrote: > > > Bruce Momjian wrote: > > > > Dave Page wrote: > > > > > > > > > > > > The only part I didn't like about the patch is the stat display: > > > > > > > > test=> sele

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-11 Thread Bruce Momjian
Alvaro Herrera wrote: > On Thu, Aug 11, 2005 at 01:19:29PM -0400, Bruce Momjian wrote: > > Alvaro Herrera wrote: > > > > So, did you try > > > > > > select * from pg_file_stats('postgresql.conf'); > > > > Yes, I got: > > > > ERROR: a column definition list is required for functions returning

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-11 Thread Alvaro Herrera
On Thu, Aug 11, 2005 at 01:19:29PM -0400, Bruce Momjian wrote: > Alvaro Herrera wrote: > > So, did you try > > > > select * from pg_file_stats('postgresql.conf'); > > Yes, I got: > > ERROR: a column definition list is required for functions returning > "record" Interesting. I wonder why t

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-11 Thread Alvaro Herrera
On Thu, Aug 11, 2005 at 11:34:43AM -0400, Bruce Momjian wrote: > Andreas Pflug wrote: > > Bruce Momjian wrote: > > > Dave Page wrote: > > > > > > > > > The only part I didn't like about the patch is the stat display: > > > > > > test=> select pg_file_stat('postgresql.conf'); > > >

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-11 Thread Andreas Pflug
Bruce Momjian wrote: Dave Page wrote: The only part I didn't like about the patch is the stat display: test=> select pg_file_stat('postgresql.conf'); pg_file_stat --

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-11 Thread Bruce Momjian
Andreas Pflug wrote: > Bruce Momjian wrote: > > Dave Page wrote: > > > > > > The only part I didn't like about the patch is the stat display: > > > > test=> select pg_file_stat('postgresql.conf'); > > pg_file_stat > > > > -

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-10 Thread Bruce Momjian
Dave Page wrote: > > The list isn't complete. pgadmin uses these three functions > > for logfile > > tracking: > > > > - pg_logdir_ls to list logfiles > > - pg_file_length to check for changes of the current logfile > > - pg_file_read to retrieve a logfile > > Yes you're right, I didn't check t

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-01 Thread Dave Page
> -Original Message- > From: Andreas Pflug [mailto:[EMAIL PROTECTED] > Sent: 01 August 2005 15:05 > To: Dave Page > Cc: Bruce Momjian; PostgreSQL-patches > Subject: Re: [PATCHES] [HACKERS] For review: Server > instrumentation patch > > Dave Page wrote:

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-01 Thread Andreas Pflug
Dave Page wrote: pg_dir_ls isn't necessary for reading the logfiles; pg_logdir_ls will do this. Err, yes, sorry - that was a thinko. The list isn't complete. pgadmin uses these three functions for logfile tracking: - pg_logdir_ls to list logfiles - pg_file_length to check for changes

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-01 Thread Dave Page
> -Original Message- > From: Andreas Pflug [mailto:[EMAIL PROTECTED] > Sent: 01 August 2005 14:47 > To: Dave Page > Cc: Bruce Momjian; PostgreSQL-patches > Subject: Re: [PATCHES] [HACKERS] For review: Server > instrumentation patch

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-01 Thread Andreas Pflug
Dave Page wrote: -Original Message- From: Bruce Momjian [mailto:[EMAIL PROTECTED] Sent: 01 August 2005 03:26 To: Dave Page Cc: PostgreSQL-patches Subject: Re: [HACKERS] For review: Server instrumentation patch Dave Page wrote: [Resent as the list seems to have rejected yesterday

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-01 Thread Dave Page
> -Original Message- > From: Bruce Momjian [mailto:[EMAIL PROTECTED] > Sent: 01 August 2005 03:26 > To: Dave Page > Cc: PostgreSQL-patches > Subject: Re: [HACKERS] For review: Server instrumentation patch > > Dave Page wrote: > > > > [Resent as the list seems to have rejected yesterda

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-07-31 Thread Bruce Momjian
Dave Page wrote: > > [Resent as the list seems to have rejected yesterdays attempt] > > As per Bruce's request, here's a copy of Andreas' server > instrumentation patch for review. I've separated out the > dbsize stuff and pg_terminate_backend is also not included. > > This version was generat