Re: [PATCHES] ALTER TYPE RENAME

2008-03-19 Thread Tom Lane 
Petr Jelinek [EMAIL PROTECTED] writes:
 Anyway, I am sending second version of the patch.

Applied with minor corrections.

regards, tom lane

-- 
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

Re: [PATCHES] ALTER TYPE RENAME

2008-03-19 Thread Bruce Momjian 

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 -  1.4
 --- doc/src/sgml/ref/alter_type.sgml  29 Sep 2007 05:43:14 -
 ***
 *** 26,31 
 --- 26,32 
 synopsis
   ALTER TYPE replaceable class=PARAMETERname/replaceable OWNER TO 
 replaceable class=PARAMETERnew_owner/replaceable 
   ALTER TYPE replaceable class=PARAMETERname/replaceable SET SCHEMA 
 replaceable class=PARAMETERnew_schema/replaceable
 + ALTER TYPE replaceable class=PARAMETERname/replaceable RNAME TO 
 replaceable class=PARAMETERnew_name/replaceable
 /synopsis
/refsynopsisdiv
   
 ***
 *** 83,88 
 --- 84,98 
 /listitem
/varlistentry
   
 +  varlistentry
 +   termreplaceable class=PARAMETERnew_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 -  1.113
 --- src/backend/catalog/pg_type.c 30 Sep 2007 04:20:03 -
 ***
 *** 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)
   {
   Relationpg_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)
   {
   Relationpg_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 =

Re: [PATCHES] ALTER TYPE RENAME

2007-11-04 Thread Bruce Momjian 

This has been saved for the 8.4 release:

http://momjian.postgresql.org/cgi-bin/pgpatches_hold

---

Petr Jelinek wrote:
 Hi,
 I noticed we don't have ALTER TYPE foo RENAME TO bar command which would 
 be handy for me especially for enum types.
 So I wrote this little patch (including very brief doc) which adds above 
 syntax. It basically just does some checks and calls existing TypeRename 
 function which is used for renaming table rowtype now.
 I hope I haven't missed anything, but I am unsure about two things. 
 First, this patch allows renaming base types which I don't know if it's 
 desired. And second we might want to throw error when renaming rowtype 
 (there is check in AlterTypeOwner for this but not in AlterTypeNamespace 
 so I don't know).
 
 -- 
 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 -  1.4
 --- doc/src/sgml/ref/alter_type.sgml  29 Sep 2007 05:43:14 -
 ***
 *** 26,31 
 --- 26,32 
 synopsis
   ALTER TYPE replaceable class=PARAMETERname/replaceable OWNER TO 
 replaceable class=PARAMETERnew_owner/replaceable 
   ALTER TYPE replaceable class=PARAMETERname/replaceable SET SCHEMA 
 replaceable class=PARAMETERnew_schema/replaceable
 + ALTER TYPE replaceable class=PARAMETERname/replaceable RENAME TO 
 replaceable class=PARAMETERnew_name/replaceable
 /synopsis
/refsynopsisdiv
   
 ***
 *** 83,88 
 --- 84,98 
 /listitem
/varlistentry
   
 +  varlistentry
 +   termreplaceable class=PARAMETERnew_name/replaceable/term
 +   listitem
 +para
 + The new name for the type.
 +/para
 +   /listitem
 +  /varlistentry
 + 
   /variablelist
  /para
 /refsect1
 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 -  1.25
 --- src/backend/commands/alter.c  29 Sep 2007 05:12:31 -
 ***
 *** 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/typecmds.c
 ===
 RCS file: /projects/cvsroot/pgsql/src/backend/commands/typecmds.c,v
 retrieving revision 1.107
 diff -c -r1.107 typecmds.c
 *** src/backend/commands/typecmds.c   4 Sep 2007 16:41:42 -   1.107
 --- src/backend/commands/typecmds.c   29 Sep 2007 05:11:22 -
 ***
 *** 2514,2519 
 --- 2514,2567 
   }
   
   /*
 +  * Execute ALTER TYPE RENAME
 +  */
 + void
 + RenameType(List *names, const char *newTypeName)
 + {
 + TypeName   *typename;
 + Oid typeOid;
 + Relationrel;
 + 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));
 + 
 + /* don't allow direct alteration of array types */
 + 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.,

[PATCHES] ALTER TYPE RENAME

2007-09-29 Thread Petr Jelinek 

Hi,
I noticed we don't have ALTER TYPE foo RENAME TO bar command which would 
be handy for me especially for enum types.
So I wrote this little patch (including very brief doc) which adds above 
syntax. It basically just does some checks and calls existing TypeRename 
function which is used for renaming table rowtype now.
I hope I haven't missed anything, but I am unsure about two things. 
First, this patch allows renaming base types which I don't know if it's 
desired. And second we might want to throw error when renaming rowtype 
(there is check in AlterTypeOwner for this but not in AlterTypeNamespace 
so I don't know).


--
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.sgml16 Sep 2006 00:30:16 -  1.4
--- doc/src/sgml/ref/alter_type.sgml29 Sep 2007 05:43:14 -
***
*** 26,31 
--- 26,32 
synopsis
  ALTER TYPE replaceable class=PARAMETERname/replaceable OWNER TO 
replaceable class=PARAMETERnew_owner/replaceable 
  ALTER TYPE replaceable class=PARAMETERname/replaceable SET SCHEMA 
replaceable class=PARAMETERnew_schema/replaceable
+ ALTER TYPE replaceable class=PARAMETERname/replaceable RENAME TO 
replaceable class=PARAMETERnew_name/replaceable
/synopsis
   /refsynopsisdiv
  
***
*** 83,88 
--- 84,98 
/listitem
   /varlistentry
  
+  varlistentry
+   termreplaceable class=PARAMETERnew_name/replaceable/term
+   listitem
+para
+ The new name for the type.
+/para
+   /listitem
+  /varlistentry
+ 
  /variablelist
 /para
/refsect1
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.c21 Aug 2007 01:11:14 -  1.25
--- src/backend/commands/alter.c29 Sep 2007 05:12:31 -
***
*** 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/typecmds.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/commands/typecmds.c,v
retrieving revision 1.107
diff -c -r1.107 typecmds.c
*** src/backend/commands/typecmds.c 4 Sep 2007 16:41:42 -   1.107
--- src/backend/commands/typecmds.c 29 Sep 2007 05:11:22 -
***
*** 2514,2519 
--- 2514,2567 
  }
  
  /*
+  * Execute ALTER TYPE RENAME
+  */
+ void
+ RenameType(List *names, const char *newTypeName)
+ {
+   TypeName   *typename;
+   Oid typeOid;
+   Relationrel;
+   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));
+ 
+   /* don't allow direct alteration of array types */
+   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;
+ 
+   /* and do the work */
+   TypeRename(typeOid, newTypeName, typTup-typnamespace);
+ 
+   /* Clean up */
+   heap_close(rel,

Re: [PATCHES] ALTER TYPE RENAME

2007-09-29 Thread Tom Lane 
I wrote:
 Hm, it's definitely a bug/oversight that AlterTypeNamespace doesn't make
 a similar test for that.

On closer look, there *is* such a test, but it's down inside
AlterTypeNamespaceInternal.

regards, tom lane

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
   subscribe-nomail command to [EMAIL PROTECTED] so that your
   message can get through to the mailing list cleanly

Re: [PATCHES] ALTER TYPE RENAME

2007-09-29 Thread Tom Lane 
BTW, another issue this brings up is whether we should reject

regression=# create type footyp as (f2 int);
CREATE TYPE
regression=# alter table footyp rename to foobar;
ALTER TABLE

Currently, since there's no ALTER TYPE RENAME command, this is useful
functionality and I wouldn't want to forbid it.  But if we provide
ALTER TYPE RENAME then consistency would suggest requiring people to
use that for composite types.

regards, tom lane

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match

Re: [PATCHES] ALTER TYPE RENAME

2007-09-29 Thread Petr Jelinek 

Tom Lane wrote:

BTW, another issue this brings up is whether we should reject

regression=# create type footyp as (f2 int);
CREATE TYPE
regression=# alter table footyp rename to foobar;
ALTER TABLE

Currently, since there's no ALTER TYPE RENAME command, this is useful
functionality and I wouldn't want to forbid it.  But if we provide
ALTER TYPE RENAME then consistency would suggest requiring people to
use that for composite types.
  

I assume ALTER TYPE RENAME should rename associated relation too, then.

--
Regards
Petr Jelinek (PJMODOS)


---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
  subscribe-nomail command to [EMAIL PROTECTED] so that your
  message can get through to the mailing list cleanly

Re: [PATCHES] ALTER TYPE RENAME

2007-09-29 Thread Petr Jelinek 

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.sgml16 Sep 2006 00:30:16 -  1.4
--- doc/src/sgml/ref/alter_type.sgml29 Sep 2007 05:43:14 -
***
*** 26,31 
--- 26,32 
synopsis
  ALTER TYPE replaceable class=PARAMETERname/replaceable OWNER TO 
replaceable class=PARAMETERnew_owner/replaceable 
  ALTER TYPE replaceable class=PARAMETERname/replaceable SET SCHEMA 
replaceable class=PARAMETERnew_schema/replaceable
+ ALTER TYPE replaceable class=PARAMETERname/replaceable RNAME TO 
replaceable class=PARAMETERnew_name/replaceable
/synopsis
   /refsynopsisdiv
  
***
*** 83,88 
--- 84,98 
/listitem
   /varlistentry
  
+  varlistentry
+   termreplaceable class=PARAMETERnew_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 -  1.113
--- src/backend/catalog/pg_type.c   30 Sep 2007 04:20:03 -
***
*** 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)
  {
Relationpg_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)
  {
Relationpg_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: