Thread

1. pg_comments

   Robert Haas <robertmhaas@gmail.com> — 2010-09-20T04:53:39Z

   The psql \dd command has a couple of infelicities.
   
   1. It doesn't actually list comments on all of the object types to
   which they can be applied using the COMMENT command.
   2. It also doesn't list comments on access methods, which have
   comments but are not supported by the COMMENT command.
   3. It doesn't even list comments on all of the object types which the
   psql documentation claims it does.
   4. It chooses to print out both the "name" and "object" columns in a
   format which is not 100% compatible with the COMMENT command, so that
   you can't necessarily use the output of \dd to construct valid input
   to COMMENT.
   5. The SQL query used to generate the output it does produce is 75
   lines long, meaning that it's really entertaining if you need, for
   some reason, to edit that query.
   
   In view of the foregoing problems, I'd like to propose adding a new
   system view, tentatively called pg_comments, which lists all of the
   comments for everything in the system in such a way that it's
   reasonably possible to do further filtering out the output in ways
   that you might care about; and which also gives objects the names and
   types in a format that matches what the COMMENT command will accept as
   input.  Patch attached.  I haven't yet written the documentation for
   the view or adjusted src/bin/psql/describe.c to do anything useful
   with it, just so that I won't waste any more time on this if it gets
   shot down.  But for the record, it took me something like three hours
   to write and test this view, which I think is an excellent argument
   for why we need it.
   
   Supposing no major objections, there are a few things to think about
   if we wish to have psql use this:
   
   A. The obvious thing to do seems to be to retain the existing code for
   server versions < 9.1 and to use pg_comments for >= 9.1.  I would be
   inclined not to bother fixing the code for pre-9.1 servers to display
   comments on everything (a 9.1 psql against a 9.0 or prior server will
   be no worse than a 9.0 psql against the same server; it just won't be
   any better).
   
   B. The existing code localizes the contents of the "object" column.
   This is arguably a misfeature if you are about (4), but if we want to
   keep the existing behavior I'm not quite sure what the best way to do
   that is.
   
   C. It's not so obvious which comments should be displayed with \dd vs.
   \ddS.  In particular, comments on toast schemas have the same problem
   recently discussed with \dn, and there is a similar issue with
   tablespaces.  Generally, it's not obvious what to do for objects that
   don't live in schemas - access methods, for example, are arguably
   always system objects.  But... that's arguable.
   
   D. Fixing (4) with respect to object names implies listing argument
   types for functions and operators, which makes the display output
   quite wide when using \ddS.  I am inclined to say that's just the cost
   of making the output accurate.
   
   There may be other issues I haven't noticed yet, too.
   
   Incidentally, if you're wondering what prompted this patch, I was
   reviewing KaiGai Kohei's patch to add security label support and
   noticed its complete lack of psql support.  I'm actually not really
   sure that there's any compelling reason to provide psql support,
   considering that we've gotten to the point where any backslash command
   is almost bound to be something not terribly mnemonic, and because
   there are likely to be either no security labels at all or so many
   that a command that just dumps them ALL out in bulk is all but
   useless.  But we at least need to provide a suitable system view,
   because the catalog structure used by these catalogs that can handle
   SQL objects of any type is pretty obnoxious for user querying (though,
   of course, it's pretty neat as an internal format).
   
   --
   Robert Haas
   EnterpriseDB: http://www.enterprisedb.com
   The Enterprise Postgres Company
   

2. Re: pg_comments

   Tom Lane <tgl@sss.pgh.pa.us> — 2010-09-20T05:07:16Z

   Robert Haas <robertmhaas@gmail.com> writes:
   > In view of the foregoing problems, I'd like to propose adding a new
   > system view, tentatively called pg_comments, which lists all of the
   > comments for everything in the system in such a way that it's
   > reasonably possible to do further filtering out the output in ways
   > that you might care about; and which also gives objects the names and
   > types in a format that matches what the COMMENT command will accept as
   > input.  Patch attached.
   
   Unless you propose to break psql's hard-won backwards compatibility,
   this isn't going to accomplish anything towards making describe.c
   simpler or shorter.  Also, it seems to me that what you've mostly done
   is to move complexity from describe.c (where the query can be fixed
   easily if it's found to be broken) to system_views.sql (where it cannot
   be changed without an initdb).
   
   How about improving the query in-place in describe.c instead?
   
   			regards, tom lane
   
   

3. Re: pg_comments

   Robert Haas <robertmhaas@gmail.com> — 2010-09-20T11:45:28Z

   On Mon, Sep 20, 2010 at 1:07 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
   > Robert Haas <robertmhaas@gmail.com> writes:
   >> In view of the foregoing problems, I'd like to propose adding a new
   >> system view, tentatively called pg_comments, which lists all of the
   >> comments for everything in the system in such a way that it's
   >> reasonably possible to do further filtering out the output in ways
   >> that you might care about; and which also gives objects the names and
   >> types in a format that matches what the COMMENT command will accept as
   >> input.  Patch attached.
   >
   > Unless you propose to break psql's hard-won backwards compatibility,
   > this isn't going to accomplish anything towards making describe.c
   > simpler or shorter. Also, it seems to me that what you've mostly done
   > is to move complexity from describe.c (where the query can be fixed
   > easily if it's found to be broken) to system_views.sql (where it cannot
   > be changed without an initdb).
   
   Those are legitimate gripes, but...
   
   > How about improving the query in-place in describe.c instead?
   
   ...I still don't care much for this option.  It doesn't do anything to
   easy the difficulty of ad-hoc queries, which I think is important (and
   seems likely to be even more important for security labels - because
   people who use that feature at all are going to label the heck out of
   everything, whereas comments are never strictly necessary), and it
   isn't useful for clients other than psql.  Most of this code hasn't
   been touched since 2002, despite numerous, relevant changes since
   then.  You could take as support for your position that we need the
   ability to fix future bugs without initdb, but my reading of it is
   that that code is just too awful to be easily maintained and so no one
   has bothered.
   
   (It also supports my previous contention that we need a way to make
   minor system catalog updates without forcing initdb, but that's a
   problem for another day.)
   
   -- 
   Robert Haas
   EnterpriseDB: http://www.enterprisedb.com
   The Enterprise Postgres Company
   
   

4. Re: pg_comments

   Kouhei Kaigai <kaigai@ak.jp.nec.com> — 2010-09-24T06:18:57Z

   Robert,
   
   I noticed a problem at the definition of the view.
   
   	:
   +UNION ALL
   +SELECT
   +	d.objoid, d.classoid, d.objsubid,
   +	'large object'::text AS objtype,
   +	NULL::oid AS objnamespace,
   +	d.objoid::text AS objname,
   +	d.description
   +FROM
   +	pg_description d
   +	JOIN pg_largeobject_metadata lom ON d.objoid = lom.oid
   +WHERE
   +	d.classoid = (SELECT oid FROM pg_class WHERE relname = 'pg_largeobject')
   +	AND d.objsubid = 0
   +UNION ALL
   	:
   
   If and when user create a table named 'pg_largeobject' on anywhere except
   for the 'pg_catalog' schema, the (SELECT oid FROM pg_class WHERE relname =
   'pg_largeobject') may not return 2613.
   
   It seems to me the query should be fixed up as follows:
   
   	:
   WHERE
   	d.classoid = (SELECT oid FROM pg_class WHERE relname = 'pg_largeobject'
   		      AND relnamespace = (SELECT oid FROM pg_namespace
   					  WHERE nspname = 'pg_catalog'))
   	:
   
   Thanks,
   
   (2010/09/20 13:53), Robert Haas wrote:
   > The psql \dd command has a couple of infelicities.
   > 
   > 1. It doesn't actually list comments on all of the object types to
   > which they can be applied using the COMMENT command.
   > 2. It also doesn't list comments on access methods, which have
   > comments but are not supported by the COMMENT command.
   > 3. It doesn't even list comments on all of the object types which the
   > psql documentation claims it does.
   > 4. It chooses to print out both the "name" and "object" columns in a
   > format which is not 100% compatible with the COMMENT command, so that
   > you can't necessarily use the output of \dd to construct valid input
   > to COMMENT.
   > 5. The SQL query used to generate the output it does produce is 75
   > lines long, meaning that it's really entertaining if you need, for
   > some reason, to edit that query.
   > 
   > In view of the foregoing problems, I'd like to propose adding a new
   > system view, tentatively called pg_comments, which lists all of the
   > comments for everything in the system in such a way that it's
   > reasonably possible to do further filtering out the output in ways
   > that you might care about; and which also gives objects the names and
   > types in a format that matches what the COMMENT command will accept as
   > input.  Patch attached.  I haven't yet written the documentation for
   > the view or adjusted src/bin/psql/describe.c to do anything useful
   > with it, just so that I won't waste any more time on this if it gets
   > shot down.  But for the record, it took me something like three hours
   > to write and test this view, which I think is an excellent argument
   > for why we need it.
   > 
   > Supposing no major objections, there are a few things to think about
   > if we wish to have psql use this:
   > 
   > A. The obvious thing to do seems to be to retain the existing code for
   > server versions<  9.1 and to use pg_comments for>= 9.1.  I would be
   > inclined not to bother fixing the code for pre-9.1 servers to display
   > comments on everything (a 9.1 psql against a 9.0 or prior server will
   > be no worse than a 9.0 psql against the same server; it just won't be
   > any better).
   > 
   > B. The existing code localizes the contents of the "object" column.
   > This is arguably a misfeature if you are about (4), but if we want to
   > keep the existing behavior I'm not quite sure what the best way to do
   > that is.
   > 
   > C. It's not so obvious which comments should be displayed with \dd vs.
   > \ddS.  In particular, comments on toast schemas have the same problem
   > recently discussed with \dn, and there is a similar issue with
   > tablespaces.  Generally, it's not obvious what to do for objects that
   > don't live in schemas - access methods, for example, are arguably
   > always system objects.  But... that's arguable.
   > 
   > D. Fixing (4) with respect to object names implies listing argument
   > types for functions and operators, which makes the display output
   > quite wide when using \ddS.  I am inclined to say that's just the cost
   > of making the output accurate.
   > 
   > There may be other issues I haven't noticed yet, too.
   > 
   > Incidentally, if you're wondering what prompted this patch, I was
   > reviewing KaiGai Kohei's patch to add security label support and
   > noticed its complete lack of psql support.  I'm actually not really
   > sure that there's any compelling reason to provide psql support,
   > considering that we've gotten to the point where any backslash command
   > is almost bound to be something not terribly mnemonic, and because
   > there are likely to be either no security labels at all or so many
   > that a command that just dumps them ALL out in bulk is all but
   > useless.  But we at least need to provide a suitable system view,
   > because the catalog structure used by these catalogs that can handle
   > SQL objects of any type is pretty obnoxious for user querying (though,
   > of course, it's pretty neat as an internal format).
   > 
   > --
   > Robert Haas
   > EnterpriseDB: http://www.enterprisedb.com
   > The Enterprise Postgres Company
   > 
   > 
   > 
   > 
   
   
   -- 
   KaiGai Kohei <kaigai@ak.jp.nec.com>
   
   

5. Re: pg_comments

   Robert Haas <robertmhaas@gmail.com> — 2010-09-24T11:58:36Z

   2010/9/24 KaiGai Kohei <kaigai@ak.jp.nec.com>:
   > If and when user create a table named 'pg_largeobject' on anywhere except
   > for the 'pg_catalog' schema, the (SELECT oid FROM pg_class WHERE relname =
   > 'pg_largeobject') may not return 2613.
   
   Oh, dear, how embarassing.  Perhaps it should be written as:
   
   d.classoid = 'pg_catalog.pg_largeobject'::regclass.
   
   -- 
   Robert Haas
   EnterpriseDB: http://www.enterprisedb.com
   The Enterprise Postgres Company
   
   

6. Re: pg_comments

   Tom Lane <tgl@sss.pgh.pa.us> — 2010-09-24T13:41:14Z

   KaiGai Kohei <kaigai@ak.jp.nec.com> writes:
   > It seems to me the query should be fixed up as follows:
   
   > 	:
   > WHERE
   > 	d.classoid = (SELECT oid FROM pg_class WHERE relname = 'pg_largeobject'
   > 		      AND relnamespace = (SELECT oid FROM pg_namespace
   > 					  WHERE nspname = 'pg_catalog'))
   > 	:
   
   Actually, the preferred way to spell that sort of thing is
   
   	WHERE
   		d.classoid = 'pg_catalog.pg_largeobject'::regclass
   
   which is not only shorter but orders of magnitude more efficient.
   
   			regards, tom lane