On Fri, Jul 22, 2011 at 12:44 PM, Robert Haas <robertmh...@gmail.com> wrote:
> On Thu, Jul 21, 2011 at 9:17 PM, Josh Kupershmidt <schmi...@gmail.com> wrote:
>> Here's a small patch against branch 8.4 to mention support for COMMENT
>> ON index_name.column_name.
>
> I am not in favor of this - because we'd also need to mention every
> other relkind that can support comments. I think if we want to do
> something here we should change it to say relation_name, and then
> clarify what that means further down. Similarly with the patch for
> master.
>
> Also, if we're going to make a change here, we probably should make
> sure it matches the actual behavior. In master, that's to allow
> comments on columns of tables, views, composite types, and foreign
> tables.
That seems like a good way to document this; patch for master updated.
I avoided mucking with the documentation for COMMENT ON RULE and
COMMENT ON TRIGGER this time; they both say "table" when they really
mean "table or view", but maybe trying to differentiate between
"table", "table_or_view", and "relation" will make things overly
complicated.
>> Also, a patch against master to:
>> * get rid of the bogus "Description" outputs for \d+ sequence_name
>> and \d+ index_name
>
> This part looks OK, but instead of doing a negative test (not-index,
> not-sequence) let's have it do a positive test, for the same types
> comment.c allows.
Right, fixed.
>> And while I'm messing with this, some further nitpicks about psql not
>> addressed by these patches:
>> * The "Storage" column for \d+ sequence_name is correct, I suppose,
>> but repetitive
>
> I'm OK with removing that.
Hrm, would it be better to keep that Storage bit around in some
non-repetitive form, maybe on its own line below the table output?
>> * The "Type" column for \dv+ view_name, \di+ index_name, \ds+
>> sequence_name , etc. seems borderline useless.. shouldn't you know
>> what type you're looking at based on the backslash command you're
>> using?
>
> Not really. You can do something like this, for example:
>
> \dti+
>
> ...to show both indexes and tables.
I see. Didn't know about that trick.
Josh
diff --git a/doc/src/sgml/ref/comment.sgml b/doc/src/sgml/ref/comment.sgml
index ab12614..736907e 100644
*** a/doc/src/sgml/ref/comment.sgml
--- b/doc/src/sgml/ref/comment.sgml
*************** COMMENT ON
*** 26,32 ****
AGGREGATE <replaceable class="PARAMETER">agg_name</replaceable> (<replaceable class="PARAMETER">agg_type</replaceable> [, ...] ) |
CAST (<replaceable>source_type</replaceable> AS <replaceable>target_type</replaceable>) |
COLLATION <replaceable class="PARAMETER">object_name</replaceable> |
! COLUMN <replaceable class="PARAMETER">table_name</replaceable>.<replaceable class="PARAMETER">column_name</replaceable> |
CONSTRAINT <replaceable class="PARAMETER">constraint_name</replaceable> ON <replaceable class="PARAMETER">table_name</replaceable> |
CONVERSION <replaceable class="PARAMETER">object_name</replaceable> |
DATABASE <replaceable class="PARAMETER">object_name</replaceable> |
--- 26,32 ----
AGGREGATE <replaceable class="PARAMETER">agg_name</replaceable> (<replaceable class="PARAMETER">agg_type</replaceable> [, ...] ) |
CAST (<replaceable>source_type</replaceable> AS <replaceable>target_type</replaceable>) |
COLLATION <replaceable class="PARAMETER">object_name</replaceable> |
! COLUMN <replaceable class="PARAMETER">relation_name</replaceable>.<replaceable class="PARAMETER">column_name</replaceable> |
CONSTRAINT <replaceable class="PARAMETER">constraint_name</replaceable> ON <replaceable class="PARAMETER">table_name</replaceable> |
CONVERSION <replaceable class="PARAMETER">object_name</replaceable> |
DATABASE <replaceable class="PARAMETER">object_name</replaceable> |
*************** COMMENT ON
*** 97,105 ****
<variablelist>
<varlistentry>
- <term><replaceable class="parameter">object_name</replaceable></term>
- <term><replaceable class="parameter">table_name.column_name</replaceable></term>
<term><replaceable class="parameter">agg_name</replaceable></term>
<term><replaceable class="parameter">constraint_name</replaceable></term>
<term><replaceable class="parameter">function_name</replaceable></term>
<term><replaceable class="parameter">operator_name</replaceable></term>
--- 97,104 ----
<variablelist>
<varlistentry>
<term><replaceable class="parameter">agg_name</replaceable></term>
+ <term><replaceable class="parameter">object_name</replaceable></term>
<term><replaceable class="parameter">constraint_name</replaceable></term>
<term><replaceable class="parameter">function_name</replaceable></term>
<term><replaceable class="parameter">operator_name</replaceable></term>
*************** COMMENT ON
*** 143,148 ****
--- 142,158 ----
</para>
</listitem>
</varlistentry>
+
+ <varlistentry>
+ <term><replaceable>relation_name.column_name</replaceable></term>
+ <listitem>
+ <para>
+ For comments on columns, the name of the relation and column. Column
+ comments may be used with tables, views, composite types, and
+ foreign tables.
+ </para>
+ </listitem>
+ </varlistentry>
<varlistentry>
<term><replaceable class="parameter">argmode</replaceable></term>
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 682cf8a..dda7097 100644
*** a/src/bin/psql/describe.c
--- b/src/bin/psql/describe.c
*************** describeOneTableDetails(const char *sche
*** 1295,1302 ****
appendPQExpBuffer(&buf, "\n NULL AS attcollation");
if (tableinfo.relkind == 'i')
appendPQExpBuffer(&buf, ",\n pg_catalog.pg_get_indexdef(a.attrelid, a.attnum, TRUE) AS indexdef");
! if (verbose)
! appendPQExpBuffer(&buf, ",\n a.attstorage, pg_catalog.col_description(a.attrelid, a.attnum)");
appendPQExpBuffer(&buf, "\nFROM pg_catalog.pg_attribute a");
appendPQExpBuffer(&buf, "\nWHERE a.attrelid = '%s' AND a.attnum > 0 AND NOT a.attisdropped", oid);
appendPQExpBuffer(&buf, "\nORDER BY a.attnum;");
--- 1295,1311 ----
appendPQExpBuffer(&buf, "\n NULL AS attcollation");
if (tableinfo.relkind == 'i')
appendPQExpBuffer(&buf, ",\n pg_catalog.pg_get_indexdef(a.attrelid, a.attnum, TRUE) AS indexdef");
! if (verbose) {
! appendPQExpBuffer(&buf, ",\n a.attstorage");
! /* In 9.0+, we have column comments for: relations, views, composite
! * types (not handled here), and foreign tables (c.f. CommentObject()
! * in comment.c).
! */
! if (tableinfo.relkind == 'r' || tableinfo.relkind == 'v' ||
! tableinfo.relkind == 'f' || tableinfo.relkind == 'c')
! appendPQExpBuffer(&buf, ", pg_catalog.col_description(a.attrelid, a.attnum)");
! }
!
appendPQExpBuffer(&buf, "\nFROM pg_catalog.pg_attribute a");
appendPQExpBuffer(&buf, "\nWHERE a.attrelid = '%s' AND a.attnum > 0 AND NOT a.attisdropped", oid);
appendPQExpBuffer(&buf, "\nORDER BY a.attnum;");
*************** describeOneTableDetails(const char *sche
*** 1379,1385 ****
if (verbose)
{
headers[cols++] = gettext_noop("Storage");
! headers[cols++] = gettext_noop("Description");
}
printTableInit(&cont, &myopt, title.data, cols, numrows);
--- 1388,1397 ----
if (verbose)
{
headers[cols++] = gettext_noop("Storage");
! /* Column comments, if the relkind supports this feature. */
! if (tableinfo.relkind == 'r' || tableinfo.relkind == 'v' ||
! tableinfo.relkind == 'c' || tableinfo.relkind == 'f')
! headers[cols++] = gettext_noop("Description");
}
printTableInit(&cont, &myopt, title.data, cols, numrows);
*************** describeOneTableDetails(const char *sche
*** 1471,1478 ****
(storage[0] == 'e' ? "external" :
"???")))),
false, false);
! printTableAddCell(&cont, PQgetvalue(res, i, firstvcol + 1),
! false, false);
}
}
--- 1483,1493 ----
(storage[0] == 'e' ? "external" :
"???")))),
false, false);
! /* Column comments, if the relkind supports this feature. */
! if (tableinfo.relkind == 'r' || tableinfo.relkind == 'v' ||
! tableinfo.relkind == 'c' || tableinfo.relkind == 'f')
! printTableAddCell(&cont, PQgetvalue(res, i, firstvcol + 1),
! false, false);
}
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
