Thread: pgsql: Make the pg_stat_activity view call a SRF

pgsql: Make the pg_stat_activity view call a SRF

From
mha@postgresql.org (Magnus Hagander)
Date:
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)

Re: pgsql: Make the pg_stat_activity view call a SRF

From
Tom Lane
Date:
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

Re: pgsql: Make the pg_stat_activity view call a SRF

From
Magnus Hagander
Date:
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

Re: pgsql: Make the pg_stat_activity view call a SRF

From
Tom Lane
Date:
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

Re: pgsql: Make the pg_stat_activity view call a SRF

From
Magnus Hagander
Date:
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


Re: pgsql: Make the pg_stat_activity view call a SRF

From
Bruce Momjian
Date:
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. +

Re: pgsql: Make the pg_stat_activity view call a SRF

From
Magnus Hagander
Date:
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


Re: pgsql: Make the pg_stat_activity view call a SRF

From
Bruce Momjian
Date:
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. +

Re: pgsql: Make the pg_stat_activity view call a SRF

From
Tom Lane
Date:
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

Re: pgsql: Make the pg_stat_activity view call a SRF

From
"Jaime Casanova"
Date:
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

Re: pgsql: Make the pg_stat_activity view call a SRF

From
Magnus Hagander
Date:
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