From b7d67954456f15762c04e5269b64adc88dcd0860 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Tue, 22 Dec 2009 23:54:17 +0000
Subject: [PATCH] Disallow comments on columns of relation types other than
tables, views, and composite types, which are the only relkinds for which
pg_dump support exists for dumping column comments. There is no obvious
usefulness for comments on columns of sequences or toast tables; and while
comments on index columns might have some value, it's not worth the risk of
compatibility problems due to possible changes in the algorithm for assigning
names to index columns. Per discussion.
In consequence, remove now-dead code for copying such comments in CREATE TABLE
LIKE.
---
src/backend/commands/comment.c | 18 +++++++++-
src/backend/parser/parse_utilcmd.c | 49 ++++-----------------------
src/test/regress/expected/inherit.out | 10 ++----
src/test/regress/sql/inherit.sql | 2 --
4 files changed, 26 insertions(+), 53 deletions(-)
diff --git a/src/backend/commands/comment.c b/src/backend/commands/comment.c
index 39cc1debc1e..59459fab0d5 100644
--- a/src/backend/commands/comment.c
+++ b/src/backend/commands/comment.c
@@ -7,7 +7,7 @@
* Copyright (c) 1996-2009, PostgreSQL Global Development Group
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/commands/comment.c,v 1.110 2009/12/21 01:34:11 rhaas Exp $
+ * $PostgreSQL: pgsql/src/backend/commands/comment.c,v 1.111 2009/12/22 23:54:17 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -627,6 +627,22 @@ CommentAttribute(List *qualname, char *comment)
aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
RelationGetRelationName(relation));
+ /*
+ * Allow comments only on columns of tables, views, and composite types
+ * (which are the only relkinds for which pg_dump will dump per-column
+ * comments). In particular we wish to disallow comments on index
+ * columns, because the naming of an index's columns may change across
+ * PG versions, so dumping per-column comments could create reload
+ * failures.
+ */
+ if (relation->rd_rel->relkind != RELKIND_RELATION &&
+ relation->rd_rel->relkind != RELKIND_VIEW &&
+ relation->rd_rel->relkind != RELKIND_COMPOSITE_TYPE)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("\"%s\" is not a table, view, or composite type",
+ RelationGetRelationName(relation))));
+
/* Now, fetch the attribute number from the system cache */
attnum = get_attnum(RelationGetRelid(relation), attrname);
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 89d81bff624..75c8d863dcd 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -19,7 +19,7 @@
* Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
- * $PostgreSQL: pgsql/src/backend/parser/parse_utilcmd.c,v 2.33 2009/12/20 18:28:14 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/parser/parse_utilcmd.c,v 2.34 2009/12/22 23:54:17 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -745,14 +745,12 @@ transformInhRelation(ParseState *pstate, CreateStmtContext *cxt,
/* Copy comment on index */
if (inhRelation->options & CREATE_TABLE_LIKE_COMMENTS)
{
- Form_pg_attribute *attrs;
- CommentStmt *stmt;
- int colno;
-
comment = GetComment(parent_index_oid, RelationRelationId, 0);
-
+
if (comment != NULL)
{
+ CommentStmt *stmt;
+
/*
* We have to assign the index a name now, so that we
* can reference it in CommentStmt.
@@ -771,41 +769,6 @@ transformInhRelation(ParseState *pstate, CreateStmtContext *cxt,
cxt->alist = lappend(cxt->alist, stmt);
}
-
- /* Copy comments on index's columns */
- attrs = RelationGetDescr(parent_index)->attrs;
- for (colno = 1;
- colno <= RelationGetNumberOfAttributes(parent_index);
- colno++)
- {
- char *attname;
-
- comment = GetComment(parent_index_oid, RelationRelationId,
- colno);
- if (comment == NULL)
- continue;
-
- /*
- * We have to assign the index a name now, so that we
- * can reference it in CommentStmt.
- */
- if (index_stmt->idxname == NULL)
- index_stmt->idxname = chooseIndexName(cxt->relation,
- index_stmt);
-
- attname = NameStr(attrs[colno - 1]->attname);
-
- stmt = makeNode(CommentStmt);
- stmt->objtype = OBJECT_COLUMN;
- stmt->objname =
- list_make3(makeString(cxt->relation->schemaname),
- makeString(index_stmt->idxname),
- makeString(attname));
- stmt->objargs = NIL;
- stmt->comment = comment;
-
- cxt->alist = lappend(cxt->alist, stmt);
- }
}
/* Save it in the inh_indexes list for the time being */
@@ -832,12 +795,12 @@ static char *
chooseIndexName(const RangeVar *relation, IndexStmt *index_stmt)
{
Oid namespaceId;
-
+
namespaceId = RangeVarGetCreationNamespace(relation);
if (index_stmt->primary)
{
/* no need for column list with pkey */
- return ChooseRelationName(relation->relname, NULL,
+ return ChooseRelationName(relation->relname, NULL,
"pkey", namespaceId);
}
else if (index_stmt->excludeOpNames != NIL)
diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index aebadd2c9c8..aa1eb4a4ddb 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -931,8 +931,6 @@ COMMENT ON COLUMN t1.b IS 'B';
COMMENT ON CONSTRAINT t1_a_check ON t1 IS 't1_a_check';
COMMENT ON INDEX t1_pkey IS 'index pkey';
COMMENT ON INDEX t1_b_key IS 'index b_key';
-COMMENT ON COLUMN t1_pkey.a IS 'index column pkey.a';
-COMMENT ON COLUMN t1_fnidx.pg_expression_1 IS 'index column fnidx';
ALTER TABLE t1 ALTER COLUMN a SET STORAGE MAIN;
CREATE TABLE t2 (c text);
ALTER TABLE t2 ALTER COLUMN c SET STORAGE EXTERNAL;
@@ -1040,13 +1038,11 @@ Check constraints:
Has OIDs: no
SELECT c.relname, objsubid, description FROM pg_description, pg_index i, pg_class c WHERE classoid = 'pg_class'::regclass AND objoid = i.indexrelid AND c.oid = i.indexrelid AND i.indrelid = 't_all'::regclass ORDER BY c.relname, objsubid;
- relname | objsubid | description
--------------+----------+---------------------
+ relname | objsubid | description
+-------------+----------+-------------
t_all_b_key | 0 | index b_key
- t_all_key | 1 | index column fnidx
t_all_pkey | 0 | index pkey
- t_all_pkey | 1 | index column pkey.a
-(4 rows)
+(2 rows)
CREATE TABLE inh_error1 () INHERITS (t1, t4);
NOTICE: merging multiple inherited definitions of column "a"
diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql
index 6f1a492676a..192e8604d6d 100644
--- a/src/test/regress/sql/inherit.sql
+++ b/src/test/regress/sql/inherit.sql
@@ -297,8 +297,6 @@ COMMENT ON COLUMN t1.b IS 'B';
COMMENT ON CONSTRAINT t1_a_check ON t1 IS 't1_a_check';
COMMENT ON INDEX t1_pkey IS 'index pkey';
COMMENT ON INDEX t1_b_key IS 'index b_key';
-COMMENT ON COLUMN t1_pkey.a IS 'index column pkey.a';
-COMMENT ON COLUMN t1_fnidx.pg_expression_1 IS 'index column fnidx';
ALTER TABLE t1 ALTER COLUMN a SET STORAGE MAIN;
CREATE TABLE t2 (c text);
--
GitLab