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

Re: [HACKERS] For review: Server instrumentation patch

From
"Dave Page"
Date:

> -----Original Message-----
> From: Andreas Pflug [mailto:pgadmin@pse-consulting.de]
> 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:
>
> >>
> >>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 of the current logfile
> - pg_file_read to retrieve a logfile

Yes you're right, I didn't check thoroughly (in my defence, the coffee
machine broke this morning). Anyhoo, pg_file_stat is used by
pg_file_length, so that would be required as well.

None of those allow any modification of the filesystem, so do not suffer
the potential security issues that Tom was concerned about, so hopefully
there is no problem with them going in?

Regards, Dave.

Re: [HACKERS] For review: Server instrumentation patch

From
Bruce Momjian
Date:
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 thoroughly (in my defence, the coffee
> machine broke this morning). Anyhoo, pg_file_stat is used by
> pg_file_length, so that would be required as well.
>
> None of those allow any modification of the filesystem, so do not suffer
> the potential security issues that Tom was concerned about, so hopefully
> there is no problem with them going in?

OK, I have modified the patch to include these functions:

    pg_reload_conf()
    pg_file_stat()
    pg_file_read()
    pg_file_length()
    pg_dir_ls()
    pg_logfile_rotate()
    pg_logdir_ls()

These can only be run by the super-user, and can only access files
inside PGDATA, or in the logdir directory if that is in a different
place from PGDATA.

The only part I didn't like about the patch is the stat display:

    test=> select pg_file_stat('postgresql.conf');
                                    pg_file_stat
    -----------------------------------------------------------------------------

     (12287,"2005-08-11 00:06:30","2005-08-11 00:06:43","2005-08-11 00:06:30",f)
    (1 row)

Shouldn't this return multiple labeled columns rather than an array?

The patch is attached and genfile.c goes in utils/adt.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
Index: doc/src/sgml/func.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/func.sgml,v
retrieving revision 1.276
diff -c -c -r1.276 func.sgml
*** doc/src/sgml/func.sgml    2 Aug 2005 16:11:56 -0000    1.276
--- doc/src/sgml/func.sgml    11 Aug 2005 04:21:37 -0000
***************
*** 9061,9066 ****
--- 9061,9069 ----
     <indexterm zone="functions-admin">
      <primary>pg_cancel_backend</primary>
     </indexterm>
+    <indexterm zone="functions-admin">
+     <primary>pg_reload_conf</primary>
+    </indexterm>

     <indexterm zone="functions-admin">
      <primary>signal</primary>
***************
*** 9068,9074 ****
     </indexterm>

     <para>
!     The function shown in <xref
      linkend="functions-admin-signal-table"> sends control signals to
      other server processes.  Use of this function is restricted
      to superusers.
--- 9071,9077 ----
     </indexterm>

     <para>
!     The functions shown in <xref
      linkend="functions-admin-signal-table"> sends control signals to
      other server processes.  Use of this function is restricted
      to superusers.
***************
*** 9090,9110 ****
         <entry><type>int</type></entry>
         <entry>Cancel a backend's current query</entry>
        </row>
       </tbody>
      </tgroup>
     </table>

     <para>
!     This function returns 1 if successful, 0 if not successful.
      The process ID (<literal>pid</literal>) of an active backend can be found
      from the <structfield>procpid</structfield> column in the
      <structname>pg_stat_activity</structname> view, or by listing the <command>postgres</command>
      processes on the server with <application>ps</>.
     </para>
!
!    <indexterm zone="functions-admin">
!     <primary>pg_start_backup</primary>
!    </indexterm>

     <indexterm zone="functions-admin">
      <primary>pg_stop_backup</primary>
--- 9093,9121 ----
         <entry><type>int</type></entry>
         <entry>Cancel a backend's current query</entry>
        </row>
+       <row>
+        <entry>
+         <literal><function>pg_reload_conf</function>()</literal>
+         </entry>
+        <entry><type>int</type></entry>
+        <entry>Triggers the server processes to reload configuration files</entry>
+       </row>
       </tbody>
      </tgroup>
     </table>

     <para>
!     These functions return 1 if successful, 0 if not successful.
      The process ID (<literal>pid</literal>) of an active backend can be found
      from the <structfield>procpid</structfield> column in the
      <structname>pg_stat_activity</structname> view, or by listing the <command>postgres</command>
      processes on the server with <application>ps</>.
     </para>
!    <para>
!     <function>pg_reload_conf</> sends a SIGHUP event to the
!     postmaster, and thus triggers a reload of the configuration files
!     in all backend processes.
!    </para>

     <indexterm zone="functions-admin">
      <primary>pg_stop_backup</primary>
***************
*** 9309,9314 ****
--- 9320,9457 ----
      appropriate.
     </para>

+    <para>
+     The functions shown in <xref
+     linkend="functions-admin-genfile"> provide native file access to
+     files on the machine hosting the server. They are restricted to
+     the cluster directory or the logfile directory.
+     Use of these functions is restricted to superusers.
+    </para>
+
+    <table id="functions-admin-genfile">
+     <title>Generic File Access Functions</title>
+     <tgroup cols="3">
+      <thead>
+       <row><entry>Name</entry> <entry>Return Type</entry> <entry>Description</entry>
+       </row>
+      </thead>
+
+      <tbody>
+       <row>
+        <entry>
+         <literal><function>pg_file_stat</function>(<parameter>filename_text</parameter>)</literal>
+         </entry>
+        <entry><type>record</type></entry>
+        <entry>Retrieves file stats</entry>
+       </row>
+       <row>
+        <entry>
+         <literal><function>pg_file_length</function>(<parameter>filename_text</parameter>)</literal>
+         </entry>
+        <entry><type>int8</type></entry>
+        <entry>Returns the file length</entry>
+       </row>
+       <row>
+        <entry>
+         <literal><function>pg_file_read</function>(<parameter>filename_text</parameter>,
+         <parameter>offset_int8</parameter>,<parameter>length_int8</parameter>)</literal>
+         </entry>
+        <entry><type>text</type></entry>
+        <entry>Returns the contents of a text file</entry>
+       </row>
+       <row>
+        <entry>
+
<literal><function>pg_dir_ls</function>(<parameter>dirname_text</parameter>,<parameter>fullpath_bool</parameter>)</literal>
+         </entry>
+        <entry><type>setof text</type></entry>
+        <entry>Returns the file length</entry>
+       </row>
+      </tbody>
+     </tgroup>
+    </table>
+
+    <indexterm zone="functions-admin">
+     <primary>pg_file_stat</primary>
+    </indexterm>
+
+    <para>
+     <function>pg_file_stat()</> returns a record that contains the
+     length, creation timestamp, last accessed timestamp, last modified
+     timestamp and and a flag indicating a directory.
+    </para>
+
+    <indexterm zone="functions-admin">
+     <primary>pg_file_length</primary>
+    </indexterm>
+    <para>
+     <function>pg_file_length()</> returns the length of the given file.
+    </para>
+
+    <indexterm zone="functions-admin">
+     <primary>pg_file_read</primary>
+    </indexterm>
+    <para>
+     <function>pg_file_read()</> returns a part of a textfile, starting
+     at the offset giving length bytes.
+
+    <para>
+     <function>pg_dir_ls</> lists all filenames in the named directory.
+    </para>
+
+    <para>
+     The functions shown in <xref
+     linkend="functions-admin-logfile"> allow access to the server
+     logfile, if the stderr log output is redirected.
+     Use of these functions is restricted to superusers.
+    </para>
+
+    <table id="functions-admin-logfile">
+     <title>Backend Logfile Functions</title>
+     <tgroup cols="3">
+      <thead>
+       <row><entry>Name</entry> <entry>Return Type</entry> <entry>Description</entry>
+       </row>
+      </thead>
+
+      <tbody>
+       <row>
+        <entry>
+         <literal><function>pg_logfile_rotate</function>()</literal>
+         </entry>
+        <entry><type>int</type></entry>
+        <entry>Trigger logfile rotation</entry>
+       </row>
+       <row>
+        <entry>
+         <literal><function>pg_logdir_ls</function>()</literal>
+         </entry>
+        <entry><type>setof record</type></entry>
+        <entry>lists all logfiles in the pg_log subdirectory</entry>
+       </row>
+      </tbody>
+     </tgroup>
+    </table>
+
+    <indexterm zone="functions-admin">
+     <primary>pg_logfile_rotate</primary>
+    </indexterm>
+    <para>
+     <function>pg_logfile_rotate</> issues a logfile rotation trigger,
+     which forces the server to close the current logfile and open a
+     fresh one.
+    </para>
+
+    <indexterm zone="functions-admin">
+     <primary>pg_logdir_ls</primary>
+    </indexterm>
+    <para>
+     <function>pg_logdir_ls</> lists all files in the pg_log
+     subdirectory. For each log file, a record consisting of a
+     timestamp of its creation time and a complete path to the file is
+     returned. For convenience, the view pg_logdir_ls wraps the
+     function.
+    </para>
+
    </sect1>
  </chapter>

Index: src/backend/catalog/system_views.sql
===================================================================
RCS file: /cvsroot/pgsql/src/backend/catalog/system_views.sql,v
retrieving revision 1.18
diff -c -c -r1.18 system_views.sql
*** src/backend/catalog/system_views.sql    31 Jul 2005 17:19:17 -0000    1.18
--- src/backend/catalog/system_views.sql    11 Aug 2005 04:21:37 -0000
***************
*** 331,333 ****
--- 331,338 ----
                      pg_stat_get_db_blocks_hit(D.oid) AS blks_read,
              pg_stat_get_db_blocks_hit(D.oid) AS blks_hit
      FROM pg_database D;
+
+ CREATE VIEW pg_logdir_ls AS
+     SELECT *
+     FROM pg_logdir_ls() AS A
+     (filetime timestamp, filename text);
Index: src/backend/postmaster/postmaster.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v
retrieving revision 1.461
diff -c -c -r1.461 postmaster.c
*** src/backend/postmaster/postmaster.c    29 Jul 2005 19:30:04 -0000    1.461
--- src/backend/postmaster/postmaster.c    11 Aug 2005 04:21:44 -0000
***************
*** 3393,3398 ****
--- 3393,3403 ----
          }
      }

+     if (CheckPostmasterSignal(PMSIGNAL_ROTATE_LOGFILE) && SysLoggerPID != 0)
+     {
+         kill(SysLoggerPID, SIGUSR1);
+     }
+
      PG_SETMASK(&UnBlockSig);

      errno = save_errno;
Index: src/backend/postmaster/syslogger.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/postmaster/syslogger.c,v
retrieving revision 1.18
diff -c -c -r1.18 syslogger.c
*** src/backend/postmaster/syslogger.c    21 Jul 2005 18:06:12 -0000    1.18
--- src/backend/postmaster/syslogger.c    11 Aug 2005 04:21:45 -0000
***************
*** 101,106 ****
--- 101,107 ----
   * Flags set by interrupt handlers for later service in the main loop.
   */
  static volatile sig_atomic_t got_SIGHUP = false;
+ static volatile sig_atomic_t rotation_requested = false;


  /* Local subroutines */
***************
*** 117,122 ****
--- 118,124 ----
  static char *logfile_getname(pg_time_t timestamp);
  static void set_next_rotation_time(void);
  static void sigHupHandler(SIGNAL_ARGS);
+ static void sigUsr1Handler(SIGNAL_ARGS);


  /*
***************
*** 200,206 ****
      pqsignal(SIGQUIT, SIG_IGN);
      pqsignal(SIGALRM, SIG_IGN);
      pqsignal(SIGPIPE, SIG_IGN);
!     pqsignal(SIGUSR1, SIG_IGN);
      pqsignal(SIGUSR2, SIG_IGN);

      /*
--- 202,208 ----
      pqsignal(SIGQUIT, SIG_IGN);
      pqsignal(SIGALRM, SIG_IGN);
      pqsignal(SIGPIPE, SIG_IGN);
!     pqsignal(SIGUSR1, sigUsr1Handler);  /* request log rotation */
      pqsignal(SIGUSR2, SIG_IGN);

      /*
***************
*** 235,241 ****
      /* main worker loop */
      for (;;)
      {
-         bool        rotation_requested = false;
          bool        time_based_rotation = false;

  #ifndef WIN32
--- 237,242 ----
***************
*** 726,731 ****
--- 727,734 ----
      char       *filename;
      FILE       *fh;

+     rotation_requested = false;
+
      /*
       * When doing a time-based rotation, invent the new logfile name based
       * on the planned rotation time, not current time, to avoid "slippage"
***************
*** 876,878 ****
--- 879,888 ----
  {
      got_SIGHUP = true;
  }
+
+ /* SIGUSR1: set flag to rotate logfile */
+ static void
+ sigUsr1Handler(SIGNAL_ARGS)
+ {
+     rotation_requested = true;
+ }
Index: src/backend/utils/adt/Makefile
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/Makefile,v
retrieving revision 1.58
diff -c -c -r1.58 Makefile
*** src/backend/utils/adt/Makefile    29 Jul 2005 14:46:57 -0000    1.58
--- src/backend/utils/adt/Makefile    11 Aug 2005 04:21:45 -0000
***************
*** 24,30 ****
      tid.o timestamp.o varbit.o varchar.o varlena.o version.o xid.o \
      network.o mac.o inet_net_ntop.o inet_net_pton.o \
      ri_triggers.o pg_lzcompress.o pg_locale.o formatting.o \
!     ascii.o quote.o pgstatfuncs.o encode.o dbsize.o

  like.o: like.c like_match.c

--- 24,30 ----
      tid.o timestamp.o varbit.o varchar.o varlena.o version.o xid.o \
      network.o mac.o inet_net_ntop.o inet_net_pton.o \
      ri_triggers.o pg_lzcompress.o pg_locale.o formatting.o \
!     ascii.o quote.o pgstatfuncs.o encode.o dbsize.o genfile.o

  like.o: like.c like_match.c

Index: src/backend/utils/adt/misc.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/misc.c,v
retrieving revision 1.45
diff -c -c -r1.45 misc.c
*** src/backend/utils/adt/misc.c    4 Jul 2005 04:51:50 -0000    1.45
--- src/backend/utils/adt/misc.c    11 Aug 2005 04:21:45 -0000
***************
*** 17,34 ****
--- 17,46 ----
  #include <sys/file.h>
  #include <signal.h>
  #include <dirent.h>
+ #include <time.h>
+ #include <sys/time.h>

  #include "commands/dbcommands.h"
  #include "miscadmin.h"
  #include "storage/procarray.h"
+ #include "storage/pmsignal.h"
  #include "storage/fd.h"
  #include "utils/builtins.h"
+ #include "utils/elog.h"
+ #include "utils/datetime.h"
  #include "funcapi.h"
  #include "catalog/pg_type.h"
  #include "catalog/pg_tablespace.h"
+ #include "postmaster/syslogger.h"

  #define atooid(x)  ((Oid) strtoul((x), NULL, 10))

+ typedef struct
+ {
+     char *location;
+     DIR *dirdesc;
+ } directory_fctx;
+

  /*
   * Check if data is Null
***************
*** 107,112 ****
--- 119,273 ----
      PG_RETURN_INT32(pg_signal_backend(PG_GETARG_INT32(0), SIGINT));
  }

+
+ Datum
+ pg_reload_conf(PG_FUNCTION_ARGS)
+ {
+     if (!superuser())
+         ereport(ERROR,
+                 (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+                  (errmsg("only superuser can signal the postmaster"))));
+
+     if (kill(PostmasterPid, SIGHUP))
+     {
+         ereport(WARNING,
+                 (errmsg("failed to send signal to postmaster: %m")));
+
+         PG_RETURN_INT32(0);
+     }
+
+     PG_RETURN_INT32(1);
+ }
+
+
+ /*
+  * Rotate log file
+  */
+ Datum
+ pg_logfile_rotate(PG_FUNCTION_ARGS)
+ {
+     if (!superuser())
+         ereport(ERROR,
+                 (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+                  (errmsg("only superuser can rotate log files"))));
+
+     if (!Redirect_stderr)
+     {
+         ereport(NOTICE,
+                 (errcode(ERRCODE_WARNING),
+                  errmsg("no logfile configured; rotation not supported")));
+         PG_RETURN_INT32(0);
+     }
+
+     SendPostmasterSignal(PMSIGNAL_ROTATE_LOGFILE);
+
+     PG_RETURN_INT32(0);
+ }
+
+ /*
+  * scan log directory for log files
+  */
+ Datum pg_logdir_ls(PG_FUNCTION_ARGS)
+ {
+     FuncCallContext *funcctx;
+     struct dirent *de;
+     directory_fctx *fctx;
+
+     if (!superuser())
+         ereport(ERROR,
+                 (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+                  (errmsg("only superuser can list the log directory"))));
+
+     if (memcmp(Log_filename, "postgresql-%Y-%m-%d_%H%M%S.log", 30) != 0)
+         ereport(ERROR,
+                 (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                  (errmsg("the log_filename parameter must equal 'postgresql-%%Y-%%m-%%d_%%H%%M%%S.log'"))));
+
+     if (SRF_IS_FIRSTCALL())
+     {
+         MemoryContext oldcontext;
+         TupleDesc tupdesc;
+
+         funcctx = SRF_FIRSTCALL_INIT();
+         oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
+
+         fctx = palloc(sizeof(directory_fctx));
+         if (is_absolute_path(Log_directory))
+             fctx->location = Log_directory;
+         else
+         {
+             fctx->location = palloc(strlen(DataDir) + strlen(Log_directory) + 2);
+             sprintf(fctx->location, "%s/%s", DataDir, Log_directory);
+         }
+         tupdesc = CreateTemplateTupleDesc(2, false);
+         TupleDescInitEntry(tupdesc, (AttrNumber) 1, "starttime",
+                            TIMESTAMPOID, -1, 0);
+         TupleDescInitEntry(tupdesc, (AttrNumber) 2, "filename",
+                            TEXTOID, -1, 0);
+
+         funcctx->attinmeta = TupleDescGetAttInMetadata(tupdesc);
+
+         fctx->dirdesc = AllocateDir(fctx->location);
+
+         if (!fctx->dirdesc)
+             ereport(ERROR,
+                     (errcode_for_file_access(),
+                      errmsg("%s is not browsable: %m", fctx->location)));
+
+         funcctx->user_fctx = fctx;
+         MemoryContextSwitchTo(oldcontext);
+     }
+
+     funcctx = SRF_PERCALL_SETUP();
+     fctx = (directory_fctx*) funcctx->user_fctx;
+
+     if (!fctx->dirdesc)  /* not a readable directory  */
+         SRF_RETURN_DONE(funcctx);
+
+     while ((de = ReadDir(fctx->dirdesc, fctx->location)) != NULL)
+     {
+         char    *values[2];
+         HeapTuple tuple;
+         char     *field[MAXDATEFIELDS];
+         char    lowstr[MAXDATELEN + 1];
+         int        dtype;
+         int        nf, ftype[MAXDATEFIELDS];
+         fsec_t    fsec;
+         int        tz = 0;
+         struct    pg_tm date;
+
+         /*
+          * Default format:
+          *        postgresql-YYYY-MM-DD_HHMMSS.log
+          */
+         if (strlen(de->d_name) != 32 || memcmp(de->d_name, "postgresql-", 11) ||
+             de->d_name[21] != '_' || strcmp(de->d_name + 28, ".log"))
+               continue;
+
+         de->d_name[17] = '\0';
+         values[0] = de->d_name + 11;       /* timestamp */
+
+         values[1] = palloc(strlen(fctx->location) + strlen(de->d_name) + 2);
+         sprintf(values[1], "%s/%s", fctx->location, de->d_name);
+
+         /* parse and decode expected timestamp */
+         if (ParseDateTime(values[0], lowstr, sizeof(lowstr), field, ftype, MAXDATEFIELDS, &nf))
+             continue;
+
+         if (DecodeDateTime(field, ftype, nf, &dtype, &date, &fsec, &tz))
+             continue;
+
+         /* Seems the format fits the expected format; feed it into the tuple */
+
+         tuple = BuildTupleFromCStrings(funcctx->attinmeta, values);
+
+         SRF_RETURN_NEXT(funcctx, HeapTupleGetDatum(tuple));
+     }
+
+     FreeDir(fctx->dirdesc);
+     SRF_RETURN_DONE(funcctx);
+ }
+
  #ifdef NOT_USED

  /* Disabled in 8.0 due to reliability concerns; FIXME someday */
Index: src/include/catalog/pg_proc.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/catalog/pg_proc.h,v
retrieving revision 1.380
diff -c -c -r1.380 pg_proc.h
*** src/include/catalog/pg_proc.h    2 Aug 2005 16:11:57 -0000    1.380
--- src/include/catalog/pg_proc.h    11 Aug 2005 04:21:54 -0000
***************
*** 3049,3054 ****
--- 3049,3073 ----
  DATA(insert OID = 2173 ( pg_stop_backup            PGNSP PGUID 12 f f t f v 0 25 "" _null_ _null_ _null_
pg_stop_backup- _null_ )); 
  DESCR("Finish taking an online backup");

+ DATA(insert OID = 2621 ( pg_reload_conf         PGNSP PGUID 12 f f t f v 0 23 "" _null_ _null_ _null_ pg_reload_conf
-_null_ )); 
+ DESCR("Reload configuration files");
+
+ DATA(insert OID = 2622 ( pg_logfile_rotate        PGNSP PGUID 12 f f t f v 0 23 "" _null_ _null_ _null_
pg_logfile_rotate- _null_ )); 
+ DESCR("rotate log file");
+ DATA(insert OID = 2623 ( pg_logdir_ls            PGNSP PGUID 12 f f t t v 0 2249 "" _null_ _null_ _null_ pg_logdir_ls
-_null_ )); 
+ DESCR("list all available log files");
+
+
+ DATA(insert OID = 2624 ( pg_file_stat        PGNSP PGUID 12 f f t f v 1 2249 "25" _null_ _null_ _null_ pg_file_stat -
_null_)); 
+ DESCR("retrieve file stats");
+ DATA(insert OID = 2625 ( pg_file_length        PGNSP PGUID 14 f f t f v 1 20 "25" _null_ _null_ _null_ "SELECT len
FROMpg_file_stat($1) AS s(len int8, c timestamp, a timestamp, m timestamp, i bool)" - _null_ )); 
+ DESCR("returns length of a file");
+ DATA(insert OID = 2626 ( pg_file_read        PGNSP PGUID 12 f f t f v 3 25 "25 20 20" _null_ _null_ _null_
pg_file_read- _null_ )); 
+ DESCR("reads text from a file");
+ DATA(insert OID = 2627 ( pg_dir_ls            PGNSP PGUID 12 f f t t v 2 25 "25 16" _null_ _null_ _null_ pg_dir_ls -
_null_)); 
+ DESCR("list all file in a directory");
+
+
  /* Aggregates (moved here from pg_aggregate for 7.3) */

  DATA(insert OID = 2100 (  avg                PGNSP PGUID 12 t f f f i 1 1700 "20" _null_ _null_ _null_
aggregate_dummy- _null_ )); 
Index: src/include/storage/pmsignal.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/storage/pmsignal.h,v
retrieving revision 1.12
diff -c -c -r1.12 pmsignal.h
*** src/include/storage/pmsignal.h    28 Jun 2005 19:51:25 -0000    1.12
--- src/include/storage/pmsignal.h    11 Aug 2005 04:21:54 -0000
***************
*** 25,30 ****
--- 25,31 ----
      PMSIGNAL_PASSWORD_CHANGE,    /* pg_auth file has changed */
      PMSIGNAL_WAKEN_CHILDREN,    /* send a SIGUSR1 signal to all backends */
      PMSIGNAL_WAKEN_ARCHIVER,    /* send a NOTIFY signal to xlog archiver */
+     PMSIGNAL_ROTATE_LOGFILE,    /* send SIGUSR1 to syslogger to rotate logfile */

      NUM_PMSIGNALS                /* Must be last value of enum! */
  } PMSignalReason;
Index: src/include/utils/builtins.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/utils/builtins.h,v
retrieving revision 1.262
diff -c -c -r1.262 builtins.h
*** src/include/utils/builtins.h    29 Jul 2005 14:47:04 -0000    1.262
--- src/include/utils/builtins.h    11 Aug 2005 04:21:54 -0000
***************
*** 374,385 ****
--- 374,393 ----
  extern Datum pg_complete_relation_size_name(PG_FUNCTION_ARGS);
  extern Datum pg_size_pretty(PG_FUNCTION_ARGS);

+ /* genfile.c */
+ extern Datum pg_file_stat(PG_FUNCTION_ARGS);
+ extern Datum pg_file_read(PG_FUNCTION_ARGS);
+ extern Datum pg_dir_ls(PG_FUNCTION_ARGS);
+
  /* misc.c */
  extern Datum nullvalue(PG_FUNCTION_ARGS);
  extern Datum nonnullvalue(PG_FUNCTION_ARGS);
  extern Datum current_database(PG_FUNCTION_ARGS);
  extern Datum pg_cancel_backend(PG_FUNCTION_ARGS);
+ extern Datum pg_reload_conf(PG_FUNCTION_ARGS);
  extern Datum pg_tablespace_databases(PG_FUNCTION_ARGS);
+ extern Datum pg_logfile_rotate(PG_FUNCTION_ARGS);
+ extern Datum pg_logdir_ls(PG_FUNCTION_ARGS);

  /* not_in.c */
  extern Datum int4notin(PG_FUNCTION_ARGS);
Index: src/test/regress/expected/rules.out
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/expected/rules.out,v
retrieving revision 1.106
diff -c -c -r1.106 rules.out
*** src/test/regress/expected/rules.out    31 Jul 2005 17:19:22 -0000    1.106
--- src/test/regress/expected/rules.out    11 Aug 2005 04:21:56 -0000
***************
*** 1280,1285 ****
--- 1280,1286 ----
   pg_group                 | SELECT pg_authid.rolname AS groname, pg_authid.oid AS grosysid, ARRAY(SELECT
pg_auth_members.memberFROM pg_auth_members WHERE (pg_auth_members.roleid = pg_authid.oid)) AS grolist FROM pg_authid
WHERE(NOT pg_authid.rolcanlogin); 
   pg_indexes               | SELECT n.nspname AS schemaname, c.relname AS tablename, i.relname AS indexname, t.spcname
AS"tablespace", pg_get_indexdef(i.oid) AS indexdef FROM ((((pg_index x JOIN pg_class c ON ((c.oid = x.indrelid))) JOIN
pg_classi ON ((i.oid = x.indexrelid))) LEFT JOIN pg_namespace n ON ((n.oid = c.relnamespace))) LEFT JOIN pg_tablespace
tON ((t.oid = i.reltablespace))) WHERE ((c.relkind = 'r'::"char") AND (i.relkind = 'i'::"char")); 
   pg_locks                 | SELECT l.locktype, l."database", l.relation, l.page, l.tuple, l.transactionid, l.classid,
l.objid,l.objsubid, l."transaction", l.pid, l."mode", l."granted" FROM pg_lock_status() l(locktype text, "database"
oid,relation oid, page integer, tuple smallint, transactionid xid, classid oid, objid oid, objsubid smallint,
"transaction"xid, pid integer, "mode" text, "granted" boolean); 
+  pg_logdir_ls             | SELECT a.filetime, a.filename FROM pg_logdir_ls() a(filetime timestamp without time zone,
filenametext); 
   pg_prepared_xacts        | SELECT p."transaction", p.gid, p."prepared", u.rolname AS "owner", d.datname AS
"database"FROM ((pg_prepared_xact() p("transaction" xid, gid text, "prepared" timestamp with time zone, ownerid oid,
dbidoid) LEFT JOIN pg_authid u ON ((p.ownerid = u.oid))) LEFT JOIN pg_database d ON ((p.dbid = d.oid))); 
   pg_roles                 | SELECT pg_authid.rolname, pg_authid.rolsuper, pg_authid.rolinherit,
pg_authid.rolcreaterole,pg_authid.rolcreatedb, pg_authid.rolcatupdate, pg_authid.rolcanlogin, pg_authid.rolconnlimit,
'********'::textAS rolpassword, pg_authid.rolvaliduntil, pg_authid.rolconfig, pg_authid.oid FROM pg_authid; 
   pg_rules                 | SELECT n.nspname AS schemaname, c.relname AS tablename, r.rulename, pg_get_ruledef(r.oid)
ASdefinition FROM ((pg_rewrite r JOIN pg_class c ON ((c.oid = r.ev_class))) LEFT JOIN pg_namespace n ON ((n.oid =
c.relnamespace)))WHERE (r.rulename <> '_RETURN'::name); 
***************
*** 1320,1326 ****
   shoelace_obsolete        | SELECT shoelace.sl_name, shoelace.sl_avail, shoelace.sl_color, shoelace.sl_len,
shoelace.sl_unit,shoelace.sl_len_cm FROM shoelace WHERE (NOT (EXISTS (SELECT shoe.shoename FROM shoe WHERE
(shoe.slcolor= shoelace.sl_color)))); 
   street                   | SELECT r.name, r.thepath, c.cname FROM ONLY road r, real_city c WHERE (c.outline ##
r.thepath);
   toyemp                   | SELECT emp.name, emp.age, emp."location", (12 * emp.salary) AS annualsal FROM emp;
! (44 rows)

  SELECT tablename, rulename, definition FROM pg_rules
      ORDER BY tablename, rulename;
--- 1321,1327 ----
   shoelace_obsolete        | SELECT shoelace.sl_name, shoelace.sl_avail, shoelace.sl_color, shoelace.sl_len,
shoelace.sl_unit,shoelace.sl_len_cm FROM shoelace WHERE (NOT (EXISTS (SELECT shoe.shoename FROM shoe WHERE
(shoe.slcolor= shoelace.sl_color)))); 
   street                   | SELECT r.name, r.thepath, c.cname FROM ONLY road r, real_city c WHERE (c.outline ##
r.thepath);
   toyemp                   | SELECT emp.name, emp.age, emp."location", (12 * emp.salary) AS annualsal FROM emp;
! (45 rows)

  SELECT tablename, rulename, definition FROM pg_rules
      ORDER BY tablename, rulename;
/*-------------------------------------------------------------------------
 *
 * genfile.c
 *
 *
 * Copyright (c) 2004, PostgreSQL Global Development Group
 *
 * Author: Andreas Pflug <pgadmin@pse-consulting.de>
 *
 * IDENTIFICATION
 *      $PostgreSQL: $
 *
 *-------------------------------------------------------------------------
 */
#include "postgres.h"

#include <sys/file.h>
#include <sys/stat.h>
#include <unistd.h>
#include <dirent.h>

#include "utils/builtins.h"
#include "miscadmin.h"
#include "storage/fd.h"
#include "catalog/pg_type.h"
#include "funcapi.h"

extern  char *Log_directory;

typedef struct
{
    char    *location;
    DIR        *dirdesc;
} directory_fctx;

/*
 * Return an absolute path. Argument may be absolute or
 * relative to the DataDir.
 */
static char *check_and_make_absolute(text *arg)
{
    char *filename;
    int filename_len = VARSIZE(arg) - VARHDRSZ;
    int datadir_len = strlen(DataDir);

    filename = palloc(filename_len + 1);
    memcpy(filename, VARDATA(arg), filename_len);
    filename[filename_len] = '\0';

    canonicalize_path(filename);
    filename_len = strlen(filename);    /* recompute */

    /*
     *    Prevent reference to the parent directory.
     *    "..a.." is a valid file name though.
     */
    if (strcmp(filename, "..") == 0 ||                            /* beginning */
        strncmp(filename, "../", 3) == 0 ||                        /* beginning */
        strcmp(filename, "/..") == 0 ||                            /* beginning */
        strncmp(filename, "../", 3) == 0 ||                        /* beginning */
        strstr(filename, "/../") != NULL ||                        /* middle */
        strncmp(filename + filename_len - 3, "/..", 3) == 0)    /* end */
            ereport(ERROR,
                  (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
                   (errmsg("Reference to a parent directory (\"..\") not allowed"))));

    if (is_absolute_path(filename))
    {
        /* The log directory might be outside our datadir, but allow it */
        if ((strncmp(filename, Log_directory, strlen(Log_directory)) == 0 &&
             (filename[strlen(Log_directory)] == '/' ||
              filename[strlen(Log_directory)] == '\0')) ||
            (strncmp(filename, DataDir, datadir_len) == 0 &&
             (filename[datadir_len] == '/' ||
              filename[datadir_len] == '\0')))
            return filename;

        ereport(ERROR,
                (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
                 (errmsg("Absolute path not allowed"))));
        return NULL;
    }
    else
    {
        char *absname = palloc(datadir_len + filename_len + 2);
        sprintf(absname, "%s/%s", DataDir, filename);
        pfree(filename);
        return absname;
    }
}


Datum pg_file_read(PG_FUNCTION_ARGS)
{
    size_t        bytes_to_read = PG_GETARG_INT64(2);
    int64        seek_offset = PG_GETARG_INT64(1);
    char         *buf = 0;
    size_t        nbytes;
    FILE        *file;
    char        *filename;

    if (!superuser())
        ereport(ERROR,
                (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
                 (errmsg("only superuser may access generic file functions"))));

    filename = check_and_make_absolute(PG_GETARG_TEXT_P(0));

    if ((file = AllocateFile(filename, PG_BINARY_R)) == NULL)
    {
        ereport(ERROR,
                (errcode_for_file_access(),
                 errmsg("could not open file %s for reading: %m", filename)));
        PG_RETURN_NULL();
    }

    if (fseeko(file, (off_t)seek_offset,
        (seek_offset >= 0) ? SEEK_SET : SEEK_END) != 0)
    {
        ereport(ERROR,
                (errcode_for_file_access(),
                 errmsg("could not seek in file %s: %m", filename)));
        PG_RETURN_NULL();
    }

    buf = palloc(bytes_to_read + VARHDRSZ);

    nbytes = fread(VARDATA(buf), bytes_to_read, 1, file);
    FreeFile(file);

    if (nbytes < 0)
    {
        ereport(ERROR,
                (errcode_for_file_access(),
                 errmsg("could not read file %s: %m", filename)));
        PG_RETURN_NULL();
    }
    VARATT_SIZEP(buf) = nbytes + VARHDRSZ;

    pfree(filename);
    PG_RETURN_TEXT_P(buf);
}


Datum pg_file_stat(PG_FUNCTION_ARGS)
{
    AttInMetadata *attinmeta;
    char        *filename = check_and_make_absolute(PG_GETARG_TEXT_P(0));
    struct stat fst;
    char        lenbuf[30], cbuf[30], abuf[30], mbuf[30], dirbuf[2];
    char        *values[5] = {lenbuf, cbuf, abuf, mbuf, dirbuf};
    pg_time_t    timestamp;
    HeapTuple    tuple;
    TupleDesc    tupdesc = CreateTemplateTupleDesc(5, false);

    if (!superuser())
        ereport(ERROR,
                (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
                 (errmsg("only superuser may access generic file functions"))));

    TupleDescInitEntry(tupdesc, (AttrNumber) 1, "length", INT8OID, -1, 0);
    TupleDescInitEntry(tupdesc, (AttrNumber) 2, "ctime", TIMESTAMPOID, -1, 0);
    TupleDescInitEntry(tupdesc, (AttrNumber) 3, "atime", TIMESTAMPOID, -1, 0);
    TupleDescInitEntry(tupdesc, (AttrNumber) 4, "mtime", TIMESTAMPOID, -1, 0);
    TupleDescInitEntry(tupdesc, (AttrNumber) 5, "isdir", BOOLOID, -1, 0);
    attinmeta = TupleDescGetAttInMetadata(tupdesc);

    if (stat(filename, &fst) < 0)
    {
        ereport(WARNING,
                (errcode_for_file_access(),
                 errmsg("could not stat file %s: %m", filename)));
        MemSet(values, 0, sizeof(char *) * 5);
        tuple = BuildTupleFromCStrings(attinmeta, values);
    }
    else
    {
        snprintf(lenbuf, 30, INT64_FORMAT, (int64)fst.st_size);

        timestamp = fst.st_ctime;
        pg_strftime(cbuf, 30, "%F %T", pg_localtime(×tamp, global_timezone));

        timestamp = fst.st_atime;
        pg_strftime(abuf, 30, "%F %T", pg_localtime(×tamp, global_timezone));

        timestamp = fst.st_mtime;
        pg_strftime(mbuf, 30, "%F %T", pg_localtime(×tamp, global_timezone));

        if (fst.st_mode & S_IFDIR)
            strcpy(dirbuf, "t");
        else
            strcpy(dirbuf, "f");

        tuple = BuildTupleFromCStrings(attinmeta, values);
    }

    pfree(filename);
    PG_RETURN_DATUM(HeapTupleGetDatum(tuple));
}


Datum pg_dir_ls(PG_FUNCTION_ARGS)
{
    FuncCallContext *funcctx;
    struct dirent *de;
    directory_fctx *fctx;

    if (!superuser())
        ereport(ERROR,
                (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
                 (errmsg("only superuser may access generic file functions"))));

    if (SRF_IS_FIRSTCALL())
    {
        MemoryContext oldcontext;

        funcctx = SRF_FIRSTCALL_INIT();
        oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);

        fctx = palloc(sizeof(directory_fctx));
        fctx->location = check_and_make_absolute(PG_GETARG_TEXT_P(0));

        fctx->dirdesc = AllocateDir(fctx->location);

        if (!fctx->dirdesc)
            ereport(ERROR,
                    (errcode_for_file_access(),
                     errmsg("%s is not browsable: %m", fctx->location)));

        if (PG_ARGISNULL(1) || !PG_GETARG_BOOL(1))
        {
            pfree(fctx->location);
            fctx->location = NULL;
        }
        funcctx->user_fctx = fctx;
        MemoryContextSwitchTo(oldcontext);
    }

    funcctx = SRF_PERCALL_SETUP();
    fctx = (directory_fctx*) funcctx->user_fctx;

    if (!fctx->dirdesc)  /* not a readable directory  */
        SRF_RETURN_DONE(funcctx);

    while ((de = ReadDir(fctx->dirdesc, fctx->location)) != NULL)
    {
        char        *name;
        text        *result;
        int            len;

        if (!strcmp(de->d_name, ".") || !strcmp(de->d_name, ".."))
            continue;

        if (fctx->location)
        {
            char *path = palloc(strlen(fctx->location) + strlen(de->d_name) + 2);
            sprintf(path, "%s/%s", fctx->location, de->d_name);
            name = path;
        }
        else
            name = de->d_name;

        len = strlen(name);
        result = palloc(len + VARHDRSZ);
        VARATT_SIZEP(result) = len + VARHDRSZ;
        memcpy(VARDATA(result), name, len);

        SRF_RETURN_NEXT(funcctx, PointerGetDatum(result));
    }

    FreeDir(fctx->dirdesc);
    SRF_RETURN_DONE(funcctx);
}

Re: [HACKERS] For review: Server instrumentation patch

From
Andreas Pflug
Date:
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
>     -----------------------------------------------------------------------------
>
>      (12287,"2005-08-11 00:06:30","2005-08-11 00:06:43","2005-08-11 00:06:30",f)
>     (1 row)
>
> Shouldn't this return multiple labeled columns rather than an array?

pg_show_all_settings output is equally unreadable, designed not to be
used directly.

Regards,
Andreas

Re: [HACKERS] For review: Server instrumentation patch

From
Bruce Momjian
Date:
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
logic to decode the log file name and convert it to a timestamp.  That
seemed best done in the client.

I also renamed a number of the functions to be use verb-noun, rather
than noun-verb.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
Index: doc/src/sgml/func.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/func.sgml,v
retrieving revision 1.276
diff -c -c -r1.276 func.sgml
*** doc/src/sgml/func.sgml    2 Aug 2005 16:11:56 -0000    1.276
--- doc/src/sgml/func.sgml    12 Aug 2005 03:18:29 -0000
***************
*** 9061,9066 ****
--- 9061,9069 ----
     <indexterm zone="functions-admin">
      <primary>pg_cancel_backend</primary>
     </indexterm>
+    <indexterm zone="functions-admin">
+     <primary>pg_reload_conf</primary>
+    </indexterm>

     <indexterm zone="functions-admin">
      <primary>signal</primary>
***************
*** 9068,9074 ****
     </indexterm>

     <para>
!     The function shown in <xref
      linkend="functions-admin-signal-table"> sends control signals to
      other server processes.  Use of this function is restricted
      to superusers.
--- 9071,9077 ----
     </indexterm>

     <para>
!     The functions shown in <xref
      linkend="functions-admin-signal-table"> sends control signals to
      other server processes.  Use of this function is restricted
      to superusers.
***************
*** 9090,9115 ****
         <entry><type>int</type></entry>
         <entry>Cancel a backend's current query</entry>
        </row>
       </tbody>
      </tgroup>
     </table>

     <para>
!     This function returns 1 if successful, 0 if not successful.
      The process ID (<literal>pid</literal>) of an active backend can be found
      from the <structfield>procpid</structfield> column in the
      <structname>pg_stat_activity</structname> view, or by listing the <command>postgres</command>
      processes on the server with <application>ps</>.
     </para>

     <indexterm zone="functions-admin">
      <primary>pg_start_backup</primary>
     </indexterm>
-
     <indexterm zone="functions-admin">
      <primary>pg_stop_backup</primary>
     </indexterm>
-
     <indexterm zone="functions-admin">
      <primary>backup</primary>
     </indexterm>
--- 9093,9128 ----
         <entry><type>int</type></entry>
         <entry>Cancel a backend's current query</entry>
        </row>
+       <row>
+        <entry>
+         <literal><function>pg_reload_conf</function>()</literal>
+         </entry>
+        <entry><type>int</type></entry>
+        <entry>Causes server processes to reload their configuration files</entry>
+       </row>
       </tbody>
      </tgroup>
     </table>

     <para>
!     These functions return 1 if successful, 0 if not successful.
      The process ID (<literal>pid</literal>) of an active backend can be found
      from the <structfield>procpid</structfield> column in the
      <structname>pg_stat_activity</structname> view, or by listing the <command>postgres</command>
      processes on the server with <application>ps</>.
     </para>
+    <para>
+     <function>pg_reload_conf</> sends a SIGHUP signal to the
+     postmaster, causing the reload of the configuration files
+     in all backend processes.
+    </para>

     <indexterm zone="functions-admin">
      <primary>pg_start_backup</primary>
     </indexterm>
     <indexterm zone="functions-admin">
      <primary>pg_stop_backup</primary>
     </indexterm>
     <indexterm zone="functions-admin">
      <primary>backup</primary>
     </indexterm>
***************
*** 9309,9314 ****
--- 9322,9434 ----
      appropriate.
     </para>

+    <para>
+     The functions shown in <xref
+     linkend="functions-admin-genfile"> provide native file access to
+     files on the machine hosting the server. Only files relative to
+     the cluster directory are allowed, and the logfile directory,
+     because the logfile directory might be stored outside the
+     cluster directory.  Use of these functions is restricted to
+     superusers.
+    </para>
+
+    <table id="functions-admin-genfile">
+     <title>Generic File Access Functions</title>
+     <tgroup cols="3">
+      <thead>
+       <row><entry>Name</entry> <entry>Return Type</entry> <entry>Description</entry>
+       </row>
+      </thead>
+
+      <tbody>
+       <row>
+        <entry>
+         <literal><function>pg_file_length</function>(<parameter>filename_text</parameter>)</literal>
+          <indexterm zone="functions-admin">
+           <primary>pg_file_length</primary>
+         </indexterm>
+        </entry>
+        <entry><type>int8</type></entry>
+        <entry>Returns the file length</entry>
+       </row>
+       <row>
+        <entry>
+
<literal><function>pg_ls_dir</function>(<parameter>dirname_text</parameter>,<parameter>fullpath_bool</parameter>)</literal>
+         <indexterm zone="functions-admin">
+          <primary>pg_ls_dir</primary>
+         </indexterm>
+        </entry>
+        <entry><type>setof text</type></entry>
+        <entry>List the contents of a directory</entry>
+       </row>
+       <row>
+        <entry>
+         <literal><function>pg_read_file</function>(<parameter>filename_text</parameter>,
+         <parameter>offset_int8</parameter>,<parameter>length_int8</parameter>)</literal>
+        </entry>
+        <entry><type>text</type></entry>
+        <entry>Returns the contents of a text file</entry>
+       </row>
+       <row>
+        <entry>
+         <literal><function>pg_stat_file</function>(<parameter>filename_text</parameter>)</literal>
+        </entry>
+        <entry><type>record</type></entry>
+        <entry>Returns information about the file</entry>
+       </row>
+      </tbody>
+     </tgroup>
+    </table>
+
+    <indexterm zone="functions-admin">
+     <primary>pg_read_file</primary>
+    </indexterm>
+    <para>
+     <function>pg_read_file()</> returns part of a textfile, starting
+     at the given offset, returning length bytes.  If offset is negative,
+     it is treated relative to the end of the file.
+    </para>
+
+    <indexterm zone="functions-admin">
+     <primary>pg_stat_file</primary>
+    </indexterm>
+    <para>
+     <function>pg_stat_file()</> returns a record containing the
+     length, last accessed timestamp, last modified timestamp,
+     creation timestamp, and a flag indicating if it is a directory.
+    </para>
+
+    <para>
+     The function shown in <xref
+     linkend="functions-admin-logfile"> forces the server
+     logfile to be rotated if <varname>redirect_stderr</>
+     is used for logging.   Use of this functions is restricted
+     to superusers.
+    </para>
+
+    <table id="functions-admin-logfile">
+     <title>Backend Logfile Functions</title>
+     <tgroup cols="3">
+      <thead>
+       <row><entry>Name</entry> <entry>Return Type</entry> <entry>Description</entry>
+       </row>
+      </thead>
+
+      <tbody>
+       <row>
+        <entry>
+         <literal><function>pg_rotate_logfile</function>()</literal>
+         <indexterm zone="functions-admin">
+          <primary>pg_rotate_logfile</primary>
+         </indexterm>
+         </entry>
+        <entry><type>int</type></entry>
+        <entry>Rotate logfile</entry>
+       </row>
+      </tbody>
+     </tgroup>
+    </table>
+
    </sect1>
  </chapter>

Index: src/backend/postmaster/postmaster.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v
retrieving revision 1.462
diff -c -c -r1.462 postmaster.c
*** src/backend/postmaster/postmaster.c    11 Aug 2005 21:11:44 -0000    1.462
--- src/backend/postmaster/postmaster.c    12 Aug 2005 03:18:32 -0000
***************
*** 3394,3399 ****
--- 3394,3402 ----
          }
      }

+     if (CheckPostmasterSignal(PMSIGNAL_ROTATE_LOGFILE) && SysLoggerPID != 0)
+         kill(SysLoggerPID, SIGUSR1);
+
      PG_SETMASK(&UnBlockSig);

      errno = save_errno;
Index: src/backend/postmaster/syslogger.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/postmaster/syslogger.c,v
retrieving revision 1.18
diff -c -c -r1.18 syslogger.c
*** src/backend/postmaster/syslogger.c    21 Jul 2005 18:06:12 -0000    1.18
--- src/backend/postmaster/syslogger.c    12 Aug 2005 03:18:33 -0000
***************
*** 101,106 ****
--- 101,107 ----
   * Flags set by interrupt handlers for later service in the main loop.
   */
  static volatile sig_atomic_t got_SIGHUP = false;
+ static volatile sig_atomic_t rotation_requested = false;


  /* Local subroutines */
***************
*** 117,122 ****
--- 118,124 ----
  static char *logfile_getname(pg_time_t timestamp);
  static void set_next_rotation_time(void);
  static void sigHupHandler(SIGNAL_ARGS);
+ static void sigUsr1Handler(SIGNAL_ARGS);


  /*
***************
*** 200,206 ****
      pqsignal(SIGQUIT, SIG_IGN);
      pqsignal(SIGALRM, SIG_IGN);
      pqsignal(SIGPIPE, SIG_IGN);
!     pqsignal(SIGUSR1, SIG_IGN);
      pqsignal(SIGUSR2, SIG_IGN);

      /*
--- 202,208 ----
      pqsignal(SIGQUIT, SIG_IGN);
      pqsignal(SIGALRM, SIG_IGN);
      pqsignal(SIGPIPE, SIG_IGN);
!     pqsignal(SIGUSR1, sigUsr1Handler);  /* request log rotation */
      pqsignal(SIGUSR2, SIG_IGN);

      /*
***************
*** 235,241 ****
      /* main worker loop */
      for (;;)
      {
-         bool        rotation_requested = false;
          bool        time_based_rotation = false;

  #ifndef WIN32
--- 237,242 ----
***************
*** 726,731 ****
--- 727,734 ----
      char       *filename;
      FILE       *fh;

+     rotation_requested = false;
+
      /*
       * When doing a time-based rotation, invent the new logfile name based
       * on the planned rotation time, not current time, to avoid "slippage"
***************
*** 876,878 ****
--- 879,888 ----
  {
      got_SIGHUP = true;
  }
+
+ /* SIGUSR1: set flag to rotate logfile */
+ static void
+ sigUsr1Handler(SIGNAL_ARGS)
+ {
+     rotation_requested = true;
+ }
Index: src/backend/utils/adt/Makefile
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/Makefile,v
retrieving revision 1.58
diff -c -c -r1.58 Makefile
*** src/backend/utils/adt/Makefile    29 Jul 2005 14:46:57 -0000    1.58
--- src/backend/utils/adt/Makefile    12 Aug 2005 03:18:33 -0000
***************
*** 24,30 ****
      tid.o timestamp.o varbit.o varchar.o varlena.o version.o xid.o \
      network.o mac.o inet_net_ntop.o inet_net_pton.o \
      ri_triggers.o pg_lzcompress.o pg_locale.o formatting.o \
!     ascii.o quote.o pgstatfuncs.o encode.o dbsize.o

  like.o: like.c like_match.c

--- 24,30 ----
      tid.o timestamp.o varbit.o varchar.o varlena.o version.o xid.o \
      network.o mac.o inet_net_ntop.o inet_net_pton.o \
      ri_triggers.o pg_lzcompress.o pg_locale.o formatting.o \
!     ascii.o quote.o pgstatfuncs.o encode.o dbsize.o genfile.o

  like.o: like.c like_match.c

Index: src/backend/utils/adt/misc.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/misc.c,v
retrieving revision 1.45
diff -c -c -r1.45 misc.c
*** src/backend/utils/adt/misc.c    4 Jul 2005 04:51:50 -0000    1.45
--- src/backend/utils/adt/misc.c    12 Aug 2005 03:18:33 -0000
***************
*** 21,34 ****
--- 21,43 ----
  #include "commands/dbcommands.h"
  #include "miscadmin.h"
  #include "storage/procarray.h"
+ #include "storage/pmsignal.h"
  #include "storage/fd.h"
  #include "utils/builtins.h"
+ #include "utils/elog.h"
  #include "funcapi.h"
  #include "catalog/pg_type.h"
  #include "catalog/pg_tablespace.h"
+ #include "postmaster/syslogger.h"

  #define atooid(x)  ((Oid) strtoul((x), NULL, 10))

+ typedef struct
+ {
+     char    *location;
+     DIR        *dirdesc;
+ } directory_fctx;
+

  /*
   * Check if data is Null
***************
*** 107,112 ****
--- 116,166 ----
      PG_RETURN_INT32(pg_signal_backend(PG_GETARG_INT32(0), SIGINT));
  }

+
+ Datum
+ pg_reload_conf(PG_FUNCTION_ARGS)
+ {
+     if (!superuser())
+         ereport(ERROR,
+                 (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+                  (errmsg("must be superuser to signal the postmaster"))));
+
+     if (kill(PostmasterPid, SIGHUP))
+     {
+         ereport(WARNING,
+                 (errmsg("failed to send signal to postmaster: %m")));
+
+         PG_RETURN_INT32(0);
+     }
+
+     PG_RETURN_INT32(1);
+ }
+
+
+ /*
+  * Rotate log file
+  */
+ Datum
+ pg_rotate_logfile(PG_FUNCTION_ARGS)
+ {
+     if (!superuser())
+         ereport(ERROR,
+                 (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+                  (errmsg("must be superuser to rotate log files"))));
+
+     if (!Redirect_stderr)
+     {
+         ereport(NOTICE,
+                 (errcode(ERRCODE_WARNING),
+                  errmsg("no logfile configured; rotation not supported")));
+         PG_RETURN_INT32(0);
+     }
+
+     SendPostmasterSignal(PMSIGNAL_ROTATE_LOGFILE);
+
+     PG_RETURN_INT32(0);
+ }
+
  #ifdef NOT_USED

  /* Disabled in 8.0 due to reliability concerns; FIXME someday */
Index: src/include/catalog/pg_proc.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/catalog/pg_proc.h,v
retrieving revision 1.380
diff -c -c -r1.380 pg_proc.h
*** src/include/catalog/pg_proc.h    2 Aug 2005 16:11:57 -0000    1.380
--- src/include/catalog/pg_proc.h    12 Aug 2005 03:18:41 -0000
***************
*** 3049,3054 ****
--- 3049,3071 ----
  DATA(insert OID = 2173 ( pg_stop_backup            PGNSP PGUID 12 f f t f v 0 25 "" _null_ _null_ _null_
pg_stop_backup- _null_ )); 
  DESCR("Finish taking an online backup");

+ DATA(insert OID = 2621 ( pg_reload_conf         PGNSP PGUID 12 f f t f v 0 23 "" _null_ _null_ _null_ pg_reload_conf
-_null_ )); 
+ DESCR("Reload configuration files");
+
+ DATA(insert OID = 2622 ( pg_rotate_logfile        PGNSP PGUID 12 f f t f v 0 23 "" _null_ _null_ _null_
pg_rotate_logfile- _null_ )); 
+ DESCR("Rotate log file");
+
+
+ DATA(insert OID = 2623 ( pg_stat_file        PGNSP PGUID 12 f f t f v 1 2249 "25" _null_ _null_ _null_ pg_stat_file -
_null_)); 
+ DESCR("Return file information");
+ DATA(insert OID = 2624 ( pg_file_length        PGNSP PGUID 14 f f t f v 1 20 "25" _null_ _null_ _null_ "SELECT len
FROMpg_stat_file($1) AS s(len int8, c timestamp, a timestamp, m timestamp, i bool)" - _null_ )); 
+ DESCR("Return file length");
+ DATA(insert OID = 2625 ( pg_read_file        PGNSP PGUID 12 f f t f v 3 25 "25 20 20" _null_ _null_ _null_
pg_read_file- _null_ )); 
+ DESCR("Read text from a file");
+ DATA(insert OID = 2626 ( pg_ls_dir            PGNSP PGUID 12 f f t t v 1 25 "25" _null_ _null_ _null_ pg_ls_dir -
_null_)); 
+ DESCR("List all file in a directory");
+
+
  /* Aggregates (moved here from pg_aggregate for 7.3) */

  DATA(insert OID = 2100 (  avg                PGNSP PGUID 12 t f f f i 1 1700 "20" _null_ _null_ _null_
aggregate_dummy- _null_ )); 
Index: src/include/storage/pmsignal.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/storage/pmsignal.h,v
retrieving revision 1.12
diff -c -c -r1.12 pmsignal.h
*** src/include/storage/pmsignal.h    28 Jun 2005 19:51:25 -0000    1.12
--- src/include/storage/pmsignal.h    12 Aug 2005 03:18:42 -0000
***************
*** 25,30 ****
--- 25,31 ----
      PMSIGNAL_PASSWORD_CHANGE,    /* pg_auth file has changed */
      PMSIGNAL_WAKEN_CHILDREN,    /* send a SIGUSR1 signal to all backends */
      PMSIGNAL_WAKEN_ARCHIVER,    /* send a NOTIFY signal to xlog archiver */
+     PMSIGNAL_ROTATE_LOGFILE,    /* send SIGUSR1 to syslogger to rotate logfile */

      NUM_PMSIGNALS                /* Must be last value of enum! */
  } PMSignalReason;
Index: src/include/utils/builtins.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/utils/builtins.h,v
retrieving revision 1.262
diff -c -c -r1.262 builtins.h
*** src/include/utils/builtins.h    29 Jul 2005 14:47:04 -0000    1.262
--- src/include/utils/builtins.h    12 Aug 2005 03:18:42 -0000
***************
*** 374,385 ****
--- 374,392 ----
  extern Datum pg_complete_relation_size_name(PG_FUNCTION_ARGS);
  extern Datum pg_size_pretty(PG_FUNCTION_ARGS);

+ /* genfile.c */
+ extern Datum pg_stat_file(PG_FUNCTION_ARGS);
+ extern Datum pg_read_file(PG_FUNCTION_ARGS);
+ extern Datum pg_ls_dir(PG_FUNCTION_ARGS);
+
  /* misc.c */
  extern Datum nullvalue(PG_FUNCTION_ARGS);
  extern Datum nonnullvalue(PG_FUNCTION_ARGS);
  extern Datum current_database(PG_FUNCTION_ARGS);
  extern Datum pg_cancel_backend(PG_FUNCTION_ARGS);
+ extern Datum pg_reload_conf(PG_FUNCTION_ARGS);
  extern Datum pg_tablespace_databases(PG_FUNCTION_ARGS);
+ extern Datum pg_rotate_logfile(PG_FUNCTION_ARGS);

  /* not_in.c */
  extern Datum int4notin(PG_FUNCTION_ARGS);
/*-------------------------------------------------------------------------
 *
 * genfile.c
 *
 *
 * Copyright (c) 2004, PostgreSQL Global Development Group
 *
 * Author: Andreas Pflug <pgadmin@pse-consulting.de>
 *
 * IDENTIFICATION
 *      $PostgreSQL: $
 *
 *-------------------------------------------------------------------------
 */
#include "postgres.h"

#include <sys/file.h>
#include <sys/stat.h>
#include <unistd.h>
#include <dirent.h>

#include "utils/builtins.h"
#include "miscadmin.h"
#include "storage/fd.h"
#include "catalog/pg_type.h"
#include "funcapi.h"

extern  char *Log_directory;

typedef struct
{
    char    *location;
    DIR        *dirdesc;
} directory_fctx;

/*
 * Return an absolute path. Argument may be absolute or
 * relative to the DataDir.
 */
static char *check_and_make_absolute(text *arg)
{
    char *filename;
    int filename_len = VARSIZE(arg) - VARHDRSZ;
    int datadir_len = strlen(DataDir);

    filename = palloc(filename_len + 1);
    memcpy(filename, VARDATA(arg), filename_len);
    filename[filename_len] = '\0';

    canonicalize_path(filename);
    filename_len = strlen(filename);    /* recompute */

    /*
     *    Prevent reference to the parent directory.
     *    "..a.." is a valid file name though.
     */
    if (strcmp(filename, "..") == 0 ||                            /* beginning */
        strncmp(filename, "../", 3) == 0 ||                        /* beginning */
        strcmp(filename, "/..") == 0 ||                            /* beginning */
        strncmp(filename, "../", 3) == 0 ||                        /* beginning */
        strstr(filename, "/../") != NULL ||                        /* middle */
        strncmp(filename + filename_len - 3, "/..", 3) == 0)    /* end */
            ereport(ERROR,
                  (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
                   (errmsg("Reference to a parent directory (\"..\") not allowed"))));

    if (is_absolute_path(filename))
    {
        /* The log directory might be outside our datadir, but allow it */
        if ((strncmp(filename, Log_directory, strlen(Log_directory)) == 0 &&
             (filename[strlen(Log_directory)] == '/' ||
              filename[strlen(Log_directory)] == '\0')) ||
            (strncmp(filename, DataDir, datadir_len) == 0 &&
             (filename[datadir_len] == '/' ||
              filename[datadir_len] == '\0')))
            return filename;

        ereport(ERROR,
                (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
                 (errmsg("Absolute path not allowed"))));
        return NULL;
    }
    else
    {
        char *absname = palloc(datadir_len + filename_len + 2);
        sprintf(absname, "%s/%s", DataDir, filename);
        pfree(filename);
        return absname;
    }
}


Datum pg_file_read(PG_FUNCTION_ARGS)
{
    size_t        bytes_to_read = PG_GETARG_INT64(2);
    int64        seek_offset = PG_GETARG_INT64(1);
    char         *buf = 0;
    size_t        nbytes;
    FILE        *file;
    char        *filename;

    if (!superuser())
        ereport(ERROR,
                (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
                 (errmsg("only superuser may access generic file functions"))));

    filename = check_and_make_absolute(PG_GETARG_TEXT_P(0));

    if ((file = AllocateFile(filename, PG_BINARY_R)) == NULL)
    {
        ereport(ERROR,
                (errcode_for_file_access(),
                 errmsg("could not open file %s for reading: %m", filename)));
        PG_RETURN_NULL();
    }

    if (fseeko(file, (off_t)seek_offset,
        (seek_offset >= 0) ? SEEK_SET : SEEK_END) != 0)
    {
        ereport(ERROR,
                (errcode_for_file_access(),
                 errmsg("could not seek in file %s: %m", filename)));
        PG_RETURN_NULL();
    }

    buf = palloc(bytes_to_read + VARHDRSZ);

    nbytes = fread(VARDATA(buf), bytes_to_read, 1, file);
    FreeFile(file);

    if (nbytes < 0)
    {
        ereport(ERROR,
                (errcode_for_file_access(),
                 errmsg("could not read file %s: %m", filename)));
        PG_RETURN_NULL();
    }
    VARATT_SIZEP(buf) = nbytes + VARHDRSZ;

    pfree(filename);
    PG_RETURN_TEXT_P(buf);
}


Datum pg_file_stat(PG_FUNCTION_ARGS)
{
    AttInMetadata *attinmeta;
    char        *filename = check_and_make_absolute(PG_GETARG_TEXT_P(0));
    struct stat fst;
    char        lenbuf[30], cbuf[30], abuf[30], mbuf[30], dirbuf[2];
    char        *values[5] = {lenbuf, cbuf, abuf, mbuf, dirbuf};
    pg_time_t    timestamp;
    HeapTuple    tuple;
    TupleDesc    tupdesc = CreateTemplateTupleDesc(5, false);

    if (!superuser())
        ereport(ERROR,
                (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
                 (errmsg("only superuser may access generic file functions"))));

    TupleDescInitEntry(tupdesc, (AttrNumber) 1, "length", INT8OID, -1, 0);
    TupleDescInitEntry(tupdesc, (AttrNumber) 2, "ctime", TIMESTAMPOID, -1, 0);
    TupleDescInitEntry(tupdesc, (AttrNumber) 3, "atime", TIMESTAMPOID, -1, 0);
    TupleDescInitEntry(tupdesc, (AttrNumber) 4, "mtime", TIMESTAMPOID, -1, 0);
    TupleDescInitEntry(tupdesc, (AttrNumber) 5, "isdir", BOOLOID, -1, 0);
    attinmeta = TupleDescGetAttInMetadata(tupdesc);

    if (stat(filename, &fst) < 0)
    {
        ereport(WARNING,
                (errcode_for_file_access(),
                 errmsg("could not stat file %s: %m", filename)));
        MemSet(values, 0, sizeof(char *) * 5);
        tuple = BuildTupleFromCStrings(attinmeta, values);
    }
    else
    {
        snprintf(lenbuf, 30, INT64_FORMAT, (int64)fst.st_size);

        timestamp = fst.st_ctime;
        pg_strftime(cbuf, 30, "%F %T", pg_localtime(×tamp, global_timezone));

        timestamp = fst.st_atime;
        pg_strftime(abuf, 30, "%F %T", pg_localtime(×tamp, global_timezone));

        timestamp = fst.st_mtime;
        pg_strftime(mbuf, 30, "%F %T", pg_localtime(×tamp, global_timezone));

        if (fst.st_mode & S_IFDIR)
            strcpy(dirbuf, "t");
        else
            strcpy(dirbuf, "f");

        tuple = BuildTupleFromCStrings(attinmeta, values);
    }

    pfree(filename);
    PG_RETURN_DATUM(HeapTupleGetDatum(tuple));
}


Datum pg_dir_ls(PG_FUNCTION_ARGS)
{
    FuncCallContext    *funcctx;
    struct dirent    *de;
    directory_fctx    *fctx;

    if (!superuser())
        ereport(ERROR,
                (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
                 (errmsg("only superuser may access generic file functions"))));

    if (SRF_IS_FIRSTCALL())
    {
        MemoryContext oldcontext;

        funcctx = SRF_FIRSTCALL_INIT();
        oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);

        fctx = palloc(sizeof(directory_fctx));
        fctx->location = check_and_make_absolute(PG_GETARG_TEXT_P(0));

        fctx->dirdesc = AllocateDir(fctx->location);

        if (!fctx->dirdesc)
            ereport(ERROR,
                    (errcode_for_file_access(),
                     errmsg("%s is not browsable: %m", fctx->location)));

        if (PG_ARGISNULL(1) || !PG_GETARG_BOOL(1))
        {
            pfree(fctx->location);
            fctx->location = NULL;
        }
        funcctx->user_fctx = fctx;
        MemoryContextSwitchTo(oldcontext);
    }

    funcctx = SRF_PERCALL_SETUP();
    fctx = (directory_fctx*) funcctx->user_fctx;

    if (!fctx->dirdesc)  /* not a readable directory  */
        SRF_RETURN_DONE(funcctx);

    while ((de = ReadDir(fctx->dirdesc, fctx->location)) != NULL)
    {
        char        *name;
        text        *result;
        int            len;

        if (!strcmp(de->d_name, ".") || !strcmp(de->d_name, ".."))
            continue;

        if (fctx->location)
        {
            char *path = palloc(strlen(fctx->location) + strlen(de->d_name) + 2);
            sprintf(path, "%s/%s", fctx->location, de->d_name);
            name = path;
        }
        else
            name = de->d_name;

        len = strlen(name);
        result = palloc(len + VARHDRSZ);
        VARATT_SIZEP(result) = len + VARHDRSZ;
        memcpy(VARDATA(result), name, len);

        SRF_RETURN_NEXT(funcctx, PointerGetDatum(result));
    }

    FreeDir(fctx->dirdesc);
    SRF_RETURN_DONE(funcctx);
}

Re: [HACKERS] For review: Server instrumentation patch

From
Andreas Pflug
Date:
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 internal's naming scheme from the user. We want
as few server side implementation specific client side code as possible.
    In addition, pg_logdir_ls filters files that are not log files.
You'll probably have trouble writing a query that performes this task.

Regards,
Andreas

Re: [HACKERS] For review: Server instrumentation patch

From
Alvaro Herrera
Date:
On Fri, Aug 12, 2005 at 11:28:22AM +0000, 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_logdir_ls is a bad idea, because the function is
> intended to hide server internal's naming scheme from the user. We want
> as few server side implementation specific client side code as possible.

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?

--
Alvaro Herrera (<alvherre[a]alvh.no-ip.org>)
Y dijo Dios: "Que sea Satanás, para que la gente no me culpe de todo a mí."
"Y que hayan abogados, para que la gente no culpe de todo a Satanás"

Re: [HACKERS] For review: Server instrumentation patch

From
Bruce Momjian
Date:
Alvaro Herrera wrote:
> On Fri, Aug 12, 2005 at 11:28:22AM +0000, 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_logdir_ls is a bad idea, because the function is
> > intended to hide server internal's naming scheme from the user. We want
> > as few server side implementation specific client side code as possible.
>
> 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?

Righ, pg_logdir_ls() was the function.  My feeling is that the
application has access to the log_directory and log_filename values and
can better and move flexibly filter pg_ls_dir() on the client end than
we can do on the server.  It just seemed like something that we better
done outside the server.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: [HACKERS] For review: Server instrumentation patch

From
Tom Lane
Date:
Bruce Momjian <pgman@candle.pha.pa.us> 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.
     *    "..a.." is a valid file name though.
     *
     * XXX this is BROKEN because it fails to prevent "C:.." on Windows.
     * Need access to "skip_drive" functionality to do it right.  (There
     * is no actual security hole because we'll prepend the DataDir below,
     * resulting in a just-plain-broken path, but we should give the right
     * error message instead.)
     */

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.

            regards, tom lane

Re: [HACKERS] For review: Server instrumentation patch

From
Tom Lane
Date:
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 first place, defined something like

bool path_contains_parent_reference(const char *path);

This is probably better since the whole business of looking for ".."
seems like something that should be in path.c and not elsewhere.

            regards, tom lane

Re: [HACKERS] For review: Server instrumentation patch

From
Andreas Pflug
Date:
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?

The logger subprocess patch originally didn't allow changing the the
logfile name pattern, to make sure it can be interpreted safely at a
later time. There's simply no way to mark the file with a timestamp
without the risk of it being arbitrarily modified by file commands, thus
screwing up the order of logfiles. Later, there was the request to
alternatively append a timestamp instead of a date pattern, to use
apache logging tools that will probably access the logfiles directly
anyway. This ended up in the log_filename GUC variable.

>
> Righ, pg_logdir_ls() was the function.  My feeling is that the
> application has access to the log_directory and log_filename values
> and can better and move flexibly filter pg_ls_dir() on the client end
>  than we can do on the server. It just seemed like something that we
> better done outside the server.

Outside the server means pure SQL, if you don't want to drop psql as
client. So how would your query to display all all available _logfiles_
look like? You'd need to check for a valid date, besides interpreting
pg_strfime's patterns. Doesn't sound exactly like fun, but I'm keen to
see how your equivalent to

SELECT *, pg_file_length(filename) AS len
   FROM pg_logdir_ls

looks like.

Regards,
Andreas

Re: [HACKERS] For review: Server instrumentation patch

From
Andreas Pflug
Date:
Tom Lane wrote:
> Andreas Pflug <pgadmin@pse-consulting.de> 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 the log filename pattern, anyway.
>
> I also don't have a problem with using the file mod times to sort them.

... until you copy the database cluster. See discussion from last year.

Regards,
Andreas

Re: [HACKERS] For review: Server instrumentation patch

From
Tom Lane
Date:
Andreas Pflug <pgadmin@pse-consulting.de> 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 the log filename pattern, anyway.

I also don't have a problem with using the file mod times to sort them.
Yes, you can break that if you go and poke the files --- but there's no
reason to do so, whereas there are many reasons for changing the log
filename pattern.

            regards, tom lane

Re: [HACKERS] For review: Server instrumentation patch

From
Bruce Momjian
Date:
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 strange anyway.  Is this for
> >> "security" reasons?
>
> The logger subprocess patch originally didn't allow changing the the
> logfile name pattern, to make sure it can be interpreted safely at a
> later time. There's simply no way to mark the file with a timestamp
> without the risk of it being arbitrarily modified by file commands, thus
> screwing up the order of logfiles. Later, there was the request to
> alternatively append a timestamp instead of a date pattern, to use
> apache logging tools that will probably access the logfiles directly
> anyway. This ended up in the log_filename GUC variable.
>
> >
> > Righ, pg_logdir_ls() was the function.  My feeling is that the
> > application has access to the log_directory and log_filename values
> > and can better and move flexibly filter pg_ls_dir() on the client end
> >  than we can do on the server. It just seemed like something that we
> > better done outside the server.
>
> Outside the server means pure SQL, if you don't want to drop psql as
> client. So how would your query to display all all available _logfiles_
> look like? You'd need to check for a valid date, besides interpreting
> pg_strfime's patterns. Doesn't sound exactly like fun, but I'm keen to
> see how your equivalent to

I don't assume people using psql will care about the current log files ---
it would be something done in C or another application language.  Aren't
the file names already ordered based on their file names, given the
default pattern, postgresql-%Y-%m-%d_%H%M%S.log?

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: [HACKERS] For review: Server instrumentation patch

From
Andreas Pflug
Date:
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 something done in C or another application language.  Aren't
> the file names already ordered based on their file names, given the
> default pattern, postgresql-%Y-%m-%d_%H%M%S.log?

The issue is _filtering_, not ordering. Since the log directory might be
directed to a different location, non-pgsql logfiles might be there too.
You'd probably won't expect to retrieve these files over a pgsql connection.

Regards,
Andreas



Re: [HACKERS] For review: Server instrumentation patch

From
Bruce Momjian
Date:
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
> interface for that either.

I don't see how listing the log files relates to editing the confuration
files.

> > it would be something done in C or another application language.  Aren't
> > the file names already ordered based on their file names, given the
> > default pattern, postgresql-%Y-%m-%d_%H%M%S.log?
>
> The issue is _filtering_, not ordering. Since the log directory might be
> directed to a different location, non-pgsql logfiles might be there too.
> You'd probably won't expect to retrieve these files over a pgsql connection.

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.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: [HACKERS] For review: Server instrumentation patch

From
Andreas Pflug
Date:
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 not interesting. Your
argumentation doesn't seem consequent to me.

>>>it would be something done in C or another application language.  Aren't
>>>the file names already ordered based on their file names, given the
>>>default pattern, postgresql-%Y-%m-%d_%H%M%S.log?
>>
>>The issue is _filtering_, not ordering. Since the log directory might be
>>directed to a different location, non-pgsql logfiles might be there too.
>>You'd probably won't expect to retrieve these files over a pgsql connection.
>
>
> 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 robust against filenames
that don't have valid dates too; imagine postgresql-2005-01-01_crash1.log.

Regards,
Andreas



Re: [HACKERS] For review: Server instrumentation patch

From
Bruce Momjian
Date:
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
> other aspect (viewing the logfile) to be not interesting. Your
> argumentation doesn't seem consequent to me.

To me monitoring (logfiles) and configuration (postgresql.conf) are
different, but if I can make it easy to do both from psql, great.

I can see making postgresql.conf easy (SET GLOBAL), I am unsure about
making pg_hba.conf easy (and many feel that way), and I am not sure how
adding a log directory listing took makes things significantly easier to
monitor log files.

What I can imagine making things very easy is a readonly GUC that returns
the current log file name.  That I think should be in the backend, and I
can see a query that combines that with pg_read_file() that prints the
last 1000 bytes from the file.

    SELECT pg_read_file(t1.setting, -1000, 1000);
    FROM     (SELECT setting FROM pg_settings WHERE NAME = 'log_current_name') AS t1

> >>>it would be something done in C or another application language.  Aren't
> >>>the file names already ordered based on their file names, given the
> >>>default pattern, postgresql-%Y-%m-%d_%H%M%S.log?
> >>
> >>The issue is _filtering_, not ordering. Since the log directory might be
> >>directed to a different location, non-pgsql logfiles might be there too.
> >>You'd probably won't expect to retrieve these files over a pgsql connection.
> >
> >
> > 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 robust against filenames
> that don't have valid dates too; imagine postgresql-2005-01-01_crash1.log.

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    (
            SELECT pg_ls_dir(t1.setting)
            FROM     (SELECT setting FROM pg_settings WHERE NAME = 'log_directory') AS t1
        ) AS t2,
        (SELECT setting FROM pg_settings WHERE NAME = 'log_filename') AS t3
    WHERE  t2.pg_ls_dir LIKE regexp_replace(t3.setting, '%.*', '') || '%';

The one thing it doesn't do, as you mentioned, is check for valid dates,
but it is certainly more flexible than embedding something in the backend.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: [HACKERS] For review: Server instrumentation patch

From
Andreas Pflug
Date:
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    (
>             SELECT pg_ls_dir(t1.setting)
>             FROM     (SELECT setting FROM pg_settings WHERE NAME = 'log_directory') AS t1
>         ) AS t2,
>         (SELECT setting FROM pg_settings WHERE NAME = 'log_filename') AS t3
>     WHERE  t2.pg_ls_dir LIKE regexp_replace(t3.setting, '%.*', '') || '%';
>
> The one thing it doesn't do, as you mentioned, is check for valid dates,
> but it is certainly more flexible than embedding something in the backend.

The interesting part of pg_logdir_ls is the filetime, to enable

SELECT pg_file_unlink(filename)
   FROM pg_logdir_ls()
  WHERE filetime < now() - '30 days'::interval

Regards,
Andreas

Re: [HACKERS] For review: Server instrumentation patch

From
Tom Lane
Date:
Andreas Pflug <pgadmin@pse-consulting.de> 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 robust against filenames
> that don't have valid dates too; imagine postgresql-2005-01-01_crash1.log.

The proposed version of pg_logdir_ls could not be called "robust" in any
way at all, considering that it fails as soon as you modify the log_filename
pattern.

I concur with Bruce that this is better left to the application side.
I don't see any basic functionality gain from doing it in the server.
The client code can look at log_filename and do the filtering just as
well (or badly) as it could possibly be done in the server.  Moreover,
having a restriction like "this doesn't work unless you use this
log_filename setting" feels more reasonable on the client side than
inside the server.

            regards, tom lane

Re: [HACKERS] For review: Server instrumentation patch

From
Tom Lane
Date:
Bruce Momjian <pgman@candle.pha.pa.us> 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 for the newest mod date among
the files in the log directory.

            regards, tom lane

Re: [HACKERS] For review: Server instrumentation patch

From
Bruce Momjian
Date:
Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> 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 shame.

> ATM I think the best you can do is look for the newest mod date among
> the files in the log directory.

I guess, but then you have the pattern problem, especially if you are
putting things in /var/log where there are other log files too.

One idea would be to implement pg_ls_logdir() as a system view, and then
build a GUC on that, but I am not sure that is possible.  Here is an
updated version of the query that also checks the file extension:

    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 setting FROM pg_settings
         WHERE NAME = 'log_filename') AS t3
    WHERE  t2.pg_ls_dir LIKE regexp_replace(t3.setting, '%.*', '') ||
        '%' || regexp_replace(t3.setting, '.*\\.', '.') ;

                pg_ls_dir
    ----------------------------------
     postgresql-2005-08-12_211251.log
     postgresql-2005-08-13_000000.log
    (2 rows)

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?

    test=> select pg_stat_file('.');
                                   pg_stat_file
    ---------------------------------------------------------------------------

     (512,"2005-08-12 21:13:01","2005-08-13 07:08:54","2005-08-12 21:13:01",t)
    (1 row)

    test=> select pg_stat_file('.')[1];
    ERROR:  syntax error at or near "[" at character 25

We have system _tables_ that return columns, like pg_settings, but of
course that doesn't take any arguments, just a WHERE clause, so that
wouldn't work here.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: [HACKERS] For review: Server instrumentation patch

From
Andreas Pflug
Date:
Tom Lane wrote:
> Andreas Pflug <pgadmin@pse-consulting.de> 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 robust against filenames
>>that don't have valid dates too; imagine postgresql-2005-01-01_crash1.log.
>
>
> The proposed version of pg_logdir_ls could not be called "robust" in any
> way at all, considering that it fails as soon as you modify the log_filename
> pattern.

This is caused by the exposure of log_filename, I never proposed to do
that for good reasons. Any try to interpret it and read files back will
break finally when log_filename is changed at runtime, i.e. it's a
'break me' option by design.

Regards,
Andreas

Re: [HACKERS] For review: Server instrumentation patch

From
Andreas Pflug
Date:
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('postgresql.conf') AS st(length
int4, ctime timestamp, atime timestamp, mtime timestamp, isdir bool)

Regards,
Andreas

Re: [HACKERS] For review: Server instrumentation patch

From
Tom Lane
Date:
Bruce Momjian <pgman@candle.pha.pa.us> 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 setting FROM pg_settings
>          WHERE NAME = 'log_filename') AS t3
>     WHERE  t2.pg_ls_dir LIKE regexp_replace(t3.setting, '%.*', '') ||
>         '%' || regexp_replace(t3.setting, '.*\\.', '.') ;

This is unnecessarily complicated --- use current_setting, eg,

select * from pg_ls_dir(current_setting('log_directory'))
where pg_ls_dir like
      regexp_replace(current_setting('log_filename'), '%.', '%', 'g');


> 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?

"select * from ...".  See the documentation:

    Use it like this:

    SELECT *
    FROM pg_stat_file('filename')
         AS s(length int8, atime timestamptz, mtime timestamptz,
              ctime timestamptz, isdir bool);

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.

            regards, tom lane

Re: [HACKERS] For review: Server instrumentation patch

From
Bruce Momjian
Date:
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_settings:
>
> SELECT length, mtime FROM pg_file_stat('postgresql.conf') AS st(length
> int4, ctime timestamp, atime timestamp, mtime timestamp, isdir bool)

Ewe, that is ugly.  How does the user even know the data types?  int4 or
int8?  timestamp or timestamptz?  \df doesn't show it:

     pg_catalog | pg_stat_file    | record         | text

and it isn't in the documenation.  However, internally, it knows.  In
fact your example is wrong because the size it int8, not int4:

    test=> SELECT length, mtime FROM pg_stat_file('postgresql.conf') AS st(length
    test(> int4, ctime timestamp, atime timestamp, mtime timestamp, isdir bool);
    ERROR:  function return row and query-specified return row do not match
    DETAIL:  Returned type bigint at ordinal position 1, but query expects integer.

    test=> SELECT length, mtime FROM pg_stat_file('postgresql.conf') AS st(length
    test(> int8, ctime timestamp, atime timestamp, mtime timestamp, isdir bool);
     length |        mtime
    --------+---------------------
      12576 | 2005-08-12 21:12:36
    (1 row)

Let me repond to Tom's email.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: [HACKERS] For review: Server instrumentation patch

From
Bruce Momjian
Date:
Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> 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 setting FROM pg_settings
> >          WHERE NAME = 'log_filename') AS t3
> >     WHERE  t2.pg_ls_dir LIKE regexp_replace(t3.setting, '%.*', '') ||
> >         '%' || regexp_replace(t3.setting, '.*\\.', '.') ;
>
> This is unnecessarily complicated --- use current_setting, eg,
>
> select * from pg_ls_dir(current_setting('log_directory'))
> where pg_ls_dir like
>       regexp_replace(current_setting('log_filename'), '%.', '%', 'g');

Nice.

> > 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?
>
> "select * from ...".  See the documentation:
>
>     Use it like this:
>
>     SELECT *
>     FROM pg_stat_file('filename')
>          AS s(length int8, atime timestamptz, mtime timestamptz,
>               ctime timestamptz, isdir bool);
>
> 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 don't even document the return type of
the function, nor does \df show it.  This seems too hard to use.

I am worried that if we improve things in 8.2, we would then be changing
the API of the function.  Are the other functions returning records usable?
Could we fix it in a way that later improvements would maintain the same
API?

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: [HACKERS] For review: Server instrumentation patch

From
Tom Lane
Date:
Bruce Momjian <pgman@candle.pha.pa.us> 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 don't even document the return type of
> the function, nor does \df show it.  This seems too hard to use.

> I am worried that if we improve things in 8.2, we would then be changing
> the API of the function.

Yeah, we would.

> Are the other functions returning records usable?

All the other ones are meant to be used via views, so it doesn't matter
so much.  pg_stat_file can't very usefully be called through a view, so
we have a problem.

I'll see about installing an initdb-time kluge to make it use OUT
parameters.

            regards, tom lane

Re: [HACKERS] For review: Server instrumentation patch

From
Tom Lane
Date:
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
--------+------------------------+------------------------+------------------------+-------
  12578 | 2005-08-13 14:51:03-04 | 2005-08-13 14:50:32-04 | 2005-08-13 14:50:32-04 | f
(1 row)

I removed the separate pg_file_length() function, as it doesn't have any
significant notational advantage anymore; you can do

regression=# select (pg_stat_file('postgresql.conf')).length;
 length
--------
  12578
(1 row)

BTW, \df is no real help when it comes to stuff with OUT parameters;
it still says

regression=# \df pg_stat_file
                         List of functions
   Schema   |     Name     | Result data type | Argument data types
------------+--------------+------------------+---------------------
 pg_catalog | pg_stat_file | record           | text
(1 row)

Possibly we should try to improve that.

            regards, tom lane

Re: [HACKERS] For review: Server instrumentation patch

From
Bruce Momjian
Date:
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
> --------+------------------------+------------------------+------------------------+-------
>   12578 | 2005-08-13 14:51:03-04 | 2005-08-13 14:50:32-04 | 2005-08-13 14:50:32-04 | f
> (1 row)

Great.

> I removed the separate pg_file_length() function, as it doesn't have any
> significant notational advantage anymore; you can do

Perfect.  I was going to suggest that could be removed once pg_stat_file
was more usable.

> regression=# select (pg_stat_file('postgresql.conf')).length;
>  length
> --------
>   12578
> (1 row)

Great.  I was also wondering if that would work.  One more closed item!

> BTW, \df is no real help when it comes to stuff with OUT parameters;
> it still says
>
> regression=# \df pg_stat_file
>                          List of functions
>    Schema   |     Name     | Result data type | Argument data types
> ------------+--------------+------------------+---------------------
>  pg_catalog | pg_stat_file | record           | text
> (1 row)
>
> Possibly we should try to improve that.

Good point.  Let's see if people ask for it.  Because they don't need to
know the data types to use the function, we might be fine.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: [HACKERS] For review: Server instrumentation patch

From
Andreas Pflug
Date:
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
compatibility.

Regards,
Andreas

Re: [HACKERS] For review: Server instrumentation patch

From
Andrew Dunstan
Date:

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 breaking backwards
> compatibility.
>
>

If the windows packagers have added extra functions into their build of
postgres, then they can maintain them, no? Of course, this would
illustrate the danger of such a practice.  Postgres is surely only
obligated to try not to break backwards compatibility with its own
released code.

cheers

andrew

Re: [HACKERS] For review: Server instrumentation patch

From
Tom Lane
Date:
Andreas Pflug <pgadmin@pse-consulting.de> 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 installations, so you're breaking backwards
> compatibility.

What backwards compatibility?  Bruce already renamed several of these
functions.

            regards, tom lane

Re: [HACKERS] For review: Server instrumentation patch

From
Andreas Pflug
Date:
Tom Lane wrote:
> Andreas Pflug <pgadmin@pse-consulting.de> 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 installations, so you're breaking backwards
>>compatibility.
>
>
> What backwards compatibility?  Bruce already renamed several of these
> functions.

You're right. These arbitrary renames brake nearly all of the existing
code. Great.

Regards,
Andreas