Thread

Re: patch: Allow \dd to show constraint comments

Josh Kupershmidt <schmiddy@gmail.com> — 2011-07-03T00:37:52Z

On Sun, Jun 19, 2011 at 11:59 AM, Josh Kupershmidt <schmiddy@gmail.com> wrote:
> I think we still need to handle my "Still TODO" concerns noted
> upthread. I don't have a lot of time this weekend due to a family
> event, but I was mulling over putting in a "is_system" boolean column
> into pg_comments and then fixing psql's \dd[S] to use pg_comments. But
> I am of course open to other suggestions.

I went ahead and did it this way, though I wasn't 100% sure about some
of the guesses for the "is_system" column I made. I assumed the
following types should have is_system = true: casts, roles, databases,
access methods, tablespaces. I assumed is_system = false for large
objects. For the rest, I just used whether the schema name of the
object was 'pg_catalog' or 'information_schema'.

With the is_system column in place, I was able to make psql's \dd
actually use pg_comments.

I had to tweak the pg_proc.h entry for pg_opfamily_is_visible to play
nice with the recent "transform function" change to that file.

Issues still to be resolved:

1.) For now, I'm just ignoring the issue of visibility checks; I
didn't see a simple way to support these checks \dd was doing:

    processSQLNamePattern(pset.db, &buf, pattern, true, false,
                          "n.nspname", "p.proname", NULL,
                          "pg_catalog.pg_function_is_visible(p.oid)");

I'm a bit leery of adding an "is_visible" column into pg_comments, but
I'm not sure I have a feasible workaround if we do want to keep this
visibility check. Maybe a big CASE statement for the last argument of
processSQLNamePattern() would work...

2.) Since we're mucking with \dd, I think now might be a good time to
re-examine what types of objects are displayed by \dd. I suggest we
explicitly advertise \dd as being only for objects which have comments
_not_ displayed by the object's individual backslash command.
Currently, the comment for objectDescription() claims

 * Note: This only lists things that actually have a description. For
complete
 * lists of things, there are other \d? commands.

but this claim is bogus; from the object type breakdown in my second
post on this thread, the de facto purpose of \dd is really to show
comments not displayed elsewhere.

I'd like to adjust what object types should have comments displayed in
their respective backslash commands instead. I went ahead and removed
the "aggregate" type from \dd, since those comments are already
displayed by \da, but I'd like to tweak things a bit further.

For instance, \dL, \dew, and  \des could be improved to display
comments for LANGUAGE, FOREIGN DATA WRAPPER, and FOREIGN DATA SERVER
respectively. Etc.

3.) I haven't tried to fix up the indentation from my whacking around
in describe.c yet. The view definition in system_views.sql might need
to be re-formatted as well to match its surroundings.

An updated patch is attached; I've marked it WIP since there are the
listed issues outstanding. I'd appreciated feedback particularly on
the way forward for 1.) and 2.), and whether my guesses for the
"is_system" column are OK.

Josh