Patch withdrawn by author.

---------------------------------------------------------------------------

Christopher Kings-Lynne wrote:
> Hi,
> 
> The attached patch adds a new command:
> 
> ALTER TABLE [ONLY] foo SET WITHOUT CLUSTER;
> 
> I would probably have preferred the DROP CLUSTER syntax, but it doesn't 
> seem easy to get working without shift/reduce problems.
> 
> It has support for inheritance.
> 
> I have also fixed the previously detailed security errors:
> 
> * Ownership is now checked
> * Can now not cluster system catalogs
> * Checks that what you are clustering is actually a table
> 
> And it fixes a related bug in the SET WITHOUT OIDS implementation:
> 
> * Checks that what you are dropping OIDS from is actually a table
> 
> I have included a documentation update and a regression test.
> 
> Chris
> 
> 

> Index: doc/src/sgml/ref/alter_table.sgml
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/doc/src/sgml/ref/alter_table.sgml,v
> retrieving revision 1.68
> diff -c -r1.68 alter_table.sgml
> *** doc/src/sgml/ref/alter_table.sgml 24 Mar 2004 09:49:20 -0000      1.68
> --- doc/src/sgml/ref/alter_table.sgml 28 Apr 2004 07:59:10 -0000
> ***************
> *** 47,52 ****
> --- 47,54 ----
>       OWNER TO <replaceable class="PARAMETER">new_owner</replaceable>
>   ALTER TABLE <replaceable class="PARAMETER">name</replaceable>
>       CLUSTER ON <replaceable class="PARAMETER">index_name</replaceable>
> + ALTER TABLE [ ONLY ] <replaceable class="PARAMETER">name</replaceable>
> +     SET WITHOUT CLUSTER
>   </synopsis>
>    </refsynopsisdiv>
>   
> ***************
> *** 218,223 ****
> --- 220,235 ----
>       </listitem>
>      </varlistentry>
>   
> +    <varlistentry>
> +     <term><literal>SET WITHOUT CLUSTER</literal></term>
> +     <listitem>
> +      <para>
> +       This form disables future <xref linkend="SQL-CLUSTER" 
> endterm="sql-cluster-title"> on
> +       any indexes on a table. 
> +      </para>
> +     </listitem>
> +    </varlistentry>
> +  
>     </variablelist>
>     </para>
>   
> Index: src/backend/commands/tablecmds.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/backend/commands/tablecmds.c,v
> retrieving revision 1.102
> diff -c -r1.102 tablecmds.c
> *** src/backend/commands/tablecmds.c  1 Apr 2004 21:28:44 -0000       1.102
> --- src/backend/commands/tablecmds.c  28 Apr 2004 07:59:12 -0000
> ***************
> *** 2433,2438 ****
> --- 2433,2444 ----
>   
>       rel = heap_open(myrelid, AccessExclusiveLock);
>   
> +     if (rel->rd_rel->relkind != RELKIND_RELATION)
> +             ereport(ERROR,
> +                             (errcode(ERRCODE_WRONG_OBJECT_TYPE),
> +                              errmsg("\"%s\" is not a table",
> +                                             RelationGetRelationName(rel))));
> + 
>       /*
>        * check to see if we actually need to change anything
>        */
> ***************
> *** 3902,3908 ****
>    * The only thing we have to do is to change the indisclustered bits.
>    */
>   void
> ! AlterTableClusterOn(Oid relOid, const char *indexName)
>   {
>       Relation        rel,
>                               pg_index;
> --- 3908,3914 ----
>    * The only thing we have to do is to change the indisclustered bits.
>    */
>   void
> ! AlterTableClusterOn(Oid relOid, const char *indexName, bool recurse)
>   {
>       Relation        rel,
>                               pg_index;
> ***************
> *** 3913,3942 ****
>   
>       rel = heap_open(relOid, AccessExclusiveLock);
>   
> !     indexOid = get_relname_relid(indexName, rel->rd_rel->relnamespace);
> ! 
> !     if (!OidIsValid(indexOid))
>               ereport(ERROR,
> !                             (errcode(ERRCODE_UNDEFINED_OBJECT),
> !                              errmsg("index \"%s\" for table \"%s\" does not exist",
> !                                             indexName, 
> NameStr(rel->rd_rel->relname))));
>   
> !     indexTuple = SearchSysCache(INDEXRELID,
> !                                                             
> ObjectIdGetDatum(indexOid),
> !                                                             0, 0, 0);
> !     if (!HeapTupleIsValid(indexTuple))
> !             elog(ERROR, "cache lookup failed for index %u", indexOid);
> !     indexForm = (Form_pg_index) GETSTRUCT(indexTuple);
>   
>       /*
> !      * If this is the same index the relation was previously clustered on,
> !      * no need to do anything.
>        */
> !     if (indexForm->indisclustered)
>       {
> !             ReleaseSysCache(indexTuple);
> !             heap_close(rel, NoLock);
> !             return;
>       }
>   
>       pg_index = heap_openr(IndexRelationName, RowExclusiveLock);
> --- 3919,4005 ----
>   
>       rel = heap_open(relOid, AccessExclusiveLock);
>   
> !     /* Only allow cluster on regular tables */
> !     if (rel->rd_rel->relkind != RELKIND_RELATION)
>               ereport(ERROR,
> !                             (errcode(ERRCODE_WRONG_OBJECT_TYPE),
> !                              errmsg("\"%s\" is not a table",
> !                                             RelationGetRelationName(rel))));
>   
> !     /* Permissions checks */
> !     if (!pg_class_ownercheck(relOid, GetUserId()))
> !             aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
> !                                        RelationGetRelationName(rel));
> ! 
> !     /* Don't allow clustering on system catalogs */
> !     if (!allowSystemTableMods && IsSystemRelation(rel))
> !             ereport(ERROR,
> !                             (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
> !                              errmsg("permission denied: \"%s\" is a system 
> catalog",
> !                                             RelationGetRelationName(rel))));
>   
>       /*
> !      * Process child tables if requested. Only if we're turning off clustering.
>        */
> !     if (recurse && indexName == NULL)
>       {
> !             List       *child,
> !                                *children;
> ! 
> !             /* This routine is actually in the planner */
> !             children = find_all_inheritors(relOid);
> ! 
> !             /*
> !              * find_all_inheritors does the recursive search of the
> !              * inheritance hierarchy, so all we have to do is process all of
> !              * the relids in the list that it returns.
> !              */
> !             foreach(child, children)
> !             {
> !                     Oid     childrelid = lfirsto(child);
> !                     if (childrelid == relOid)
> !                             continue;
> !                     AlterTableClusterOn(childrelid, indexName, FALSE);
> !             }
> !     }
> ! 
> !     /* Now proceed with the actions on just this table. */
> ! 
> !     /*
> !      * We only fetch the index if indexName is not null.  A null index
> !          * name indicates that we're removing all clustering on this table.
> !      */
> !     if (indexName != NULL) {
> !             indexOid = get_relname_relid(indexName, rel->rd_rel->relnamespace);
> ! 
> !             if (!OidIsValid(indexOid))
> !                     ereport(ERROR,
> !                                     (errcode(ERRCODE_UNDEFINED_OBJECT),
> !                                      errmsg("index \"%s\" for table \"%s\" does 
> not exist",
> !                                                     indexName, 
> NameStr(rel->rd_rel->relname))));
> ! 
> !             indexTuple = SearchSysCache(INDEXRELID,
> !                                                                     
> ObjectIdGetDatum(indexOid),
> !                                                                     0, 0, 0);
> !             if (!HeapTupleIsValid(indexTuple))
> !                     elog(ERROR, "cache lookup failed for index %u", indexOid);
> !             indexForm = (Form_pg_index) GETSTRUCT(indexTuple);
> ! 
> !             /*
> !              * If this is the same index the relation was previously clustered on,
> !              * no need to do anything.
> !              */
> !             if (indexForm->indisclustered)
> !             {
> !                     ReleaseSysCache(indexTuple);
> !                     heap_close(rel, NoLock);
> !                     return;
> !             }
> !     }
> !     else {
> !             /* Set to NULL to prevent compiler warnings */
> !             indexTuple = NULL;
> !             indexForm = NULL;
>       }
>   
>       pg_index = heap_openr(IndexRelationName, RowExclusiveLock);
> ***************
> *** 3959,3965 ****
>   
>               /*
>                * Unset the bit if set.  We know it's wrong because we checked
> !              * this earlier.
>                */
>               if (idxForm->indisclustered)
>               {
> --- 4022,4028 ----
>   
>               /*
>                * Unset the bit if set.  We know it's wrong because we checked
> !              * this earlier.  If we're removing all clustering, we do this too.
>                */
>               if (idxForm->indisclustered)
>               {
> ***************
> *** 3967,3973 ****
>                       simple_heap_update(pg_index, &idxtuple->t_self, idxtuple);
>                       CatalogUpdateIndexes(pg_index, idxtuple);
>               }
> !             else if (idxForm->indexrelid == indexForm->indexrelid)
>               {
>                       idxForm->indisclustered = true;
>                       simple_heap_update(pg_index, &idxtuple->t_self, idxtuple);
> --- 4030,4040 ----
>                       simple_heap_update(pg_index, &idxtuple->t_self, idxtuple);
>                       CatalogUpdateIndexes(pg_index, idxtuple);
>               }
> !             /*
> !              * If the index is the one we're clustering, set its cluster flag to 
> true.
> !              * However, we do this only if we're not removing all clustering.
> !              */
> !             else if (indexName != NULL && idxForm->indexrelid == 
> indexForm->indexrelid)
>               {
>                       idxForm->indisclustered = true;
>                       simple_heap_update(pg_index, &idxtuple->t_self, idxtuple);
> ***************
> *** 3978,3984 ****
>   
>       heap_close(pg_index, RowExclusiveLock);
>   
> !     ReleaseSysCache(indexTuple);
>   
>       heap_close(rel, NoLock);        /* close rel, but keep lock till commit */
>   }
> --- 4045,4051 ----
>   
>       heap_close(pg_index, RowExclusiveLock);
>   
> !     if (indexName != NULL) ReleaseSysCache(indexTuple);
>   
>       heap_close(rel, NoLock);        /* close rel, but keep lock till commit */
>   }
> Index: src/backend/parser/gram.y
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/backend/parser/gram.y,v
> retrieving revision 2.452
> diff -c -r2.452 gram.y
> *** src/backend/parser/gram.y 21 Apr 2004 00:34:18 -0000      2.452
> --- src/backend/parser/gram.y 28 Apr 2004 07:59:13 -0000
> ***************
> *** 1250,1255 ****
> --- 1250,1264 ----
>                                       n->name = $6;
>                                       $$ = (Node *)n;
>                               }
> +                     /* ALTER TABLE <name> SET WITHOUT CLUSTER */ 
> +                     | ALTER TABLE relation_expr SET WITHOUT CLUSTER
> +                             {
> +                                     AlterTableStmt *n = makeNode(AlterTableStmt);
> +                                     n->subtype = 'l';
> +                                     n->relation = $3;
> +                                     n->name = NULL;
> +                                     $$ = (Node *)n;
> +                             }
>               ;
>   
>   alter_column_default:
> Index: src/backend/tcop/utility.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/backend/tcop/utility.c,v
> retrieving revision 1.213
> diff -c -r1.213 utility.c
> *** src/backend/tcop/utility.c        22 Apr 2004 02:58:20 -0000      1.213
> --- src/backend/tcop/utility.c        28 Apr 2004 07:59:14 -0000
> ***************
> *** 603,609 ****
>                                                                               
> get_usesysid(stmt->name));
>                                               break;
>                                       case 'L':       /* CLUSTER ON */
> !                                             AlterTableClusterOn(relid, stmt->name);
>                                               break;
>                                       case 'o':       /* SET WITHOUT OIDS */
>                                               AlterTableAlterOids(relid,
> --- 603,612 ----
>                                                                               
> get_usesysid(stmt->name));
>                                               break;
>                                       case 'L':       /* CLUSTER ON */
> !                                             AlterTableClusterOn(relid, stmt->name, 
> false);
> !                                             break;
> !                                     case 'l':       /* SET WITHOUT CLUSTER */
> !                                             AlterTableClusterOn(relid, stmt->name, 
> interpretInhOption(stmt->relation->inhOpt));
>                                               break;
>                                       case 'o':       /* SET WITHOUT OIDS */
>                                               AlterTableAlterOids(relid,
> Index: src/include/commands/tablecmds.h
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/include/commands/tablecmds.h,v
> retrieving revision 1.15
> diff -c -r1.15 tablecmds.h
> *** src/include/commands/tablecmds.h  23 Mar 2004 19:35:17 -0000      1.15
> --- src/include/commands/tablecmds.h  28 Apr 2004 07:59:14 -0000
> ***************
> *** 43,49 ****
>                                                const char *constrName,
>                                                DropBehavior behavior);
>   
> ! extern void AlterTableClusterOn(Oid relOid, const char *indexName);
>   
>   extern void AlterTableCreateToastTable(Oid relOid, bool silent);
>   
> --- 43,49 ----
>                                                const char *constrName,
>                                                DropBehavior behavior);
>   
> ! extern void AlterTableClusterOn(Oid relOid, const char *indexName, bool recurse);
>   
>   extern void AlterTableCreateToastTable(Oid relOid, bool silent);
>   
> Index: src/test/regress/expected/cluster.out
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/test/regress/expected/cluster.out,v
> retrieving revision 1.14
> diff -c -r1.14 cluster.out
> *** src/test/regress/expected/cluster.out     2 Oct 2003 06:32:46 -0000       1.14
> --- src/test/regress/expected/cluster.out     28 Apr 2004 07:59:14 -0000
> ***************
> *** 297,302 ****
> --- 297,313 ----
>    clstr_tst_b_c
>   (1 row)
>   
> + -- Try turning off all clustering
> + ALTER TABLE clstr_tst SET WITHOUT CLUSTER;
> + SELECT pg_class.relname FROM pg_index, pg_class, pg_class AS pg_class_2
> + WHERE pg_class.oid=indexrelid
> +     AND indrelid=pg_class_2.oid
> +     AND pg_class_2.relname = 'clstr_tst'
> +     AND indisclustered;
> +   relname 
> +  ---------
> +  (0 rows)
> + 
>   -- Verify that clustering all tables does in fact cluster the right ones
>   CREATE USER clstr_user;
>   CREATE TABLE clstr_1 (a INT PRIMARY KEY);
> Index: src/test/regress/sql/cluster.sql
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/test/regress/sql/cluster.sql,v
> retrieving revision 1.7
> diff -c -r1.7 cluster.sql
> *** src/test/regress/sql/cluster.sql  20 Mar 2003 18:52:48 -0000      1.7
> --- src/test/regress/sql/cluster.sql  28 Apr 2004 07:59:14 -0000
> ***************
> *** 95,100 ****
> --- 95,108 ----
>       AND pg_class_2.relname = 'clstr_tst'
>       AND indisclustered;
>   
> + -- Try turning off all clustering
> + ALTER TABLE clstr_tst SET WITHOUT CLUSTER;
> + SELECT pg_class.relname FROM pg_index, pg_class, pg_class AS pg_class_2
> + WHERE pg_class.oid=indexrelid
> +     AND indrelid=pg_class_2.oid
> +     AND pg_class_2.relname = 'clstr_tst'
> +     AND indisclustered;
> + 
>   -- Verify that clustering all tables does in fact cluster the right ones
>   CREATE USER clstr_user;
>   CREATE TABLE clstr_1 (a INT PRIMARY KEY);

> 
> ---------------------------(end of broadcast)---------------------------
> TIP 7: don't forget to increase your free space map settings

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  [EMAIL PROTECTED]               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

---------------------------(end of broadcast)---------------------------
TIP 4: Don't 'kill -9' the postmaster

Reply via email to