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
