Thread: ALTER TABLE...SET WITHOUT CLUSTER

ALTER TABLE...SET WITHOUT CLUSTER

From
Christopher Kings-Lynne
Date:
This patch imlements the TODO that calls for the ability to turn off all
clustering on a table.

Syntax is ALTER TABLE ... SET WITHOUT CLUSTER;

Doc patch plus regression test is included.

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.66
diff -c -r1.66 alter_table.sgml
*** doc/src/sgml/ref/alter_table.sgml    9 Mar 2004 16:57:47 -0000    1.66
--- doc/src/sgml/ref/alter_table.sgml    18 Mar 2004 03:51:41 -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 <replaceable class="PARAMETER">name</replaceable>
+     SET WITHOUT CLUSTER
  </synopsis>
   </refsynopsisdiv>

***************
*** 219,224 ****
--- 221,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 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.100
diff -c -r1.100 tablecmds.c
*** src/backend/commands/tablecmds.c    13 Mar 2004 22:09:13 -0000    1.100
--- src/backend/commands/tablecmds.c    18 Mar 2004 03:51:42 -0000
***************
*** 3970,3999 ****

      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);
--- 3970,4010 ----

      rel = heap_open(relOid, AccessExclusiveLock);

      /*
!      * 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);
***************
*** 4016,4022 ****

          /*
           * Unset the bit if set.  We know it's wrong because we checked
!          * this earlier.
           */
          if (idxForm->indisclustered)
          {
--- 4027,4033 ----

          /*
           * 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)
          {
***************
*** 4024,4030 ****
              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);
--- 4035,4045 ----
              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);
***************
*** 4035,4041 ****

      heap_close(pg_index, RowExclusiveLock);

!     ReleaseSysCache(indexTuple);

      heap_close(rel, NoLock);    /* close rel, but keep lock till commit */
  }
--- 4050,4056 ----

      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.449
diff -c -r2.449 gram.y
*** src/backend/parser/gram.y    17 Mar 2004 20:48:42 -0000    2.449
--- src/backend/parser/gram.y    18 Mar 2004 03:51:45 -0000
***************
*** 1248,1253 ****
--- 1248,1262 ----
                      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/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    18 Mar 2004 03:51:45 -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    18 Mar 2004 03:51:46 -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);

Re: ALTER TABLE...SET WITHOUT CLUSTER

From
Christopher Kings-Lynne
Date:
OK, based on Tom's comments about qualified_name vs. relation_expr, I
may have used the wrong one in this case.  Will check soon.

Chris

Christopher Kings-Lynne wrote:

> This patch imlements the TODO that calls for the ability to turn off all
> clustering on a table.
>
> Syntax is ALTER TABLE ... SET WITHOUT CLUSTER;
>
> Doc patch plus regression test is included.
>
> Chris


Re: ALTER TABLE...SET WITHOUT CLUSTER

From
Christopher Kings-Lynne
Date:
> This patch imlements the TODO that calls for the ability to turn off all
> clustering on a table.
>
> Syntax is ALTER TABLE ... SET WITHOUT CLUSTER;
>
> Doc patch plus regression test is included.

OK, I have a problem here.  This is the new grammar that I added:

/* 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;
         }

Now, I have to change that relation_expr to qualified_name.  However,
this causes shift/reduce errors. (Due to ALTER TABLE relation_expr SET
WITHOUT OIDS.)

Even changing the syntax to "qualified_name DROP CLUSTER" doesn't work
due to the existence of "relation_expr DROP ...".

What's the solution?  I can't figure it out...

Chris


Re: ALTER TABLE...SET WITHOUT CLUSTER

From
Bruce Momjian
Date:
Christopher Kings-Lynne wrote:
> > This patch imlements the TODO that calls for the ability to turn off all
> > clustering on a table.
> >
> > Syntax is ALTER TABLE ... SET WITHOUT CLUSTER;
> >
> > Doc patch plus regression test is included.
>
> OK, I have a problem here.  This is the new grammar that I added:
>
> /* 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;
>          }
>
> Now, I have to change that relation_expr to qualified_name.  However,
> this causes shift/reduce errors. (Due to ALTER TABLE relation_expr SET
> WITHOUT OIDS.)
>
> Even changing the syntax to "qualified_name DROP CLUSTER" doesn't work
> due to the existence of "relation_expr DROP ...".

I have an idea.  Change the code to use relation_expr, then throw an
error from gram.y if $$->inhOpt != INH_DEFAULT.

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

Re: ALTER TABLE...SET WITHOUT CLUSTER

From
Christopher Kings-Lynne
Date:
>>Even changing the syntax to "qualified_name DROP CLUSTER" doesn't work
>>due to the existence of "relation_expr DROP ...".
>
> I have an idea.  Change the code to use relation_expr, then throw an
> error from gram.y if $$->inhOpt != INH_DEFAULT.

Yeah, that was the plan.  I have been too busy (or too lazy more like)
to actually get around to doing it and submitting it however...  I am
actually more tempted to do the right thing and actually make it recurse
properly over the hierarchy...

Chris