Re: patch: Allow \dd to show constraint comments - Mailing list pgsql-hackers
From | Josh Kupershmidt |
---|---|
Subject | Re: patch: Allow \dd to show constraint comments |
Date | |
Msg-id | CAK3UJREoOJ5Tba9MLoP1yb=E=tk3meJc4N2hekFEGjRn103HMA@mail.gmail.com Whole thread Raw |
In response to | Re: patch: Allow \dd to show constraint comments (Josh Kupershmidt <schmiddy@gmail.com>) |
Responses |
Re: patch: Allow \dd to show constraint comments
|
List | pgsql-hackers |
[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@gmail.com> wrote: > On Mon, Jul 18, 2011 at 10:57 PM, Josh Kupershmidt <schmiddy@gmail.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
Attachment
pgsql-hackers by date: