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

Re: [HACKERS] For review: Server instrumentation patch

From
Bruce Momjian
Date:
Andreas Pflug wrote:
> 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.

True, but we have SELECT * FROM pg_settings() that does display the
titles.  Right now the values aren't even documented.

--
  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
Alvaro Herrera
Date:
On Thu, Aug 11, 2005 at 11:34:43AM -0400, Bruce Momjian wrote:
> Andreas Pflug wrote:
> > 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.
>
> True, but we have SELECT * FROM pg_settings() that does display the
> titles.  Right now the values aren't even documented.

So, did you try

select * from pg_file_stats('postgresql.conf');

--
Alvaro Herrera (<alvherre[a]alvh.no-ip.org>)
Si no sabes adonde vas, es muy probable que acabes en otra parte.

Re: [HACKERS] For review: Server instrumentation patch

From
Alvaro Herrera
Date:
On Thu, Aug 11, 2005 at 01:19:29PM -0400, Bruce Momjian wrote:
> Alvaro Herrera wrote:

> > So, did you try
> >
> > select * from pg_file_stats('postgresql.conf');
>
> Yes, I got:
>
>   ERROR:  a column definition list is required for functions returning "record"

Interesting.  I wonder why the function is not defined instead with OUT
parameters.  That way you don't have to declare the record at execution
time.  See my patch to pgstattuple for an example.

--
Alvaro Herrera (<alvherre[a]alvh.no-ip.org>)
"No es bueno caminar con un hombre muerto"

Re: [HACKERS] For review: Server instrumentation patch

From
Bruce Momjian
Date:
Alvaro Herrera wrote:
> On Thu, Aug 11, 2005 at 01:19:29PM -0400, Bruce Momjian wrote:
> > Alvaro Herrera wrote:
>
> > > So, did you try
> > >
> > > select * from pg_file_stats('postgresql.conf');
> >
> > Yes, I got:
> >
> >   ERROR:  a column definition list is required for functions returning "record"
>
> Interesting.  I wonder why the function is not defined instead with OUT
> parameters.  That way you don't have to declare the record at execution
> time.  See my patch to pgstattuple for an example.

Uh, where is that patch?  I can't find it.

--
  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:
Alvaro Herrera wrote:
> On Thu, Aug 11, 2005 at 11:34:43AM -0400, Bruce Momjian wrote:
> > Andreas Pflug wrote:
> > > 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.
> >
> > True, but we have SELECT * FROM pg_settings() that does display the
> > titles.  Right now the values aren't even documented.
>
> So, did you try
>
> select * from pg_file_stats('postgresql.conf');

Yes, I got:

  ERROR:  a column definition list is required for functions returning "record"


--
  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
Alvaro Herrera
Date:
On Thu, Aug 11, 2005 at 02:26:16PM -0400, Bruce Momjian wrote:
> Alvaro Herrera wrote:

> > Interesting.  I wonder why the function is not defined instead with OUT
> > parameters.  That way you don't have to declare the record at execution
> > time.  See my patch to pgstattuple for an example.
>
> Uh, where is that patch?  I can't find it.

Hmm, seems it never made it to the list, I can't find it in the
archives either.  Here it is.

--
Alvaro Herrera (<alvherre[a]alvh.no-ip.org>)
"¿Qué importan los años?  Lo que realmente importa es comprobar que
a fin de cuentas la mejor edad de la vida es estar vivo"  (Mafalda)

Attachment

Re: [HACKERS] For review: Server instrumentation patch

From
Tom Lane
Date:
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> Interesting.  I wonder why the function is not defined instead with OUT
> parameters.

Because bootstrap mode isn't capable of dealing with array columns,
so you can't define stuff in pg_proc.h that sets up an array of OUT
parameter types.  I tried to apply that idea for the pg_locks function
a month or two ago, but it blew up in my face :-(.

It'd be nice to fix this sometime, but not while we are so far past
feature freeze.

            regards, tom lane

Re: [HACKERS] For review: Server instrumentation patch

From
Bruce Momjian
Date:
Tom Lane wrote:
> Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> > Interesting.  I wonder why the function is not defined instead with OUT
> > parameters.
>
> Because bootstrap mode isn't capable of dealing with array columns,
> so you can't define stuff in pg_proc.h that sets up an array of OUT
> parameter types.  I tried to apply that idea for the pg_locks function
> a month or two ago, but it blew up in my face :-(.
>
> It'd be nice to fix this sometime, but not while we are so far past
> feature freeze.

The patch includes documentation about the meaning the return values,
and I think that is good enough.

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