Re: patch: Allow \dd to show constraint comments

[Resending with gzip'ed patch this time, I think the last attempt got eaten.]

On Mon, Jul 18, 2011 at 11:15 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Mon, Jul 18, 2011 at 10:57 PM, Josh Kupershmidt <schmiddy(at)gmail(dot)com> wrote:
>>>> 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...
>>>
>>> Yeah... or we could add a function pg_object_is_visible(classoid,
>>> objectoid) that would dispatch to the relevant visibility testing
>>> function based on object type.  Not sure if that's too much of a
>>> kludge, but it wouldn't be useful only here: you could probably use it
>>> in combination with pg_locks, for example.
>>
>> Something like that might work, though an easy escape is just leaving
>> the query style of \dd as it was originally (i.e. querying the various
>> catalogs directly, and not using pg_comments): discussed a bit in the
>> recent pg_comments thread w/ Josh Berkus.
>
> That's another approach.  It seems a bit lame, but...

I went ahead and patched up \dd to display its five object types with
its old-style rooting around in catalogs. I played around again with
the idea of having \dd query pg_comments, but gave up when I realized:

 1. We might not be saving much complexity in \dd's query
 2. Taking the is_system column out was probably good for pg_comments,
but exacerbates point 1.), not to mention the visibility testing that
would have to be done somehow.
 3. The "objname" column of pg_comments is intentionally different
than the "Name" column displayed by \dd; the justification for this
was that \dd's "Name" display wasn't actually useful to recreate the
call to COMMENT ON, but this difference in pg_comments would make it
pretty tricky to keep \dd's "Name" the same
 4. I still would like to get rid of \dd entirely, thus it seems less
important whether it uses pg_comments. It's down to five object types
now; I think that triggers, constraints, and rules could feasibly be
incorporated into \d+ output as Robert suggested upthread, and perhaps
operator class & operator family could get their own backslash
commands.

Some fixes:
 * shuffled the query components in describe.c's objectDescription()
so they're alphabetized by object type
 * untabified pg_comments in system_views.sql to match its surroundings
 * the WHERE d.objsubid = 0 was being omitted in one or two spots,
 * the objects with descriptions coming from pg_shdescription, which
does not have the objsubid column, were using NULL::integer instead of
0, as all the other non-column object types should have. This seemed
undesirable, and counter to what the doc page claimed.
 * fixed up psql's documentation and help string for \dd

Updated patch attached, along with a revised SQL script to make
testing easier. I can add this to the next CF.

Note, there is a separate thread[1] with just the psql changes broken
out, if it's helpful to consider the psql changes separately from
pg_comments. I do need to update the patch posted there with this
latest set of changes.

Josh
--
[1] http://archives.postgresql.org/pgsql-hackers/2011-07/msg00459.php