Re: [PATCHES] Patch for - Allow server logs to be remotely read

2006-06-09 Thread Andreas Pflug

Alvaro Herrera wrote:

Bruce Momjian wrote:


Uh, I just added /contrib/adminpack a few weeks ago to CVS, which does
this, and more.  Sorry I forgot to mark the TODO item as completed.



Huh, how do you read files with adminpack?  


try
select * from pg_logdir_ls() as (filetime timestamp, filename text)
and read the file you need.

Regards,
Andreas



---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings


Re: [PATCHES] Patch for - Allow server logs to be remotely read

2006-06-09 Thread Andreas Pflug

Tom Lane wrote:

Bruce Momjian pgman@candle.pha.pa.us writes:


Tom Lane wrote:


I wonder if we should take pg_read_file (and the rest of genfile.c)
back out of the backend and stick them into contrib/adminpack.




I thought about that but what we have in the backend now is read-only
which basically could be done using COPY, so I don't see any security
value to moving them out.  They are super-user only just like COPY.



The you-can-do-it-with-COPY argument doesn't apply to pg_ls_dir, nor to
pg_stat_file, and I find it unconvincing even for pg_read_file.  COPY
isn't at all friendly for trying to read binary files, for instance.


pg_file_read returns text which isn't binary-friendly either.

Regards,
Andreas

---(end of broadcast)---
TIP 6: explain analyze is your friend


[PATCHES] Patch for - Allow server logs to be remotely read

2006-06-08 Thread Dhanaraj M

The patch is attached for the following TODO item:

*Allow server logs to be remotely read.


Steps:

1. When the server starts (**pg_ctl start)**, the path of the postmaster file is stored 
2. The user can access the logfile using the following 2 functions: 
  * pg_file_readlog(int linenumber) -  Retrieves the string for the given line number

  * **pg_file_countlog() -  Retrieves the number of lines existing in the 
logfile*



I have implemented this based on the suggestions given in the hackers mailing 
list.
If you know a better way, please share it with me. Waiting for your reply.


Thanks
Dhanaraj

*** ./contrib/adminpack/adminpack.c.orig	Thu Jun  8 17:04:42 2006
--- ./contrib/adminpack/adminpack.c	Thu Jun  8 18:34:37 2006
***
*** 24,30 
--- 24,32 
  #include catalog/pg_type.h
  #include funcapi.h
  #include utils/datetime.h
+ #include utils/builtins.h
  
+ #define GET_TEXT(cstrp) DatumGetTextP(DirectFunctionCall1(textin, CStringGetDatum(cstrp)))
  
  #ifdef WIN32
  
***
*** 48,58 
--- 50,64 
  Datum pg_file_rename(PG_FUNCTION_ARGS);
  Datum pg_file_unlink(PG_FUNCTION_ARGS);
  Datum pg_logdir_ls(PG_FUNCTION_ARGS);
+ Datum pg_file_readlog(PG_FUNCTION_ARGS);
+ Datum pg_file_countlog();
  
  PG_FUNCTION_INFO_V1(pg_file_write);
  PG_FUNCTION_INFO_V1(pg_file_rename);
  PG_FUNCTION_INFO_V1(pg_file_unlink);
  PG_FUNCTION_INFO_V1(pg_logdir_ls);
+ PG_FUNCTION_INFO_V1(pg_file_readlog);
+ PG_FUNCTION_INFO_V1(pg_file_countlog);
  
  typedef struct 
  {
***
*** 390,392 
--- 396,520 
  	FreeDir(fctx-dirdesc);
  	SRF_RETURN_DONE(funcctx);
  }
+ 
+ /* Retrieves n-th line from the postmaster logfile */
+ 
+ Datum pg_file_readlog(PG_FUNCTION_ARGS)
+ {
+ 	char c, buffer[MAXPGPATH];
+ 	char file_path2[MAXPGPATH], file_path1[MAXPGPATH];
+ 	int64 linenumber = PG_GETARG_INT64(0);
+ 	FILE *filePtr1, *filePtr2;
+ 	int64 count=1; 
+ int index=0;
+ 
+ 	if(OutputFileName[0])
+ 	{
+ 	   filePtr1 = fopen(OutputFileName, r);
+ 	}
+ 	else
+ 	{
+ 	   /* Finds the logfile path from postmasterlog_path file */
+ 	   snprintf(file_path1, MAXPGPATH, %s/postmasterlog_path, DataDir); 
+ 	   filePtr2 = fopen(file_path1, r);
+ 	   if(filePtr2 != NULL)
+ 	   {
+ 	 memset(file_path2, 0, MAXPGPATH);
+  fread(file_path2, 1, MAXPGPATH, filePtr2);
+ 	   }
+ 	   else
+ 	   {
+ 	 snprintf(buffer, MAXPGPATH, Could not find the logfile path in: %s, file_path1);
+ 	 ereport(ERROR, (errcode_for_file_access(), errmsg(buffer)));
+ 	   }
+ 
+ 	   fclose(filePtr2);	   
+ 	   filePtr1 = fopen(file_path2, r);
+ 	}
+ 
+  	memset(buffer, 0, MAXPGPATH);
+ 	
+ 	if(filePtr1 == NULL)
+ 	{
+ 	   snprintf(buffer, MAXPGPATH, Logfile: %s - Not found, file_path2);
+ 	   ereport(ERROR, (errcode_for_file_access(), errmsg(buffer)));
+ 	}
+ 
+ 	/* Reads the logfile and extract n-th line */
+ 	while( (c= fgetc(filePtr1))!=EOF)
+ 	{
+ 	   if (linenumber == count)
+ 	   {
+ 	  buffer[index] = c;
+ 	  index++;
+ 	   }
+ 
+ 	   /* Counts the number of lines */
+ 	   if(c=='\n')
+{
+   	  count++;
+ 	   }
+ 	 }
+ 
+ 	if(count = linenumber)
+ 	{
+ 	   sprintf(buffer, Given line number does not exist);
+ 	   ereport(ERROR, (errcode_for_file_access(), errmsg(buffer)));
+ 	}
+ 
+ fclose(filePtr1); 	
+ PG_RETURN_TEXT_P(GET_TEXT(buffer));
+ }
+ 
+ /* Counts the number of lines in the postmaster logfile */
+ 
+ Datum pg_file_countlog()
+ {
+ 	char c, buffer[MAXPGPATH];
+ 	char file_path2[MAXPGPATH], file_path1[MAXPGPATH];
+ 	FILE *filePtr1, *filePtr2;
+ 	int64 count=1; 
+ 
+ 	if(OutputFileName[0])
+ 	{
+ 	   filePtr1 = fopen(OutputFileName, r);
+ 	}
+ 	else
+ 	{
+ 	   /* Finds the logfile path from postmasterlog_path file */
+ 	   snprintf(file_path1, MAXPGPATH, %s/postmasterlog_path, DataDir); 
+ 	   filePtr2 = fopen(file_path1, r);
+ 	   if(filePtr2 != NULL)
+ 	   {
+ 	 memset(file_path2, 0, MAXPGPATH);
+  fread(file_path2, 1, MAXPGPATH, filePtr2);
+ 	   }
+ 	   else
+ 	   {
+ 	 snprintf(buffer, MAXPGPATH, Could not find the logfile path in: %s, file_path1);
+ 	 ereport(ERROR, (errcode_for_file_access(), errmsg(buffer)));
+ 	   }
+ 
+ 	   fclose(filePtr2);
+ 	   filePtr1 = fopen(file_path2, r);
+ 	}
+ 
+  	memset(buffer, 0, MAXPGPATH);
+ 	
+ 	if(filePtr1== NULL)
+ 	{
+ 	   snprintf(buffer, MAXPGPATH, Logfile: %s - Not found, file_path2);
+ 	   ereport(ERROR, (errcode_for_file_access(), errmsg(buffer)));
+ 	}
+ 
+ /* Counts the number of lines */
+ 	while( (c= fgetc(filePtr1))!=EOF)
+ 	{
+ 	   if(c=='\n')
+   	  count++;
+ 	}
+ 
+ fclose(filePtr1); 	
+ 	PG_RETURN_INT64(count-1);
+ }
+ 
*** ./contrib/adminpack/adminpack.sql.in.orig	Thu Jun  8 17:05:16 2006
--- ./contrib/adminpack/adminpack.sql.in	Thu Jun  8 17:06:11 2006
***
*** 24,30 
--- 24,37 
 AS 'MODULE_PATHNAME', 'pg_logdir_ls'
  	LANGUAGE C VOLATILE STRICT;
  
+ CREATE FUNCTION pg_catalog.pg_file_readlog(bigint) RETURNS text
+

Re: [PATCHES] Patch for - Allow server logs to be remotely read

2006-06-08 Thread Bruce Momjian

Uh, I just added /contrib/adminpack a few weeks ago to CVS, which does
this, and more.  Sorry I forgot to mark the TODO item as completed.

---

Dhanaraj M wrote:
 The patch is attached for the following TODO item:
 
 *Allow server logs to be remotely read.
 
 
 Steps:
 
 1. When the server starts (**pg_ctl start)**, the path of the postmaster file 
 is stored 
 2. The user can access the logfile using the following 2 functions: 
* pg_file_readlog(int linenumber) -  Retrieves the string for the given 
 line number
* **pg_file_countlog() -  Retrieves the number of lines existing in the 
 logfile*
 
 
 
 I have implemented this based on the suggestions given in the hackers mailing 
 list.
 If you know a better way, please share it with me. Waiting for your reply.
 
 
 Thanks
 Dhanaraj
 


 
 ---(end of broadcast)---
 TIP 5: don't forget to increase your free space map settings

-- 
  Bruce Momjian   http://candle.pha.pa.us
  EnterpriseDBhttp://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
   subscribe-nomail command to [EMAIL PROTECTED] so that your
   message can get through to the mailing list cleanly


Re: [PATCHES] Patch for - Allow server logs to be remotely read

2006-06-08 Thread Alvaro Herrera
Bruce Momjian wrote:
 
 Uh, I just added /contrib/adminpack a few weeks ago to CVS, which does
 this, and more.  Sorry I forgot to mark the TODO item as completed.

Huh, how do you read files with adminpack?  I only see

Datum pg_file_write(PG_FUNCTION_ARGS);
Datum pg_file_rename(PG_FUNCTION_ARGS);
Datum pg_file_unlink(PG_FUNCTION_ARGS);
Datum pg_logdir_ls(PG_FUNCTION_ARGS);

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org


Re: [PATCHES] Patch for - Allow server logs to be remotely read

2006-06-08 Thread Bruce Momjian
Alvaro Herrera wrote:
 Bruce Momjian wrote:
  
  Uh, I just added /contrib/adminpack a few weeks ago to CVS, which does
  this, and more.  Sorry I forgot to mark the TODO item as completed.
 
 Huh, how do you read files with adminpack?  I only see
 
 Datum pg_file_write(PG_FUNCTION_ARGS);
 Datum pg_file_rename(PG_FUNCTION_ARGS);
 Datum pg_file_unlink(PG_FUNCTION_ARGS);
 Datum pg_logdir_ls(PG_FUNCTION_ARGS);

Uh, README.adminpack shows:

int8 pg_catalog.pg_file_write(fname text, data text, append bool)
int8 pg_catalog.pg_file_read(fname text, data text, append bool)
bool pg_catalog.pg_file_rename(oldname text, newname text)
bool pg_catalog.pg_file_rename(oldname text, newname text, archivname 
text)
bool pg_catalog.pg_file_unlink(fname text)
bigint pg_catalog.pg_file_size(text)
int4 pg_catalog.pg_logfile_rotate()
setof record pg_catalog.pg_logdir_ls()

Is that wrong?  Yes.  Looking at the C file, I see what you mean.  Let
me update the README.adminpack file.  read_file is already in the
backend code, and was in 8.1.X too.

test= \df *read*
  List of functions
   Schema   | Name | Result data type | Argument data types
+--+--+--
 pg_catalog | loread   | bytea| integer, integer
 pg_catalog | pg_read_file | text | text, bigint, bigint
(2 rows)

The unlink part is part of the adminpack.

-- 
  Bruce Momjian   http://candle.pha.pa.us
  EnterpriseDBhttp://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match


Re: [PATCHES] Patch for - Allow server logs to be remotely read

2006-06-08 Thread Bruce Momjian
Bruce Momjian wrote:
 Alvaro Herrera wrote:
  Bruce Momjian wrote:
   
   Uh, I just added /contrib/adminpack a few weeks ago to CVS, which does
   this, and more.  Sorry I forgot to mark the TODO item as completed.
  
  Huh, how do you read files with adminpack?  I only see
  
  Datum pg_file_write(PG_FUNCTION_ARGS);
  Datum pg_file_rename(PG_FUNCTION_ARGS);
  Datum pg_file_unlink(PG_FUNCTION_ARGS);
  Datum pg_logdir_ls(PG_FUNCTION_ARGS);
 
 Uh, README.adminpack shows:
 
   int8 pg_catalog.pg_file_write(fname text, data text, append bool)
   int8 pg_catalog.pg_file_read(fname text, data text, append bool)
   bool pg_catalog.pg_file_rename(oldname text, newname text)
   bool pg_catalog.pg_file_rename(oldname text, newname text, archivname 
 text)
   bool pg_catalog.pg_file_unlink(fname text)
   bigint pg_catalog.pg_file_size(text)
   int4 pg_catalog.pg_logfile_rotate()
   setof record pg_catalog.pg_logdir_ls()
 
 Is that wrong?  Yes.  Looking at the C file, I see what you mean.  Let
 me update the README.adminpack file.  read_file is already in the
 backend code, and was in 8.1.X too.
   
   test= \df *read*
 List of functions
  Schema   | Name | Result data type | Argument data types
   +--+--+--
pg_catalog | loread   | bytea| integer, integer
pg_catalog | pg_read_file | text | text, bigint, bigint
   (2 rows)
 
 The unlink part is part of the adminpack.

I see it now.  They do some renaming of functions to match the use by
pgadmin, etc:

CREATE FUNCTION pg_catalog.pg_file_read(text, bigint, bigint)
RETURNS text
   AS 'pg_read_file'
LANGUAGE INTERNAL VOLATILE STRICT;

'pg_file_read' becomes 'pg_read_file.'  Anyway, the read part was
already in the backend, and the unlink part is new in adminpack.


-- 
  Bruce Momjian   http://candle.pha.pa.us
  EnterpriseDBhttp://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org


Re: [PATCHES] Patch for - Allow server logs to be remotely read

2006-06-08 Thread Tom Lane
Bruce Momjian pgman@candle.pha.pa.us writes:
 Is that wrong?  Yes.  Looking at the C file, I see what you mean.  Let
 me update the README.adminpack file.  read_file is already in the
 backend code, and was in 8.1.X too.

I wonder if we should take pg_read_file (and the rest of genfile.c)
back out of the backend and stick them into contrib/adminpack.  The
argument for having them in the backend was always pretty weak to me.
In particular, a DBA who doesn't want them in his system for security
reasons has no simple way to get rid of them if they're in core, but
not installing a contrib module is easy enough.

regards, tom lane

---(end of broadcast)---
TIP 6: explain analyze is your friend


Re: [PATCHES] Patch for - Allow server logs to be remotely read

2006-06-08 Thread Tom Lane
Bruce Momjian pgman@candle.pha.pa.us writes:
 Tom Lane wrote:
 I wonder if we should take pg_read_file (and the rest of genfile.c)
 back out of the backend and stick them into contrib/adminpack.

 I thought about that but what we have in the backend now is read-only
 which basically could be done using COPY, so I don't see any security
 value to moving them out.  They are super-user only just like COPY.

The you-can-do-it-with-COPY argument doesn't apply to pg_ls_dir, nor to
pg_stat_file, and I find it unconvincing even for pg_read_file.  COPY
isn't at all friendly for trying to read binary files, for instance.
Even for plain ASCII text you'd have to try to find a delimiter
character not present anywhere in the file, and backslashes in the file
would get corrupted.

But the basic point here is that someone who wants filesystem access
from the database is going to install adminpack anyway.  Why should
someone who *doesn't* want filesystem access from the database be
forced to have some capabilities of that type installed anyway?

regards, tom lane

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org


Re: [PATCHES] Patch for - Allow server logs to be remotely read

2006-06-08 Thread Bruce Momjian
Tom Lane wrote:
 Bruce Momjian pgman@candle.pha.pa.us writes:
  Tom Lane wrote:
  I wonder if we should take pg_read_file (and the rest of genfile.c)
  back out of the backend and stick them into contrib/adminpack.
 
  I thought about that but what we have in the backend now is read-only
  which basically could be done using COPY, so I don't see any security
  value to moving them out.  They are super-user only just like COPY.
 
 The you-can-do-it-with-COPY argument doesn't apply to pg_ls_dir, nor to
 pg_stat_file, and I find it unconvincing even for pg_read_file.  COPY
 isn't at all friendly for trying to read binary files, for instance.
 Even for plain ASCII text you'd have to try to find a delimiter
 character not present anywhere in the file, and backslashes in the file
 would get corrupted.
 
 But the basic point here is that someone who wants filesystem access
 from the database is going to install adminpack anyway.  Why should
 someone who *doesn't* want filesystem access from the database be
 forced to have some capabilities of that type installed anyway?

Remember we went around and around on this with the pgAdmin guys, so you
are going to have to get their input.  Also consider that pgAdmin might
be doing remote administration on a database it can't load shared
objects into, so having the read stuff always be there might help them.

I don't see anyone complaining about our read-only file access in the
backend, so I don't see a readon to remove it.

-- 
  Bruce Momjian   http://candle.pha.pa.us
  EnterpriseDBhttp://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings


Re: [PATCHES] Patch for - Allow server logs to be remotely read

2006-06-08 Thread Andrew Dunstan

Bruce Momjian wrote:

Tom Lane wrote:
  

Bruce Momjian pgman@candle.pha.pa.us writes:


Tom Lane wrote:
  

I wonder if we should take pg_read_file (and the rest of genfile.c)
back out of the backend and stick them into contrib/adminpack.


I thought about that but what we have in the backend now is read-only
which basically could be done using COPY, so I don't see any security
value to moving them out.  They are super-user only just like COPY.
  

The you-can-do-it-with-COPY argument doesn't apply to pg_ls_dir, nor to
pg_stat_file, and I find it unconvincing even for pg_read_file.  COPY
isn't at all friendly for trying to read binary files, for instance.
Even for plain ASCII text you'd have to try to find a delimiter
character not present anywhere in the file, and backslashes in the file
would get corrupted.

But the basic point here is that someone who wants filesystem access
from the database is going to install adminpack anyway.  Why should
someone who *doesn't* want filesystem access from the database be
forced to have some capabilities of that type installed anyway?



Remember we went around and around on this with the pgAdmin guys, so you
are going to have to get their input.  Also consider that pgAdmin might
be doing remote administration on a database it can't load shared
objects into, so having the read stuff always be there might help them.

I don't see anyone complaining about our read-only file access in the
backend, so I don't see a readon to remove it.

  


For the record, I am not happy with forcing this either. Providing it in 
an optional module seems perfectly reasonable to me. I suppose an 
alternative would be to turn the capability on or off via a (yet 
another) switch.



cheers

andrew

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
  subscribe-nomail command to [EMAIL PROTECTED] so that your
  message can get through to the mailing list cleanly


Re: [PATCHES] Patch for - Allow server logs to be remotely read

2006-06-08 Thread Dhanaraj M

Bruce Momjian wrote:


Uh, I just added /contrib/adminpack a few weeks ago to CVS, which does
this, and more.  Sorry I forgot to mark the TODO item as completed.

---

 


1.  int8 pg_catalog.pg_file_read(fname text, data text, append bool)

Though the above pg_file_read provides an interface to read the files,
the admin has to know the complete file path in order to read from them.
This can be simplified. As I have implemented. it does not take the file 
name as a parameter.
It automatically stored the postmaster file path and reads whenever 
required.


2. As suggested in the mailing list by Tom lane, this feature is 
implemented in contrib pkg.

  Hence, this feature is not forced on all.







---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster