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

Reply via email to