Thread: lock listing
The attached patch completes the TODO list item
* Add SHOW command to display locks
Using the SRF scheme discussed on -hackers yesterday. The only changes
to the patch since it was posted on -hackers are cosmetic: I adapted
the patch to apply cleanly against the latest CVS code, and changed
the name of the return view to pg_show_locks_result.
When/if the patch is applied, I'll send in another patch adding some
documentation, and perhaps some higher-level views that use the SRF
and the system catalogs to return some useful information.
This requires an initdb. Unless anyone sees a problem, please apply.
Cheers,
Neil
--
Neil Conway <neilconway@rogers.com>
PGP Key ID: DB3C29FC
Attachment
nconway@klamath.dyndns.org (Neil Conway) writes:
> The attached patch completes the TODO list item
> * Add SHOW command to display locks
Sorry for not having time to review this sooner, but better late than
never...
Why does this patch arbitrarily remove the #ifdefs and documentation
for USER_LOCKS? That seems quite unrelated to the stated purpose.
I'm very unthrilled with this approach to faking up a composite type
for pg_show_locks to return. Aside from being ugly, this will lead to a
broken dependency structure (pg_show_locks is pinned, being a
built-in function, but its return type isn't pinned and yet there's no
dependency for it). We need a better answer for making built-in
functions that return tuples. I don't have one yet, but I also don't
think that this function is so critical to have in the main backend that
we must put in a kluge to have it there. I still think that the right
short-term answer is to make the function be a contrib item so it can
have an install script. (I don't object to adding stuff to lmgr for the
function to call.)
> When/if the patch is applied, I'll send in another patch adding some
> documentation, and perhaps some higher-level views that use the SRF
> and the system catalogs to return some useful information.
I'd like to see the documentation and the views *first*, as evidence
that this is the right API for the function to have. It may be that
we'll need more or different processing in the function to produce
something that can be displayed usefully by a view.
Minor code complaints:
It seems to me that you could do one palloc, or at most three, while
holding the LockMgrLock --- doing three per holderTable entry will take
significantly longer.
> + tupdesc = RelationNameGetTupleDesc("pg_show_locks_result");
This is certainly unreliable in the schema world; you might get a tupledesc
for some other pg_show_locks_result relation than the one you want.
> + /*
> + * Preload all the locking information that we will eventually format
> + * and send out as a result set. This is palloc'ed, but since the
> + * MemoryContext is reset when the SRF finishes, we don't need to
> + * free it ourselves.
> + */
> + funccxt->user_fctx = (LockData *) palloc(sizeof(LockData));
> +
> + GetLockStatusData(funccxt->user_fctx);
This seems quite wrong; the active context when the function is called
will be a short-term context, which *will* get reset before the next
call. Have you tested this with --enable-cassert (which turns on
CLOBBER_FREED_MEMORY)? I think you need to switch into a longer-lived
context before calling GetLockStatusData.
> + /* The OID of the locked relation */
> + snprintf(values[0], 16, "%d", lock->tag.relId);
OIDs are unsigned and must be printed with %u (same problem multiple
places). Also, why are you using "16" as the buffer length here when
you allocated 32 bytes?
> + * procede to the next one. (Note: "Go To Statement Considered
> + * Harmful" notwithstanding, GOTO is appropriate here IMHO)
Personally, I'd replace "top:" and the if/else construct with
/* loop in case currIdx proves not to have any more locks */
while (lockData->currIdx < lockData->nelements)
{
PROCLOCK *holder;
...
SRF_RETURN_NEXT(funccxt, result);
}
SRF_RETURN_DONE(funccxt);
and use "continue" in place of "goto". This is arguably cleaner,
and will definitely result in better code (many compilers punt on
optimization if there's any "goto" in the routine).
> + /* Cleanup and return next tuple in result set */
> + for (i = 0; i < NUM_ATTRS; i++)
> + pfree(values[i]);
> + pfree(values);
The pfree's here are largely a waste of cycles if I'm right about the
memory management behavior. Also, instead of using a NUM_ATTRS #define
you could get the number of attributes from the tupledesc, which might
be slightly more maintainable.
regards, tom lane
On Fri, Jul 19, 2002 at 01:21:10PM -0400, Tom Lane wrote:
> Why does this patch arbitrarily remove the #ifdefs and documentation
> for USER_LOCKS? That seems quite unrelated to the stated purpose.
I probably should have mentioned that -- it looked like dead code,
and the behavior that it referred to (a loadable module called
user-locks.c, which doesn't make sense to begin with) doesn't
appear to exist anymore.
> I'm very unthrilled with this approach to faking up a composite type
> for pg_show_locks to return.
As am I, and I agree that the proper long-term answer is some new
infrastructure for adding builtin SRFs. However, I don't think that's
a really good reason for rejecting the patch -- the dependancy
implications aren't a big deal IMHO (who drops pg_* anyway?), and
the ugliness is limited to a single place in initdb.sh, which would
be very easy to adapt to a new composite type setup.
> > When/if the patch is applied, I'll send in another patch adding some
> > documentation, and perhaps some higher-level views that use the SRF
> > and the system catalogs to return some useful information.
>
> I'd like to see the documentation and the views *first*, as evidence
> that this is the right API for the function to have. It may be that
> we'll need more or different processing in the function to produce
> something that can be displayed usefully by a view.
Well, my preference would be to write the documentation when/if the
patch is applied, so I don't waste my time documentating a feature
that may not exist.
As for the higher-level views, I'd personally view (heh) those as
optional -- IMHO the data returned by the SRF is sufficient for most
competent admins, and is certainly all that a GUI tool would need.
I'll certainly think about whether some high-level views are needed
(and if so, which ones), but I'm not too concerned about it.
> Minor code complaints:
>
> It seems to me that you could do one palloc, or at most three, while
> holding the LockMgrLock --- doing three per holderTable entry will take
> significantly longer.
Can you elaborate a bit on what changes you'd like to see? Can this
be done with the data structures I'm using now, or do I need to roll
my own?
> > + tupdesc = RelationNameGetTupleDesc("pg_show_locks_result");
>
> This is certainly unreliable in the schema world; you might get a tupledesc
> for some other pg_show_locks_result relation than the one you want.
Okay -- I've replaced that with:
RelationNameGetTupleDesc("pg_catalog.pg_show_locks_result");
(The SRF API docs should probably include a note on that.)
> > + /*
> > + * Preload all the locking information that we will eventually format
> > + * and send out as a result set. This is palloc'ed, but since the
> > + * MemoryContext is reset when the SRF finishes, we don't need to
> > + * free it ourselves.
> > + */
> > + funccxt->user_fctx = (LockData *) palloc(sizeof(LockData));
> > +
> > + GetLockStatusData(funccxt->user_fctx);
>
> This seems quite wrong; the active context when the function is called
> will be a short-term context, which *will* get reset before the next
> call. Have you tested this with --enable-cassert (which turns on
> CLOBBER_FREED_MEMORY)? I think you need to switch into a longer-lived
> context before calling GetLockStatusData.
Hmmm -- I tested it with '--enable-cassert' and didn't observe any
problems. I've revised to patch to allocate that long-term memory in
FuncCallContext.fctx, which I think should be sufficiently long-lived.
> > + /* The OID of the locked relation */
> > + snprintf(values[0], 16, "%d", lock->tag.relId);
>
> OIDs are unsigned and must be printed with %u (same problem multiple
> places). Also, why are you using "16" as the buffer length here when
> you allocated 32 bytes?
Ok, both of these are fixed.
> > + * procede to the next one. (Note: "Go To Statement Considered
> > + * Harmful" notwithstanding, GOTO is appropriate here IMHO)
>
> Personally, I'd replace "top:" and the if/else construct with
> [snip]
Ok, I've made this change.
Cheers,
Neil
--
Neil Conway <neilconway@rogers.com>
PGP Key ID: DB3C29FC
nconway@klamath.dyndns.org (Neil Conway) writes:
> On Fri, Jul 19, 2002 at 01:21:10PM -0400, Tom Lane wrote:
>> I'm very unthrilled with this approach to faking up a composite type
>> for pg_show_locks to return.
> As am I, and I agree that the proper long-term answer is some new
> infrastructure for adding builtin SRFs. However, I don't think that's
> a really good reason for rejecting the patch --
I'm not wanting to reject the patch; I'm wanting to restructure it as
additions to lmgr.c plus a contrib module that includes the API function
and perhaps some sample views. The contrib module's install script
could avoid these pesky problems because it can just CREATE a dummy
table or view and then CREATE the function. Once we have a better
answer about declaring built-in SRFs, we can migrate the code into the
core.
>> It seems to me that you could do one palloc, or at most three, while
>> holding the LockMgrLock --- doing three per holderTable entry will take
>> significantly longer.
> Can you elaborate a bit on what changes you'd like to see? Can this
> be done with the data structures I'm using now, or do I need to roll
> my own?
You could get down to 3 pallocs (which is enough of a savings for me)
by creating the workspace as an array of nelements PROCLOCK structs,
an array of nelements PGPROC structs, and an array of nlements LOCK
structs. The intermediate pointer arrays don't seem to buy anything
much except palloc overhead.
>> This seems quite wrong; the active context when the function is called
>> will be a short-term context, which *will* get reset before the next
>> call. Have you tested this with --enable-cassert (which turns on
>> CLOBBER_FREED_MEMORY)? I think you need to switch into a longer-lived
>> context before calling GetLockStatusData.
> Hmmm -- I tested it with '--enable-cassert' and didn't observe any
> problems. I've revised to patch to allocate that long-term memory in
> FuncCallContext.fctx, which I think should be sufficiently long-lived.
Yeah, that's where it should go. I'm puzzled why you didn't observe
a failure ...
regards, tom lane
Neil Conway wrote: > On Fri, Jul 19, 2002 at 01:21:10PM -0400, Tom Lane wrote: > > Why does this patch arbitrarily remove the #ifdefs and documentation > > for USER_LOCKS? That seems quite unrelated to the stated purpose. > > I probably should have mentioned that -- it looked like dead code, > and the behavior that it referred to (a loadable module called > user-locks.c, which doesn't make sense to begin with) doesn't > appear to exist anymore. Is it contrib/userlock/? -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
On Fri, Jul 19, 2002 at 08:07:52PM -0400, Bruce Momjian wrote: > Neil Conway wrote: > > On Fri, Jul 19, 2002 at 01:21:10PM -0400, Tom Lane wrote: > > > Why does this patch arbitrarily remove the #ifdefs and documentation > > > for USER_LOCKS? That seems quite unrelated to the stated purpose. > > > > I probably should have mentioned that -- it looked like dead code, > > and the behavior that it referred to (a loadable module called > > user-locks.c, which doesn't make sense to begin with) doesn't > > appear to exist anymore. > > Is it contrib/userlock/? Ah, thanks for pointing that out (I guess I was searching within src/). I suppose it can stay, although I'm not sure that it's very useful. Legal Question: the #ifdef USER_LOCKS code is only useful in conjunction with the contrib/userlock module, which is GPL'd. Would that mean that part of the backend "depends" upon GPL'd software in order to operate? I have no idea what the answer is, just curious... Cheers, Neil -- Neil Conway <neilconway@rogers.com> PGP Key ID: DB3C29FC
Neil Conway wrote: > On Fri, Jul 19, 2002 at 08:07:52PM -0400, Bruce Momjian wrote: > > Neil Conway wrote: > > > On Fri, Jul 19, 2002 at 01:21:10PM -0400, Tom Lane wrote: > > > > Why does this patch arbitrarily remove the #ifdefs and documentation > > > > for USER_LOCKS? That seems quite unrelated to the stated purpose. > > > > > > I probably should have mentioned that -- it looked like dead code, > > > and the behavior that it referred to (a loadable module called > > > user-locks.c, which doesn't make sense to begin with) doesn't > > > appear to exist anymore. > > > > Is it contrib/userlock/? > > Ah, thanks for pointing that out (I guess I was searching within src/). > > I suppose it can stay, although I'm not sure that it's very useful. > > Legal Question: the #ifdef USER_LOCKS code is only useful in conjunction > with the contrib/userlock module, which is GPL'd. Would that mean that > part of the backend "depends" upon GPL'd software in order to operate? > > I have no idea what the answer is, just curious... Even with userlocks enabled, I don't think you have to use them to use PostgreSQL, so you should be OK. Of course, as soon as you load the userlocks module, you have GPL'ed yourself. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
On Fri, Jul 19, 2002 at 08:59:50PM -0400, Bruce Momjian wrote: > Even with userlocks enabled, I don't think you have to use them to use > PostgreSQL, so you should be OK. Of course, as soon as you load the > userlocks module, you have GPL'ed yourself. Ok, sounds reasonable to me. While we're on the topic, there was talk a couple months ago of removing or asking the authors to relicense the contrib/ modules that are GPL'd (or perhaps LGPL'd). Would this be a good idea? If so, I can do it... Cheers, Neil -- Neil Conway <neilconway@rogers.com> PGP Key ID: DB3C29FC
Neil Conway wrote: > On Fri, Jul 19, 2002 at 08:59:50PM -0400, Bruce Momjian wrote: > > Even with userlocks enabled, I don't think you have to use them to use > > PostgreSQL, so you should be OK. Of course, as soon as you load the > > userlocks module, you have GPL'ed yourself. > > Ok, sounds reasonable to me. > > While we're on the topic, there was talk a couple months ago of removing > or asking the authors to relicense the contrib/ modules that are GPL'd > (or perhaps LGPL'd). Would this be a good idea? If so, I can do it... Yes, Tom was going to address this. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Neil Conway wrote:
>> While we're on the topic, there was talk a couple months ago of removing
>> or asking the authors to relicense the contrib/ modules that are GPL'd
>> (or perhaps LGPL'd). Would this be a good idea? If so, I can do it...
> Yes, Tom was going to address this.
Yeah, it's the sort of thing that I think a core member has to do ...
as the most junior core member, I'm not sure *I'm* the most appropriate
person to do it, but ...
regards, tom lane
On Fri, Jul 19, 2002 at 05:15:43PM -0400, Tom Lane wrote: > nconway@klamath.dyndns.org (Neil Conway) writes: > > On Fri, Jul 19, 2002 at 01:21:10PM -0400, Tom Lane wrote: > >> I'm very unthrilled with this approach to faking up a composite type > >> for pg_show_locks to return. > > > As am I, and I agree that the proper long-term answer is some new > > infrastructure for adding builtin SRFs. However, I don't think that's > > a really good reason for rejecting the patch -- > > I'm not wanting to reject the patch; I'm wanting to restructure it as > additions to lmgr.c plus a contrib module that includes the API function > and perhaps some sample views. The contrib module's install script > could avoid these pesky problems because it can just CREATE a dummy > table or view and then CREATE the function. Once we have a better > answer about declaring built-in SRFs, we can migrate the code into the > core. Personally, that doesn't strike me as a lot cleaner than just putting the code into the core in the first place. Since the changes to adapt the SRF to a new composite type scheme would be trivial (less than 20 lines of changes, probably less), I'd personally vote to include it in the core, and put up with a little bit of ugliness in initdb until we get a proper solution. I've attached a revised patch which incorporates Tom's suggestions, as well as including a few more code cleanups/fixes, and doesn't remove the USER_LOCKS code. Cheers, Neil -- Neil Conway <neilconway@rogers.com> PGP Key ID: DB3C29FC
Attachment
Neil Conway writes: > Using the SRF scheme discussed on -hackers yesterday. The only changes > to the patch since it was posted on -hackers are cosmetic: I adapted > the patch to apply cleanly against the latest CVS code, and changed > the name of the return view to pg_show_locks_result. A view or function shouldn't be called "show". Just pg_lock[s] should be fine. -- Peter Eisentraut peter_e@gmx.net
Neil Conway wrote: > On Fri, Jul 19, 2002 at 05:15:43PM -0400, Tom Lane wrote: > > nconway@klamath.dyndns.org (Neil Conway) writes: > > > On Fri, Jul 19, 2002 at 01:21:10PM -0400, Tom Lane wrote: > > >> I'm very unthrilled with this approach to faking up a composite type > > >> for pg_show_locks to return. > > > > > As am I, and I agree that the proper long-term answer is some new > > > infrastructure for adding builtin SRFs. However, I don't think that's > > > a really good reason for rejecting the patch -- > > > > I'm not wanting to reject the patch; I'm wanting to restructure it as > > additions to lmgr.c plus a contrib module that includes the API function > > and perhaps some sample views. The contrib module's install script > > could avoid these pesky problems because it can just CREATE a dummy > > table or view and then CREATE the function. Once we have a better > > answer about declaring built-in SRFs, we can migrate the code into the > > core. > > Personally, that doesn't strike me as a lot cleaner than just putting > the code into the core in the first place. Since the changes to adapt > the SRF to a new composite type scheme would be trivial (less than > 20 lines of changes, probably less), I'd personally vote to include it > in the core, and put up with a little bit of ugliness in initdb until > we get a proper solution. Agreed. We are not running a beauty context here. It is a TODO item and users want it. Get it to into the core and fix it when the SRF API improves. I agree with Peter that it is better to rename it pg_lock*. The show isn't needed and it makes it look more like our other system info tables. Please resubmit your patch. Thanks. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
On Tue, Jul 23, 2002 at 05:52:39PM -0400, Bruce Momjian wrote:
> I agree with Peter that it is better to rename it pg_lock*. The show
> isn't needed and it makes it look more like our other system info
> tables.
Well, it's not a system info table -- it's a built-in function, and I'm
not aware of any naming conventions for those. Personally, I find it a
bit confusing to do:
SELECT * FROM pg_tables;
SELECT * FROM pg_class;
/* ... */
SELECT * FROM pg_locks();
However, I'm not really worried about the name, so I've changed it
to pg_locks() -- a revised patch is attached.
Cheers,
Neil
--
Neil Conway <neilconway@rogers.com>
PGP Key ID: DB3C29FC
Attachment
Neil Conway wrote: > On Tue, Jul 23, 2002 at 05:52:39PM -0400, Bruce Momjian wrote: > > I agree with Peter that it is better to rename it pg_lock*. The show > > isn't needed and it makes it look more like our other system info > > tables. > > Well, it's not a system info table -- it's a built-in function, and I'm > not aware of any naming conventions for those. Personally, I find it a > bit confusing to do: > > SELECT * FROM pg_tables; > SELECT * FROM pg_class; > /* ... */ > SELECT * FROM pg_locks(); > > However, I'm not really worried about the name, so I've changed it > to pg_locks() -- a revised patch is attached. Oh, I see. Should we make it appear as a table rather than a function? Does that make any sense? -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
Neil Conway wrote: > On Tue, Jul 23, 2002 at 05:52:39PM -0400, Bruce Momjian wrote: > > I agree with Peter that it is better to rename it pg_lock*. The show > > isn't needed and it makes it look more like our other system info > > tables. > > Well, it's not a system info table -- it's a built-in function, and I'm > not aware of any naming conventions for those. Personally, I find it a > bit confusing to do: > > SELECT * FROM pg_tables; > SELECT * FROM pg_class; > /* ... */ > SELECT * FROM pg_locks(); > > However, I'm not really worried about the name, so I've changed it > to pg_locks() -- a revised patch is attached. We have pg_stat_activity. Seems pg_locks should behave the same, without the (). Let it show up in \dS. Do you need assistance with this? I know FRS is neat, but logically it seems it should appear as a table like that statistics stuff. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
On Tue, Jul 30, 2002 at 03:11:40PM -0400, Bruce Momjian wrote: > We have pg_stat_activity. Seems pg_locks should behave the same, > without the (). Let it show up in \dS. Do you need assistance with > this? I know FRS is neat, but logically it seems it should appear as a > table like that statistics stuff. So I should just add a view called "pg_locks" that calls the table function? I'll submit a new patch once Joe's anonymous composite type patch gets into CVS. Cheers, Neil -- Neil Conway <neilconway@rogers.com> PGP Key ID: DB3C29FC
Yes, I think that would be the way to go, or look at the stat functions that return tuple sets and use those. That may be a cleaner solution. --------------------------------------------------------------------------- Neil Conway wrote: > On Tue, Jul 30, 2002 at 03:11:40PM -0400, Bruce Momjian wrote: > > We have pg_stat_activity. Seems pg_locks should behave the same, > > without the (). Let it show up in \dS. Do you need assistance with > > this? I know FRS is neat, but logically it seems it should appear as a > > table like that statistics stuff. > > So I should just add a view called "pg_locks" that calls the table > function? > > I'll submit a new patch once Joe's anonymous composite type patch gets > into CVS. > > Cheers, > > Neil > > -- > Neil Conway <neilconway@rogers.com> > PGP Key ID: DB3C29FC > > ---------------------------(end of broadcast)--------------------------- > TIP 5: Have you checked our extensive FAQ? > > http://www.postgresql.org/users-lounge/docs/faq.html > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
On Wed, Jul 31, 2002 at 02:34:19PM -0400, Bruce Momjian wrote: > Yes, I think that would be the way to go, or look at the stat functions > that return tuple sets and use those. That may be a cleaner solution. Why is that cleaner? Cheers, Neil -- Neil Conway <neilconway@rogers.com> PGP Key ID: DB3C29FC
Neil Conway wrote: > On Wed, Jul 31, 2002 at 02:34:19PM -0400, Bruce Momjian wrote: > > Yes, I think that would be the way to go, or look at the stat functions > > that return tuple sets and use those. That may be a cleaner solution. > > Why is that cleaner? Uh, not sure. Comments? -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
On Wed, 2002-07-31 at 14:47, Neil Conway wrote: > On Wed, Jul 31, 2002 at 02:34:19PM -0400, Bruce Momjian wrote: > > Yes, I think that would be the way to go, or look at the stat functions > > that return tuple sets and use those. That may be a cleaner solution. > > Why is that cleaner? Cleaner may be the wrong word. Consistent, follows tradition, more obvious to those who have used PostgreSQL for a number of years... Slightly lower learning curve for newbies. Sub-selects in the from based on functions is not that common. Selecting from tables is :) Lastly, it'll show up in \dS if it's a sudo table. The function is buried in thousands of \df results.
On Wed, Jul 31, 2002 at 03:15:56PM -0400, Rod Taylor wrote: > On Wed, 2002-07-31 at 14:47, Neil Conway wrote: > > On Wed, Jul 31, 2002 at 02:34:19PM -0400, Bruce Momjian wrote: > > > Yes, I think that would be the way to go, or look at the stat functions > > > that return tuple sets and use those. That may be a cleaner solution. [...] > Lastly, it'll show up in \dS if it's a sudo table. The function is > buried in thousands of \df results. I'm confused: I thought that Bruce was suggesting that I change the lock status functions to be similar to the stats functions (i.e. one function for every piece of data and a view that pulls them all together). There's no problem with wrapping a view over the table function -- but IMHO using 5 different functions when one would suffice is just ugly. Cheers, Neil -- Neil Conway <neilconway@rogers.com> PGP Key ID: DB3C29FC
nconway@klamath.dyndns.org (Neil Conway) writes:
> There's no problem with wrapping a view over the table function -- but
> IMHO using 5 different functions when one would suffice is just ugly.
Right. The way the pg_stats functions are implemented is actually
pretty ugly; it was forced by the lack of support for functions
returning tuples in 7.2, but I'm not sure why we'd want to copy it now.
I do agree with providing a view wrapper over the function, for the
reasons Rod mentioned.
regards, tom lane
Neil Conway wrote: > On Wed, Jul 31, 2002 at 03:15:56PM -0400, Rod Taylor wrote: > > On Wed, 2002-07-31 at 14:47, Neil Conway wrote: > > > On Wed, Jul 31, 2002 at 02:34:19PM -0400, Bruce Momjian wrote: > > > > Yes, I think that would be the way to go, or look at the stat functions > > > > that return tuple sets and use those. That may be a cleaner solution. > > [...] > > > Lastly, it'll show up in \dS if it's a sudo table. The function is > > buried in thousands of \df results. > > I'm confused: I thought that Bruce was suggesting that I change the > lock status functions to be similar to the stats functions (i.e. one > function for every piece of data and a view that pulls them all > together). No, sorry, I wasn't suggesting that. Is that how they do it? Yuck. We well do the table function as soon as Joe has it working 100%. So will have virtual tables created two ways, one the pg_stat way and the other the FRS way. Seems fine to me. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026