Re: patch: Allow \dd to show constraint comments

On Thu, May 19, 2011 at 9:36 PM, Josh Kupershmidt <schmiddy(at)gmail(dot)com> wrote:
> On Thu, May 19, 2011 at 10:26 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> At the risk of opening a can of worms, if we're going to fix \dd,
>> shouldn't we fix it completely, and include comments on ALL the object
>> types that can have them?  IIRC it's missing a bunch, not just
>> constraints.
>
> You opened this can up, so I hope you like worms ;-) Let's break down
> the objects that the COMMENT docs say one can comment on. [Disclaimer:
> It's likely I've made some mistakes in this list, data was gleaned
> mostly from perusing describe.c]
>
> 1.) Comments displayed by \dd:
>
>  TABLE
>  AGGREGATE
>  OPERATOR
>  RULE
>  FUNCTION
>  INDEX
>  SEQUENCE
>  TRIGGER
>  TYPE
>  VIEW
>
> 2.) Comments displayed in the backslash commands for the object:
>
>  AGGREGATE                     \da
>  COLUMN                        \d+ tablename
>  COLLATION                     \dO
>  DATABASE                      \l+
>  EXTENSION                     \dx
>  FUNCTION                      \df+
>  FOREIGN TABLE                 \d+ tablename (I think?)
>  LARGE OBJECT                  \dl
>  ROLE                          \dg+
>  SCHEMA                        \dn+
>  TABLESPACE                    \db+
>  TYPE                          \dT
>  TEXT SEARCH CONFIGURATION     \dF
>  TEXT SEARCH DICTIONARY        \dFd
>  TEXT SEARCH PARSER            \dFp
>  TEXT SEARCH TEMPLATE          \dFt
>
> 3.) Objects for which we don't display the comments anywhere:
>  CAST
>  CONSTRAINT (addressed by this patch)
>  CONVERSION
>  DOMAIN
>  PROCEDURAL LANGUAGE
>
> 4.) My eyes are starting to glaze over, and I'm too lazy to figure out
> how or if comments for these objects are handled:
>  FOREIGN DATA WRAPPER
>  OPERATOR CLASS
>  OPERATOR FAMILY
>  SERVER
>
>> Another thought is that I wonder if it's really useful to have a
>> backslash commands that dumps out comments on many different object
>> types.
>
> ISTM that \dd is best suited as a command to show the comments for
> objects for which we don't have an individual backslash command, or
> objects for which it's not practical to show the comment in the
> backslash command.
>
>> In some cases, e.g. \db+, we include the description for the
>> object in the output of the backslash command that lists objects just
>> of that type, which seems like a better design.
>
> I agree that's the way to go for objects where such display is
> feasible (the backslash command produces columnar output, and we can
> just stick in a "comment" column).
>
> I wouldn't want to try to pollute, say, \d+ with comments about
> tables, rules, triggers, etc. But for the few objects displayed by
> both \dd and the object's respective backslash command (aggregates,
> types, and functions), I would be in favor of ripping out the \dd
> display.
>
>> Of course we have no
>> backslash command for constraints anyway....
>
> Precisely, and I think there's a solid argument for putting
> constraints into bucket 1 above, as this patch does, since there's no
> good room to display constraint comments inside \d+, and there's no
> backslash command specifically for constraints.
>
> I was kind of hoping to avoid dealing with this can of worms with this
> simple patch, which by itself seems uncontroversial. If there's
> consensus that \dd and the other backslash commands need further
> reworking, I can probably devote a little more time to this. But let's
> not have the perfect be the enemy of the good.

Patch applies clean, does what it is supposed to do, and matches other
conventions in describe.c Passing to committer. pg_comments may be
a better way to go, but that is a problem for another day...

merlin