Re: Admin functions contrib

These files add administrative functions to pgsql 7.5. All are used by
pgAdmin3 or will be used in the near future if available. This is meant
as contrib module, whilst IMHO all these functions should be integrated
into the backend.

Included are functions to

log file access
These where previously posted together with the logger subprocess patch
and might get committed to the backend code.

misc.
send SIGHUP to postmaster

generic file access functions
These are more restrictive than the previously posted: Write access is
necessary for files to rename and unlink, and paths are restricted to
PGDATA and the logdir.

size functions
These are 7.5 replacements for dbsize.

Regards,
Andreas

I talked to Tom about this today. First, I want to apologize for
running you around in circles in this. I don't think we are giving it
the attention it needs because of our schedule. I also think the
functionality is drifting into the "new features" territory and this is
also part of the delay you are seeing.

I think you did a great thing by breaking the patch into two parts: one
for logging, and the other for log reading and other stuff. The logging
part is already in the patch queue. As for the function below, I first
think the security issue brough up about them wasn't a valid concern
because as I stated someone could just load the plperl server-side
language and do anything to the OS.

In fact this might be the best solution for you. Instead of trying to
code read/write/rename/unlink and other functions into the backend as
hardcoded, why not just have pgadmin load plperlu and as the super-user
you have access to that functionality, and much more, especially with
the new plperl in 7.5. In fact, your goal of modifying the
postgresql.conf file is much more natural in perl than in the API you
supplied, and probably more reliable.

So, I suggest we get the logging code into the backend, and you can code
anything you want pgadmin to do in plperlu, and Win32 supports plperlu
too. The big advantage is that you can improve the plperlu functions
with every release of pgadmin.

---------------------------------------------------------------------------

Andreas Pflug wrote:
> These files add administrative functions to pgsql 7.5. All are used by
> pgAdmin3 or will be used in the near future if available. This is meant
> as contrib module, whilst IMHO all these functions should be integrated
> into the backend.
>
> Included are functions to
>
> log file access
> These where previously posted together with the logger subprocess patch
> and might get committed to the backend code.
>
> misc.
> send SIGHUP to postmaster
>
> generic file access functions
> These are more restrictive than the previously posted: Write access is
> necessary for files to rename and unlink, and paths are restricted to
> PGDATA and the logdir.
>
> size functions
> These are 7.5 replacements for dbsize.
>
> Regards,
> Andreas

> subdir = contrib/admin
> top_builddir = ../..
> include $(top_builddir)/src/Makefile.global
>
> MODULE_big = admin
> DATA_built = admin.sql
> DOCS = README.admin
> OBJS = size.o genfile.o misc.o
> include $(top_srcdir)/contrib/contrib-global.mk

> /* **********************************
> * Administrative functions
> * ********************************* */
>
> /* database object size functions (admin.c) */
>
> CREATE FUNCTION pg_tablespace_size(oid) RETURNS bigint
> AS 'MODULE_PATHNAME', 'pg_tablespace_size'
> LANGUAGE C STABLE STRICT;
>
> CREATE FUNCTION pg_database_size(oid) RETURNS bigint
> AS 'MODULE_PATHNAME', 'pg_database_size'
> LANGUAGE C STABLE STRICT;
>
> CREATE FUNCTION pg_relation_size(oid) RETURNS bigint
> AS 'MODULE_PATHNAME', 'pg_relation_size'
> LANGUAGE C STABLE STRICT;
>
> CREATE FUNCTION pg_size_pretty(bigint) RETURNS text
> AS 'MODULE_PATHNAME', 'pg_size_pretty'
> LANGUAGE C STABLE STRICT;
>
>
> /* generic file access functions (genfile.c) */
>
> CREATE FUNCTION pg_file_stat(text) RETURNS record
> AS 'MODULE_PATHNAME', 'pg_file_stat'
> LANGUAGE C VOLATILE STRICT;
>
> CREATE FUNCTION pg_file_length(text) RETURNS bigint
> AS 'SELECT len FROM pg_file_stat($1) AS s(len int8, c timestamp, a timestamp, m timestamp, i bool)'
> LANGUAGE SQL VOLATILE STRICT;
>
> CREATE FUNCTION pg_file_read(text, bigint, bigint) RETURNS text
> AS 'MODULE_PATHNAME', 'pg_file_read'
> LANGUAGE C VOLATILE STRICT;
>
> CREATE FUNCTION pg_file_write(text, text, bool) RETURNS bigint
> AS 'MODULE_PATHNAME', 'pg_file_write'
> LANGUAGE C VOLATILE STRICT;
>
> CREATE FUNCTION pg_file_rename(text, text, text) RETURNS bool
> AS 'MODULE_PATHNAME', 'pg_file_rename'
> LANGUAGE C VOLATILE STRICT;
>
> CREATE FUNCTION pg_file_unlink(text) RETURNS bool
> AS 'MODULE_PATHNAME', 'pg_file_unlink'
> LANGUAGE C VOLATILE STRICT;
>
> CREATE FUNCTION pg_file_rename(text, text) RETURNS bool
> AS 'SELECT pg_file_rename($1, $2, NULL); '
> LANGUAGE SQL VOLATILE STRICT;
>
> CREATE FUNCTION pg_dir_ls(text, bool) RETURNS setof text
> AS 'MODULE_PATHNAME', 'pg_dir_ls'
> LANGUAGE C VOLATILE STRICT;
>
>
> /* Miscellaneous functions (misc.c) */
>
> CREATE FUNCTION pg_reload_conf() RETURNS int4
> AS 'MODULE_PATHNAME', 'pg_reload_conf'
> LANGUAGE C STABLE STRICT;
>
> CREATE FUNCTION pg_logfile_rotate() RETURNS bool
> AS 'MODULE_PATHNAME', 'pg_logfile_rotate'
> LANGUAGE C STABLE STRICT;
>
> CREATE FUNCTION pg_logdir_ls() RETURNS setof record
> AS 'MODULE_PATHNAME', 'pg_logdir_ls'
> LANGUAGE C VOLATILE STRICT;
>
> CREATE VIEW pg_logdir_ls AS
> SELECT *
> FROM pg_logdir_ls() AS A
> (filetime timestamp, pid int4, filename text);

> Misc functions for administrative purposes
>
> size.c:
> -----------------------------
> int8 pg_tablespace_size(oid)
> int8 pg_database_size(oid)
> int8 pg_relation_size(oid)
> text pg_size_pretty(int8)
>
>
> genfile.c (superuser only):
> -----------------------------
> record pg_file_stat(fname text)
> int8 pg_file_length(fname text)
> text pg_file_read(fname text, offs int8, len int8)
> int8 pg_file_write(fname text, data text, append bool)
> bool pg_file_rename(oldname text, newname text)
> bool pg_file_rename(oldname text, newname text, archivname text)
> bool pg_file_unlink(fname text)
> setof text pg_dir_ls(dirname text, bool fullpath)
>
>
> misc.c (superuser only):
> -----------------------------
> int4 pg_reload_conf()
> bool pg_logfile_rotate()
> setof record pg_logdir_ls()
>
> VIEW pg_logdir_ls(filetime timestamp, pid int4, filename text)

>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/docs/faqs/FAQ.html

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

Bruce Momjian wrote:
> I talked to Tom about this today. First, I want to apologize for
> running you around in circles in this. I don't think we are giving it
> the attention it needs because of our schedule. I also think the
> functionality is drifting into the "new features" territory and this is
> also part of the delay you are seeing.
>
> I think you did a great thing by breaking the patch into two parts: one
> for logging, and the other for log reading and other stuff. The logging
> part is already in the patch queue. As for the function below, I first
> think the security issue brough up about them wasn't a valid concern
> because as I stated someone could just load the plperl server-side
> language and do anything to the OS.
>
> In fact this might be the best solution for you. Instead of trying to
> code read/write/rename/unlink and other functions into the backend as
> hardcoded, why not just have pgadmin load plperlu and as the super-user
> you have access to that functionality, and much more, especially with
> the new plperl in 7.5. In fact, your goal of modifying the
> postgresql.conf file is much more natural in perl than in the API you
> supplied, and probably more reliable.
>
> So, I suggest we get the logging code into the backend, and you can code
> anything you want pgadmin to do in plperlu, and Win32 supports plperlu
> too. The big advantage is that you can improve the plperlu functions
> with every release of pgadmin.

I do not agree on this. Administrative tools should require as few
additional backend packages as possible. What you're proposing is simply
a nightmare. Actually, IMHO all functions should be *backend* code, not
contrib code, even less arbitrary loadable language functions. Certainly
an external package relying on a loadable language is quite the
opposite, generating lots of support issues. It won't generate trust if
pgadmin documentation advises "install untrusted plperl to maintain your
machine".
Additionally, several of the functions are by no means new, but
replacements, did you notice pg_xxx_size? I posted this stuff as contrib
module to keep it off the feature freeze issue. If it still can't go
there, it must stay an external module which will be distributed as
pgadmin add-on. Reimplementing it as plperlu is crap.

Regards,
Andreas

Andreas Pflug wrote:
> > So, I suggest we get the logging code into the backend, and you can code
> > anything you want pgadmin to do in plperlu, and Win32 supports plperlu
> > too. The big advantage is that you can improve the plperlu functions
> > with every release of pgadmin.
>
> I do not agree on this. Administrative tools should require as few
> additional backend packages as possible. What you're proposing is simply
> a nightmare. Actually, IMHO all functions should be *backend* code, not
> contrib code, even less arbitrary loadable language functions. Certainly
> an external package relying on a loadable language is quite the
> opposite, generating lots of support issues. It won't generate trust if
> pgadmin documentation advises "install untrusted plperl to maintain your
> machine".
> Additionally, several of the functions are by no means new, but
> replacements, did you notice pg_xxx_size? I posted this stuff as contrib
> module to keep it off the feature freeze issue. If it still can't go
> there, it must stay an external module which will be distributed as
> pgadmin add-on. Reimplementing it as plperlu is crap.

Well, if plperlu was always compiled by default we would be OK. If it
isn't then you are right it would be a problem. I see the --with-perl
option to configure so you are right it might be a problem.

Basically I think we are converging on an answer that we can't do any of
this for 7.5. The scope has gone way beyond what we had at feature
freeze, and we can't even get it to work on Win32, which was one of the
big goals for easier Win32 administration, so I think we should just
shelve all this and get it right for 7.6 when we have time to address
this.

I have added this to the TODO list:

* Allow server logs to be read using SQL commands
* Allow server configuration parameters to be modified remotetly

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

Andreas Pflug wrote:
> Bruce Momjian wrote:
>
> >
> > Basically I think we are converging on an answer that we can't do any of
> > this for 7.5.
>
> If it's not going into the distribution as contrib or core, I'll package
> that as additional admin pack. I'm quite sure I can convince the win32
> installer packager guys to include that as default-on option as soon as
> I'm able to prove them how it's working.

Uh, but it isn't working on Win32, right? How do you fix that? During
beta? That would be OK as an add-on.

> > The scope has gone way beyond what we had at feature
> > freeze, and we can't even get it to work on Win32,
>
> ??? The functions will work for win32, are you talking about logging?

I mean logging.

> The win32 log issue isn't a serverlog rotation issue, as I stated also
> the current stderr output is affected! I'd clearly see that as a fix,
> whilst it might be more than 10 lines of code. I'll try to fix that
> tomorrow. The very log_destination=file and rotation code will be more
> or less the same as for ***x.

But does it work on Win32 and Unix? It has to.

> > I have added this to the TODO list:
> >
> > * Allow server logs to be read using SQL commands
> > * Allow server configuration parameters to be modified remotetly
>
> This is desirable in any case. But until 7.6 will be around, 1-1.5 years
> and ???,000 installations will happen. Since the functionality is
> available now or will be available VSN (certainly before 7.5 release),
> there's no good reason suppress it.

Yes, that is a problem but we have to cut or we will never get to beta.
Probably doing an add-on until 7.6 might be the best but you are working
on it so let's see.

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

Andreas Pflug <pgadmin(at)pse-consulting(dot)de> writes:
> Bruce Momjian wrote:
>> So, I suggest we get the logging code into the backend, and you can code
>> anything you want pgadmin to do in plperlu, and Win32 supports plperlu
>> too. The big advantage is that you can improve the plperlu functions
>> with every release of pgadmin.

> I do not agree on this. Administrative tools should require as few
> additional backend packages as possible.

This argument doesn't really hold water when the alternative is a
contrib package. It's considerably more likely that the installation
will have plperl than that it will have some random contrib package.

Also, what happens if you find that the contrib package doesn't do
everything you need? You'll be stuck for another PG release cycle,
whereas rejiggering a plperl function that pgadmin is defining for
itself is no problem.

I think that developing with plperl for a few months would be a good
path, because then you would have some actual field experience under
your belt to justify the specific design of the contrib or core backend
functions you want. Right now they look mighty scattershot.

> It won't generate trust if pgadmin documentation advises "install
> untrusted plperl to maintain your machine".

Nonsense. You are already expecting these people to give you superuser
access, no? They had better trust you already. (Actually, you wouldn't
even have to ask them --- you could just create plperlu for yourself
--- but I suppose that would seem impolite.)

> I posted this stuff as contrib
> module to keep it off the feature freeze issue.

Contrib does not have an exception for the feature freeze rule. It's a
little looser maybe but that doesn't mean we'll accept an entire new
module after freeze.

> Reimplementing it as plperlu is crap.

I'm sorry you feel that way. It seemed like a fairly attractive
alternative to me, especially for early development.

regards, tom lane

Bruce Momjian wrote:

>
> Basically I think we are converging on an answer that we can't do any of
> this for 7.5.

If it's not going into the distribution as contrib or core, I'll package
that as additional admin pack. I'm quite sure I can convince the win32
installer packager guys to include that as default-on option as soon as
I'm able to prove them how it's working.

> The scope has gone way beyond what we had at feature
> freeze, and we can't even get it to work on Win32,

??? The functions will work for win32, are you talking about logging?
The win32 log issue isn't a serverlog rotation issue, as I stated also
the current stderr output is affected! I'd clearly see that as a fix,
whilst it might be more than 10 lines of code. I'll try to fix that
tomorrow. The very log_destination=file and rotation code will be more
or less the same as for ***x.

> I have added this to the TODO list:
>
> * Allow server logs to be read using SQL commands
> * Allow server configuration parameters to be modified remotetly

This is desirable in any case. But until 7.6 will be around, 1-1.5 years
and ???,000 installations will happen. Since the functionality is
available now or will be available VSN (certainly before 7.5 release),
there's no good reason suppress it.

Regards,
Andreas

Bruce Momjian wrote:
>>
>>If it's not going into the distribution as contrib or core, I'll package
>>that as additional admin pack. I'm quite sure I can convince the win32
>>installer packager guys to include that as default-on option as soon as
>>I'm able to prove them how it's working.
>
>
> Uh, but it isn't working on Win32, right? How do you fix that? During
> beta? That would be OK as an add-on.

As I said, I'm going to fix the stderr issue. This will enable the
pending log rotation stuff more or less incidentially.

>
>
>>The win32 log issue isn't a serverlog rotation issue, as I stated also
>>the current stderr output is affected! I'd clearly see that as a fix,
>>whilst it might be more than 10 lines of code. I'll try to fix that
>>tomorrow. The very log_destination=file and rotation code will be more
>>or less the same as for ***x.
>
>
> But does it work on Win32 and Unix? It has to.

I meant to implement that stderr handling stuff for win32, in order to
have a clean stderr output. Unix isn't affected, it works as posted.

>
> Yes, that is a problem but we have to cut or we will never get to beta.
> Probably doing an add-on until 7.6 might be the best but you are working
> on it so let's see.

The add-on are the functions, not the log rotation. Log rotation for
unix is beta-ready, and virtually unchanged for weeks now. It will be
available shortly for win32 too, as soon as the stderr piping issue is
solved. I'm a bit tired of arguing, that's why I didn't split the
functions in a part that was proposed together with the core
functionality before feature freeze and an additional part. I feel that
the majority of installations (win32) will have them anyway.

Regards,
Andreas