pgsql: Make the pg_stat_activity view call a SRF

Started by Nonameover 17 years ago16 messages
#1Noname
mha@postgresql.org

Log Message:
-----------
Make the pg_stat_activity view call a SRF (pg_stat_get_activity())
instead of calling a bunch of individual functions.

This function can also be called directly, taking a PID as an argument, to
return only the data for a single PID.

Modified Files:
--------------
pgsql/doc/src/sgml:
monitoring.sgml (r1.57 -> r1.58)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/doc/src/sgml/monitoring.sgml?r1=1.57&r2=1.58)
pgsql/src/backend/catalog:
system_views.sql (r1.49 -> r1.50)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/catalog/system_views.sql?r1=1.49&r2=1.50)
pgsql/src/backend/utils/adt:
pgstatfuncs.c (r1.49 -> r1.50)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/utils/adt/pgstatfuncs.c?r1=1.49&r2=1.50)
pgsql/src/include/catalog:
catversion.h (r1.455 -> r1.456)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/include/catalog/catversion.h?r1=1.455&r2=1.456)
pg_proc.h (r1.496 -> r1.497)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/include/catalog/pg_proc.h?r1=1.496&r2=1.497)
pgsql/src/test/regress/expected:
rules.out (r1.136 -> r1.137)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/test/regress/expected/rules.out?r1=1.136&r2=1.137)

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Noname (#1)
Re: pgsql: Make the pg_stat_activity view call a SRF

mha@postgresql.org (Magnus Hagander) writes:

Make the pg_stat_activity view call a SRF (pg_stat_get_activity())
instead of calling a bunch of individual functions.

This function can also be called directly, taking a PID as an argument, to
return only the data for a single PID.

Hmm, if you really intend the function to be used directly, then having
to provide the output column list for it is a pretty big usability hit.
Why not declare it with OUT parameters, instead?

regards, tom lane

#3Magnus Hagander
magnus@hagander.net
In reply to: Tom Lane (#2)
Re: pgsql: Make the pg_stat_activity view call a SRF

Tom Lane wrote:

mha@postgresql.org (Magnus Hagander) writes:

Make the pg_stat_activity view call a SRF (pg_stat_get_activity())
instead of calling a bunch of individual functions.

This function can also be called directly, taking a PID as an
argument, to return only the data for a single PID.

Hmm, if you really intend the function to be used directly, then
having to provide the output column list for it is a pretty big
usability hit. Why not declare it with OUT parameters, instead?

That does sound like a very good idea... I did notice that as being a
problem, but didn't know how to fix it without thinking more :-)
How do I do that in pg_proc.h? Is there some other function that does
this that I can peek at for inspiration?

//Magnus

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Magnus Hagander (#3)
Re: pgsql: Make the pg_stat_activity view call a SRF

Magnus Hagander <magnus@hagander.net> writes:

Tom Lane wrote:

Why not declare it with OUT parameters, instead?

That does sound like a very good idea... I did notice that as being a
problem, but didn't know how to fix it without thinking more :-)
How do I do that in pg_proc.h? Is there some other function that does
this that I can peek at for inspiration?

Sure, see pg_timezone_abbrevs(), pg_timezone_names().

(This only started to work recently, which is why we have so many
record-returning functions that don't do it that way. It might be
an idea to fix them all sooner or later.)

regards, tom lane

#5Magnus Hagander
magnus@hagander.net
In reply to: Tom Lane (#4)
Re: pgsql: Make the pg_stat_activity view call a SRF

Tom Lane wrote:

Magnus Hagander <magnus@hagander.net> writes:

Tom Lane wrote:

Why not declare it with OUT parameters, instead?

That does sound like a very good idea... I did notice that as being
a problem, but didn't know how to fix it without thinking more :-)
How do I do that in pg_proc.h? Is there some other function that
does this that I can peek at for inspiration?

Sure, see pg_timezone_abbrevs(), pg_timezone_names().

(This only started to work recently, which is why we have so many
record-returning functions that don't do it that way. It might be
an idea to fix them all sooner or later.)

Wow, that was easy. Not sure where I got the impression it was hard
from - probably last looked at it before it was made easy, and didn't
re-check :-)

Updated the pg_stat_get_activity() function to use this.

And yes, I agree that it's probably a very good idea to go over our
other SRFs and fix them all. I can take a look at that eventually, but
for now I think we stick it on the TODO?

//Magnus

#6Bruce Momjian
bruce@momjian.us
In reply to: Magnus Hagander (#5)
Re: pgsql: Make the pg_stat_activity view call a SRF

Magnus Hagander wrote:

Sure, see pg_timezone_abbrevs(), pg_timezone_names().

(This only started to work recently, which is why we have so many
record-returning functions that don't do it that way. It might be
an idea to fix them all sooner or later.)

Wow, that was easy. Not sure where I got the impression it was hard
from - probably last looked at it before it was made easy, and didn't
re-check :-)

Updated the pg_stat_get_activity() function to use this.

And yes, I agree that it's probably a very good idea to go over our
other SRFs and fix them all. I can take a look at that eventually, but
for now I think we stick it on the TODO?

Added to TODO:

* Fix system views like pg_stat_all_tables to use set-returning
functions, rather than views of per-column functions

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

#7Magnus Hagander
magnus@hagander.net
In reply to: Bruce Momjian (#6)
Re: pgsql: Make the pg_stat_activity view call a SRF

Bruce Momjian wrote:

Magnus Hagander wrote:

Sure, see pg_timezone_abbrevs(), pg_timezone_names().

(This only started to work recently, which is why we have so many
record-returning functions that don't do it that way. It might be
an idea to fix them all sooner or later.)

Wow, that was easy. Not sure where I got the impression it was hard
from - probably last looked at it before it was made easy, and didn't
re-check :-)

Updated the pg_stat_get_activity() function to use this.

And yes, I agree that it's probably a very good idea to go over our
other SRFs and fix them all. I can take a look at that eventually, but
for now I think we stick it on the TODO?

Added to TODO:

* Fix system views like pg_stat_all_tables to use set-returning
functions, rather than views of per-column functions

Thanks, and while I approve of that TODO, that's not actually the one I
was talking about in the email. The one I was talking about was "change
builtin set-returning functions to use OUT parameters so you can query
them without knowing the result format" or something like that.

So, please keep the one you added, but add this one as well.

//Magnus

#8Bruce Momjian
bruce@momjian.us
In reply to: Magnus Hagander (#7)
Re: pgsql: Make the pg_stat_activity view call a SRF

Magnus Hagander wrote:

Bruce Momjian wrote:

Magnus Hagander wrote:

Sure, see pg_timezone_abbrevs(), pg_timezone_names().

(This only started to work recently, which is why we have so many
record-returning functions that don't do it that way. It might be
an idea to fix them all sooner or later.)

Wow, that was easy. Not sure where I got the impression it was hard
from - probably last looked at it before it was made easy, and didn't
re-check :-)

Updated the pg_stat_get_activity() function to use this.

And yes, I agree that it's probably a very good idea to go over our
other SRFs and fix them all. I can take a look at that eventually, but
for now I think we stick it on the TODO?

Added to TODO:

* Fix system views like pg_stat_all_tables to use set-returning
functions, rather than views of per-column functions

Thanks, and while I approve of that TODO, that's not actually the one I
was talking about in the email. The one I was talking about was "change
builtin set-returning functions to use OUT parameters so you can query
them without knowing the result format" or something like that.

So, please keep the one you added, but add this one as well.

Uh, I need more details on this. Can you give an example?

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

#9Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#8)
Re: pgsql: Make the pg_stat_activity view call a SRF

Bruce Momjian <bruce@momjian.us> writes:

Magnus Hagander wrote:

Bruce Momjian wrote:

* Fix system views like pg_stat_all_tables to use set-returning
functions, rather than views of per-column functions

Thanks, and while I approve of that TODO, that's not actually the one I
was talking about in the email. The one I was talking about was "change
builtin set-returning functions to use OUT parameters so you can query
them without knowing the result format" or something like that.

So, please keep the one you added, but add this one as well.

Uh, I need more details on this. Can you give an example?

Good:

regression=# select * from pg_get_keywords();
word | catcode | catdesc
-------------------+---------+-----------------------
abort | U | Unreserved
absolute | U | Unreserved
access | U | Unreserved
...

Not so good:

regression=# select * from pg_show_all_settings();
ERROR: a column definition list is required for functions returning "record"

There's no longer any very good reason for built-in SRFs to not define
their own output record type.

regards, tom lane

#10Jaime Casanova
jcasanov@systemguards.com.ec
In reply to: Tom Lane (#9)
Re: pgsql: Make the pg_stat_activity view call a SRF

On Sat, Aug 16, 2008 at 12:03 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

regression=# select * from pg_show_all_settings();
ERROR: a column definition list is required for functions returning "record"

There's no longer any very good reason for built-in SRFs to not define
their own output record type.

is there any one doing this? if not i want to give it a try... seems
easy enough, even for me :)

--
regards,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. (593) 87171157

#11Magnus Hagander
magnus@hagander.net
In reply to: Jaime Casanova (#10)
Re: pgsql: Make the pg_stat_activity view call a SRF

Jaime Casanova wrote:

On Sat, Aug 16, 2008 at 12:03 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

regression=# select * from pg_show_all_settings();
ERROR: a column definition list is required for functions returning "record"

There's no longer any very good reason for built-in SRFs to not define
their own output record type.

is there any one doing this? if not i want to give it a try... seems
easy enough, even for me :)

It's on my list, but I haven't actually started it yet. So - take it away!

//Magnus

#12Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#9)
Re: [COMMITTERS] pgsql: Make the pg_stat_activity view call a SRF

Tom Lane wrote:

Thanks, and while I approve of that TODO, that's not actually the one I
was talking about in the email. The one I was talking about was "change
builtin set-returning functions to use OUT parameters so you can query
them without knowing the result format" or something like that.

So, please keep the one you added, but add this one as well.

Uh, I need more details on this. Can you give an example?

Good:

regression=# select * from pg_get_keywords();
word | catcode | catdesc
-------------------+---------+-----------------------
abort | U | Unreserved
absolute | U | Unreserved
access | U | Unreserved
...

Not so good:

regression=# select * from pg_show_all_settings();
ERROR: a column definition list is required for functions returning "record"

There's no longer any very good reason for built-in SRFs to not define
their own output record type.

TODO updated:

* Fix all set-returning system functions so they support a wildcard
target list

SELECT * FROM pg_get_keywords() works but SELECT * FROM
pg_show_all_settings() does not.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

#13Robert Treat
xzilla@users.sourceforge.net
In reply to: Bruce Momjian (#12)
Re: Re: [COMMITTERS] pgsql: Make the pg_stat_activity view call a SRF

On Monday 18 August 2008 10:53:51 Bruce Momjian wrote:

Tom Lane wrote:

Thanks, and while I approve of that TODO, that's not actually the one
I was talking about in the email. The one I was talking about was
"change builtin set-returning functions to use OUT parameters so you
can query them without knowing the result format" or something like
that.

So, please keep the one you added, but add this one as well.

Uh, I need more details on this. Can you give an example?

Good:

regression=# select * from pg_get_keywords();
word | catcode | catdesc
-------------------+---------+-----------------------
abort | U | Unreserved
absolute | U | Unreserved
access | U | Unreserved
...

Not so good:

regression=# select * from pg_show_all_settings();
ERROR: a column definition list is required for functions returning
"record"

There's no longer any very good reason for built-in SRFs to not define
their own output record type.

TODO updated:

* Fix all set-returning system functions so they support a wildcard
target list

SELECT * FROM pg_get_keywords() works but SELECT * FROM
pg_show_all_settings() does not.

If this isn't critical, and no one is working on it yet, I can see about
whittling away at it for 8.4.

--
Robert Treat
Build A Brighter LAMP :: Linux Apache {middleware} PostgreSQL

#14Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Treat (#13)
Re: Re: [COMMITTERS] pgsql: Make the pg_stat_activity view call a SRF

Robert Treat <xzilla@users.sourceforge.net> writes:

On Monday 18 August 2008 10:53:51 Bruce Momjian wrote:

TODO updated:
* Fix all set-returning system functions so they support a wildcard
target list

If this isn't critical, and no one is working on it yet, I can see about
whittling away at it for 8.4.

I think someone did already volunteer.

regards, tom lane

#15David Fetter
david@fetter.org
In reply to: Robert Treat (#13)
Re: Re: [COMMITTERS] pgsql: Make the pg_stat_activity view call a SRF

On Tue, Aug 19, 2008 at 10:03:04PM -0400, Robert Treat wrote:

On Monday 18 August 2008 10:53:51 Bruce Momjian wrote:

Tom Lane wrote:

Thanks, and while I approve of that TODO, that's not actually the one
I was talking about in the email. The one I was talking about was
"change builtin set-returning functions to use OUT parameters so you
can query them without knowing the result format" or something like
that.

So, please keep the one you added, but add this one as well.

Uh, I need more details on this. Can you give an example?

Good:

regression=# select * from pg_get_keywords();
word | catcode | catdesc
-------------------+---------+-----------------------
abort | U | Unreserved
absolute | U | Unreserved
access | U | Unreserved
...

Not so good:

regression=# select * from pg_show_all_settings();
ERROR: a column definition list is required for functions returning
"record"

There's no longer any very good reason for built-in SRFs to not define
their own output record type.

TODO updated:

* Fix all set-returning system functions so they support a wildcard
target list

SELECT * FROM pg_get_keywords() works but SELECT * FROM
pg_show_all_settings() does not.

If this isn't critical, and no one is working on it yet, I can see about
whittling away at it for 8.4.

Looks like there are just 5 of these:

SELECT n.nspname as "Schema",
p.proname as "Name",
pg_catalog.pg_get_function_result(p.oid) as "Result data type",
pg_catalog.pg_get_function_arguments(p.oid) as "Argument data types"
FROM pg_catalog.pg_proc p
LEFT JOIN pg_catalog.pg_namespace n ON n.oid = p.pronamespace
WHERE p.prorettype <> 'pg_catalog.cstring'::pg_catalog.regtype
AND p.proargtypes[0] IS DISTINCT FROM 'pg_catalog.cstring'::pg_catalog.regtype
AND NOT p.proisagg
AND pg_catalog.pg_get_function_result(p.oid) = 'SETOF record'
AND pg_catalog.pg_get_function_arguments(p.oid) !~ 'OUT'

Schema | Name | Result data type | Argument data types
------------+-----------------------+------------------+---------------------
pg_catalog | pg_cursor | SETOF record |
pg_catalog | pg_lock_status | SETOF record |
pg_catalog | pg_prepared_statement | SETOF record |
pg_catalog | pg_prepared_xact | SETOF record |
pg_catalog | pg_show_all_settings | SETOF record |
(5 rows)

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

#16Jaime Casanova
jcasanov@systemguards.com.ec
In reply to: David Fetter (#15)
Re: Re: [COMMITTERS] pgsql: Make the pg_stat_activity view call a SRF

On Tue, Aug 19, 2008 at 10:56 PM, David Fetter <david@fetter.org> wrote:

If this isn't critical, and no one is working on it yet, I can see about
whittling away at it for 8.4.

i'm doing it...

Looks like there are just 5 of these:

pg_catalog | pg_cursor | SETOF record |
pg_catalog | pg_lock_status | SETOF record |
pg_catalog | pg_prepared_statement | SETOF record |
pg_catalog | pg_prepared_xact | SETOF record |
pg_catalog | pg_show_all_settings | SETOF record |

i have done pg_lock_status and pg_show_all_settings the other three
will do tomorrow

--
regards,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. (593) 87171157