This has been applied by Tom.
---------------------------------------------------------------------------
Petr Jelinek wrote:
> Tom Lane wrote:
> > Hm, I'm not entirely sure if you got the point or not. For either
> > relations or composite types, there is both a pg_class entry and a
> > pg_type entry, and their names *must* stay in sync. We could allow
> > people to rename both entries using either ALTER TABLE or ALTER TYPE,
> > but the general consensus seems to be that ALTER TYPE should be used
> > for composite types and ALTER TABLE for tables/views/etc. The fact
> > that there's a pg_class entry for a composite type is really an
> > implementation detail that would best not be exposed to users, so
> > enforcing the use of the appropriate command seems reasonable to me.
> >
> > regards, tom lane
> >
> Yes, that's exactly what I meant (my language skills are not best).
>
> Anyway, I am sending second version of the patch. Changes are:
> - renamed TypeRename function to RenameTypeInternal and changed its
> header comment
> - throw error when using ALTER TYPE to rename rowtype
> - split function renamerel to RenameRelation and RenameRelationInternal
> where RenameRelation does permission checks and stuff and also checks if
> it's not used for composite types and RenameRelationInternal does the
> actual rename. And I also did a little cleanup in those functions
> (removed unused code and changed some hardcoded relkind types to globaly
> predefined constants)
> - RenameType now calls RenameRelationInternal for composite types
> (which calls RenameTypeInternal automatically)
>
> Any other comments ?
>
> --
> Regards
> Petr Jelinek (PJMODOS)
>
> Index: doc/src/sgml/ref/alter_type.sgml
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/doc/src/sgml/ref/alter_type.sgml,v
> retrieving revision 1.4
> diff -c -r1.4 alter_type.sgml
> *** doc/src/sgml/ref/alter_type.sgml 16 Sep 2006 00:30:16 -0000 1.4
> --- doc/src/sgml/ref/alter_type.sgml 29 Sep 2007 05:43:14 -0000
> ***************
> *** 26,31 ****
> --- 26,32 ----
> <synopsis>
> ALTER TYPE <replaceable class="PARAMETER">name</replaceable> OWNER TO
> <replaceable class="PARAMETER">new_owner</replaceable>
> ALTER TYPE <replaceable class="PARAMETER">name</replaceable> SET SCHEMA
> <replaceable class="PARAMETER">new_schema</replaceable>
> + ALTER TYPE <replaceable class="PARAMETER">name</replaceable> RNAME TO
> <replaceable class="PARAMETER">new_name</replaceable>
> </synopsis>
> </refsynopsisdiv>
>
> ***************
> *** 83,88 ****
> --- 84,98 ----
> </listitem>
> </varlistentry>
>
> + <varlistentry>
> + <term><replaceable class="PARAMETER">new_name</replaceable></term>
> + <listitem>
> + <para>
> + The new name for the type.
> + </para>
> + </listitem>
> + </varlistentry>
> +
> </variablelist>
> </para>
> </refsect1>
> Index: src/backend/catalog/pg_type.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/backend/catalog/pg_type.c,v
> retrieving revision 1.113
> diff -c -r1.113 pg_type.c
> *** src/backend/catalog/pg_type.c 12 May 2007 00:54:59 -0000 1.113
> --- src/backend/catalog/pg_type.c 30 Sep 2007 04:20:03 -0000
> ***************
> *** 552,566 ****
> }
>
> /*
> ! * TypeRename
> * This renames a type, as well as any associated array type.
> *
> ! * Note: this isn't intended to be a user-exposed function; it doesn't check
> ! * permissions etc. (Perhaps TypeRenameInternal would be a better name.)
> ! * Currently this is only used for renaming table rowtypes.
> */
> void
> ! TypeRename(Oid typeOid, const char *newTypeName, Oid typeNamespace)
> {
> Relation pg_type_desc;
> HeapTuple tuple;
> --- 552,567 ----
> }
>
> /*
> ! * RenameTypeInternal
> * This renames a type, as well as any associated array type.
> *
> ! * Caller must have already checked privileges.
> ! *
> ! * Currently this is used for renaming table rowtypes and for
> ! * ALTER TYPE RENAME TO command.
> */
> void
> ! RenameTypeInternal(Oid typeOid, const char *newTypeName, Oid typeNamespace)
> {
> Relation pg_type_desc;
> HeapTuple tuple;
> ***************
> *** 606,612 ****
> {
> char *arrname = makeArrayTypeName(newTypeName, typeNamespace);
>
> ! TypeRename(arrayOid, arrname, typeNamespace);
> pfree(arrname);
> }
> }
> --- 607,613 ----
> {
> char *arrname = makeArrayTypeName(newTypeName, typeNamespace);
>
> ! RenameTypeInternal(arrayOid, arrname, typeNamespace);
> pfree(arrname);
> }
> }
> ***************
> *** 706,712 ****
> newname = makeArrayTypeName(typeName, typeNamespace);
>
> /* Apply the rename */
> ! TypeRename(typeOid, newname, typeNamespace);
>
> /*
> * We must bump the command counter so that any subsequent use of
> --- 707,713 ----
> newname = makeArrayTypeName(typeName, typeNamespace);
>
> /* Apply the rename */
> ! RenameTypeInternal(typeOid, newname, typeNamespace);
>
> /*
> * We must bump the command counter so that any subsequent use of
> Index: src/backend/commands/alter.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/backend/commands/alter.c,v
> retrieving revision 1.25
> diff -c -r1.25 alter.c
> *** src/backend/commands/alter.c 21 Aug 2007 01:11:14 -0000 1.25
> --- src/backend/commands/alter.c 30 Sep 2007 03:53:05 -0000
> ***************
> *** 117,123 ****
>
> aclcheck_error(aclresult, ACL_KIND_NAMESPACE,
>
> get_namespace_name(namespaceId));
>
> ! renamerel(relid,
> stmt->newname, stmt->renameType);
> break;
> }
> case OBJECT_COLUMN:
> --- 117,123 ----
>
> aclcheck_error(aclresult, ACL_KIND_NAMESPACE,
>
> get_namespace_name(namespaceId));
>
> ! RenameRelation(relid,
> stmt->newname, stmt->renameType);
> break;
> }
> case OBJECT_COLUMN:
> ***************
> *** 154,159 ****
> --- 154,164 ----
> RenameTSConfiguration(stmt->object, stmt->newname);
> break;
>
> + case OBJECT_TYPE:
> + case OBJECT_DOMAIN:
> + RenameType(stmt->object, stmt->newname);
> + break;
> +
> default:
> elog(ERROR, "unrecognized rename stmt type: %d",
> (int) stmt->renameType);
> Index: src/backend/commands/tablecmds.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/backend/commands/tablecmds.c,v
> retrieving revision 1.233
> diff -c -r1.233 tablecmds.c
> *** src/backend/commands/tablecmds.c 29 Sep 2007 17:18:58 -0000 1.233
> --- src/backend/commands/tablecmds.c 30 Sep 2007 03:55:31 -0000
> ***************
> *** 1612,1637 ****
> relation_close(targetrelation, NoLock); /* close rel but keep
> lock */
> }
>
> /*
> ! * renamerel - change the name of a relation
> ! *
> ! * XXX - When renaming sequences, we don't bother to modify the
> ! * sequence name that is stored within the sequence
> itself
> ! * (this would cause problems with MVCC). In the future,
> ! * the sequence name should probably be removed from the
> ! * sequence, AFAIK there's no need for it to be there.
> */
> void
> ! renamerel(Oid myrelid, const char *newrelname, ObjectType reltype)
> {
> Relation targetrelation;
> - Relation relrelation; /* for RELATION relation */
> - HeapTuple reltup;
> - Form_pg_class relform;
> Oid namespaceId;
> - char *oldrelname;
> char relkind;
> - bool relhastriggers;
>
> /*
> * Grab an exclusive lock on the target table, index, sequence or
> --- 1612,1627 ----
> relation_close(targetrelation, NoLock); /* close rel but keep
> lock */
> }
>
> +
> /*
> ! * Execute ALTER TABLE/VIEW/SEQUENCE RENAME
> */
> void
> ! RenameRelation(Oid myrelid, const char *newrelname, ObjectType reltype)
> {
> Relation targetrelation;
> Oid namespaceId;
> char relkind;
>
> /*
> * Grab an exclusive lock on the target table, index, sequence or
> ***************
> *** 1639,1645 ****
> */
> targetrelation = relation_open(myrelid, AccessExclusiveLock);
>
> - oldrelname = pstrdup(RelationGetRelationName(targetrelation));
> namespaceId = RelationGetNamespace(targetrelation);
>
> if (!allowSystemTableMods && IsSystemRelation(targetrelation))
> --- 1629,1634 ----
> ***************
> *** 1654,1672 ****
> * view.
> */
> relkind = targetrelation->rd_rel->relkind;
> ! if (reltype == OBJECT_SEQUENCE && relkind != 'S')
> ereport(ERROR,
> (errcode(ERRCODE_WRONG_OBJECT_TYPE),
> errmsg("\"%s\" is not a sequence",
>
> RelationGetRelationName(targetrelation))));
>
> ! if (reltype == OBJECT_VIEW && relkind != 'v')
> ereport(ERROR,
> (errcode(ERRCODE_WRONG_OBJECT_TYPE),
> errmsg("\"%s\" is not a view",
>
> RelationGetRelationName(targetrelation))));
>
> ! relhastriggers = (targetrelation->rd_rel->reltriggers > 0);
>
> /*
> * Find relation's pg_class tuple, and make sure newrelname isn't in
> use.
> --- 1643,1702 ----
> * view.
> */
> relkind = targetrelation->rd_rel->relkind;
> ! if (reltype == OBJECT_SEQUENCE && relkind != RELKIND_SEQUENCE)
> ereport(ERROR,
> (errcode(ERRCODE_WRONG_OBJECT_TYPE),
> errmsg("\"%s\" is not a sequence",
>
> RelationGetRelationName(targetrelation))));
>
> ! if (reltype == OBJECT_VIEW && relkind != RELKIND_VIEW)
> ereport(ERROR,
> (errcode(ERRCODE_WRONG_OBJECT_TYPE),
> errmsg("\"%s\" is not a view",
>
> RelationGetRelationName(targetrelation))));
>
> ! /*
> ! * Don't allow ALTER TABLE on composite types.
> ! * We want people to use ALTER TYPE for that.
> ! */
> ! if (relkind == RELKIND_COMPOSITE_TYPE)
> ! ereport(ERROR,
> ! (errcode(ERRCODE_WRONG_OBJECT_TYPE),
> ! errmsg("\"%s\" is a composite type",
> !
> RelationGetRelationName(targetrelation)),
> ! errhint("Use ALTER TYPE RENAME TO instead.")));
> !
> ! /* Do the work */
> ! RenameRelationInternal(myrelid, newrelname, namespaceId);
> !
> ! /*
> ! * Close rel, but keep exclusive lock!
> ! */
> ! relation_close(targetrelation, NoLock);
> ! }
> !
> ! /*
> ! * RenameRelationInternal - change the name of a relation
> ! *
> ! * XXX - When renaming sequences, we don't bother to modify the
> ! * sequence name that is stored within the sequence
> itself
> ! * (this would cause problems with MVCC). In the future,
> ! * the sequence name should probably be removed from the
> ! * sequence, AFAIK there's no need for it to be there.
> ! */
> ! void
> ! RenameRelationInternal(Oid myrelid, const char *newrelname, Oid namespaceId)
> ! {
> ! Relation targetrelation;
> ! Relation relrelation; /* for RELATION relation */
> ! HeapTuple reltup;
> ! Form_pg_class relform;
> !
> ! /*
> ! * Grab an exclusive lock on the target table, index, sequence or
> ! * view, which we will NOT release until end of transaction.
> ! */
> ! targetrelation = relation_open(myrelid, AccessExclusiveLock);
>
> /*
> * Find relation's pg_class tuple, and make sure newrelname isn't in
> use.
> ***************
> *** 1704,1710 ****
> * Also rename the associated type, if any.
> */
> if (OidIsValid(targetrelation->rd_rel->reltype))
> ! TypeRename(targetrelation->rd_rel->reltype, newrelname,
> namespaceId);
>
> /*
> * Close rel, but keep exclusive lock!
> --- 1734,1740 ----
> * Also rename the associated type, if any.
> */
> if (OidIsValid(targetrelation->rd_rel->reltype))
> ! RenameTypeInternal(targetrelation->rd_rel->reltype, newrelname,
> namespaceId);
>
> /*
> * Close rel, but keep exclusive lock!
> Index: src/backend/commands/typecmds.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/backend/commands/typecmds.c,v
> retrieving revision 1.108
> diff -c -r1.108 typecmds.c
> *** src/backend/commands/typecmds.c 29 Sep 2007 17:18:58 -0000 1.108
> --- src/backend/commands/typecmds.c 30 Sep 2007 04:36:08 -0000
> ***************
> *** 2654,2656 ****
> --- 2654,2723 ----
> if (OidIsValid(arrayOid))
> AlterTypeNamespaceInternal(arrayOid, nspOid, true, true);
> }
> +
> +
> + /*
> + * Execute ALTER TYPE RENAME
> + */
> + void
> + RenameType(List *names, const char *newTypeName)
> + {
> + TypeName *typename;
> + Oid typeOid;
> + Relation rel;
> + HeapTuple tup;
> + Form_pg_type typTup;
> +
> + /* Make a TypeName so we can use standard type lookup machinery */
> + typename = makeTypeNameFromNameList(names);
> + typeOid = typenameTypeId(NULL, typename);
> +
> + /* Look up the type in the type table */
> + rel = heap_open(TypeRelationId, RowExclusiveLock);
> +
> + tup = SearchSysCacheCopy(TYPEOID,
> +
> ObjectIdGetDatum(typeOid),
> + 0, 0, 0);
> + if (!HeapTupleIsValid(tup))
> + elog(ERROR, "cache lookup failed for type %u", typeOid);
> + typTup = (Form_pg_type) GETSTRUCT(tup);
> +
> + /* check permissions on type */
> + if (!pg_type_ownercheck(typeOid, GetUserId()))
> + aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_TYPE,
> + format_type_be(typeOid));
> +
> + /*
> + * If it's a composite type, we need to check that it really is a
> + * free-standing composite type, and not a table's rowtype. We
> + * want people to use ALTER TABLE not ALTER TYPE for that case.
> + */
> + if (typTup->typtype == TYPTYPE_COMPOSITE &&
> + get_rel_relkind(typTup->typrelid) != RELKIND_COMPOSITE_TYPE)
> + ereport(ERROR,
> + (errcode(ERRCODE_WRONG_OBJECT_TYPE),
> + errmsg("\"%s\" is a table's row type",
> + TypeNameToString(typename))));
> +
> + /* don't allow direct alteration of array types, either */
> + if (OidIsValid(typTup->typelem) &&
> + get_array_type(typTup->typelem) == typeOid)
> + ereport(ERROR,
> + (errcode(ERRCODE_WRONG_OBJECT_TYPE),
> + errmsg("cannot alter array type %s",
> + format_type_be(typeOid)),
> + errhint("You can alter type %s, which will
> alter the array type as well.",
> +
> format_type_be(typTup->typelem))));
> +
> + /*
> + * If type is composite type we need to rename associated relation too.
> + * RenameRelationInternal will call RenameTypeInternal automatically.
> + */
> + if (typTup->typtype == TYPTYPE_COMPOSITE)
> + RenameRelationInternal(typTup->typrelid, newTypeName,
> typTup->typnamespace);
> + else
> + RenameTypeInternal(typeOid, newTypeName, typTup->typnamespace);
> +
> + /* Clean up */
> + heap_close(rel, RowExclusiveLock);
> + }
> Index: src/backend/parser/gram.y
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/backend/parser/gram.y,v
> retrieving revision 2.603
> diff -c -r2.603 gram.y
> *** src/backend/parser/gram.y 24 Sep 2007 01:29:28 -0000 2.603
> --- src/backend/parser/gram.y 29 Sep 2007 05:13:21 -0000
> ***************
> *** 4748,4753 ****
> --- 4748,4761 ----
> n->newname = $8;
> $$ = (Node *)n;
> }
> + | ALTER TYPE_P any_name RENAME TO name
> + {
> + RenameStmt *n = makeNode(RenameStmt);
> + n->renameType = OBJECT_TYPE;
> + n->object = $3;
> + n->newname = $6;
> + $$ = (Node *)n;
> + }
> ;
>
> opt_column: COLUMN
> { $$ = COLUMN; }
> Index: src/include/catalog/pg_type.h
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/include/catalog/pg_type.h,v
> retrieving revision 1.188
> diff -c -r1.188 pg_type.h
> *** src/include/catalog/pg_type.h 3 Sep 2007 02:30:45 -0000 1.188
> --- src/include/catalog/pg_type.h 29 Sep 2007 23:04:57 -0000
> ***************
> *** 678,684 ****
> Node *defaultExpr,
> bool rebuild);
>
> ! extern void TypeRename(Oid typeOid, const char *newTypeName,
> Oid typeNamespace);
>
> extern char *makeArrayTypeName(const char *typeName, Oid typeNamespace);
> --- 678,684 ----
> Node *defaultExpr,
> bool rebuild);
>
> ! extern void RenameTypeInternal(Oid typeOid, const char *newTypeName,
> Oid typeNamespace);
>
> extern char *makeArrayTypeName(const char *typeName, Oid typeNamespace);
> Index: src/include/commands/tablecmds.h
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/include/commands/tablecmds.h,v
> retrieving revision 1.34
> diff -c -r1.34 tablecmds.h
> *** src/include/commands/tablecmds.h 3 Jul 2007 01:30:37 -0000 1.34
> --- src/include/commands/tablecmds.h 30 Sep 2007 03:49:44 -0000
> ***************
> *** 42,51 ****
> bool recurse,
> bool recursing);
>
> ! extern void renamerel(Oid myrelid,
> const char *newrelname,
> ObjectType reltype);
>
> extern void find_composite_type_dependencies(Oid typeOid,
>
> const char *origTblName,
>
> const char *origTypeName);
> --- 42,55 ----
> bool recurse,
> bool recursing);
>
> ! extern void RenameRelation(Oid myrelid,
> const char *newrelname,
> ObjectType reltype);
>
> + extern void RenameRelationInternal(Oid myrelid,
> + const char *newrelname,
> + Oid namespaceId);
> +
> extern void find_composite_type_dependencies(Oid typeOid,
>
> const char *origTblName,
>
> const char *origTypeName);
> Index: src/include/commands/typecmds.h
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/include/commands/typecmds.h,v
> retrieving revision 1.19
> diff -c -r1.19 typecmds.h
> *** src/include/commands/typecmds.h 11 May 2007 17:57:14 -0000 1.19
> --- src/include/commands/typecmds.h 29 Sep 2007 05:11:57 -0000
> ***************
> *** 43,46 ****
> --- 43,48 ----
> bool
> isImplicitArray,
> bool
> errorOnTableType);
>
> + extern void RenameType(List *names, const char *newTypeName);
> +
> #endif /* TYPECMDS_H */
>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/docs/faq
--
Bruce Momjian <[EMAIL PROTECTED]> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches
