Re: Dealing with CLUSTER failures - Mailing list pgsql-patches

Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > I looked over this item, originally posted as:
> >     http://archives.postgresql.org/pgsql-hackers/2005-03/msg01055.php
>
> I still think this is substantial complication (more than likely
> introducing some bugs) to deal with a non problem.
>
> http://archives.postgresql.org/pgsql-hackers/2005-03/msg01058.php

Well, it is a demonstrated problem, and we currently don't even report
the index name that caused the cluster to fail.

Here is a new patch that continues to throw an error for a failed
database-wide cluster (no warning) if a single table fails.  It does
print the index name and it prints a hint that the user can use ALTER
TABLE ... SET WITHOUT CLUSTER to avoid the failure:

    test=> CLUSTER  test_gist_idx ON  test;
    ERROR:  cannot cluster on index "test_gist_idx" because access method
    does not handle null values
    HINT:  You may be able to work around this by marking column "a" NOT
    NULL.

    test=> CLUSTER;
    ERROR:  cannot cluster on index "test_gist_idx" because access method
    does not handle null values
    HINT:  You may be able to work around this by marking column "a" NOT
    NULL, or use ALTER TABLE ... SET WITHOUT CLUSTER to remove the cluster
    specification from the table.

--
  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
Index: src/backend/commands/cluster.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/cluster.c,v
retrieving revision 1.137
diff -c -c -r1.137 cluster.c
*** src/backend/commands/cluster.c    6 May 2005 17:24:53 -0000    1.137
--- src/backend/commands/cluster.c    10 May 2005 00:26:48 -0000
***************
*** 292,298 ****
      OldHeap = heap_open(rvtc->tableOid, AccessExclusiveLock);

      /* Check index is valid to cluster on */
!     check_index_is_clusterable(OldHeap, rvtc->indexOid);

      /* rebuild_relation does all the dirty work */
      rebuild_relation(OldHeap, rvtc->indexOid);
--- 292,298 ----
      OldHeap = heap_open(rvtc->tableOid, AccessExclusiveLock);

      /* Check index is valid to cluster on */
!     check_index_is_clusterable(OldHeap, rvtc->indexOid, recheck);

      /* rebuild_relation does all the dirty work */
      rebuild_relation(OldHeap, rvtc->indexOid);
***************
*** 309,315 ****
   * definition can't change under us.
   */
  void
! check_index_is_clusterable(Relation OldHeap, Oid indexOid)
  {
      Relation    OldIndex;

--- 309,315 ----
   * definition can't change under us.
   */
  void
! check_index_is_clusterable(Relation OldHeap, Oid indexOid, bool recheck)
  {
      Relation    OldIndex;

***************
*** 336,342 ****
      if (!heap_attisnull(OldIndex->rd_indextuple, Anum_pg_index_indpred))
          ereport(ERROR,
                  (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
!                  errmsg("cannot cluster on partial index")));
      if (!OldIndex->rd_am->amindexnulls)
      {
          AttrNumber    colno;
--- 336,344 ----
      if (!heap_attisnull(OldIndex->rd_indextuple, Anum_pg_index_indpred))
          ereport(ERROR,
                  (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
!                  errmsg("cannot cluster on partial index \"%s\"",
!                         RelationGetRelationName(OldIndex))));
!
      if (!OldIndex->rd_am->amindexnulls)
      {
          AttrNumber    colno;
***************
*** 354,374 ****
              if (!OldHeap->rd_att->attrs[colno - 1]->attnotnull)
                  ereport(ERROR,
                          (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
!                          errmsg("cannot cluster when index access method does not handle null values"),
!                          errhint("You may be able to work around this by marking column \"%s\" NOT NULL.",
!                   NameStr(OldHeap->rd_att->attrs[colno - 1]->attname))));
          }
          else if (colno < 0)
          {
              /* system column --- okay, always non-null */
          }
          else
-         {
              /* index expression, lose... */
              ereport(ERROR,
                      (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
!                      errmsg("cannot cluster on expressional index when index access method does not handle null
values")));
!         }
      }

      /*
--- 356,380 ----
              if (!OldHeap->rd_att->attrs[colno - 1]->attnotnull)
                  ereport(ERROR,
                          (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
!                          errmsg("cannot cluster on index \"%s\" because access method\n"
!                                 "does not handle null values",
!                               RelationGetRelationName(OldIndex)),
!                          errhint("You may be able to work around this by marking column \"%s\" NOT NULL%s",
!                             NameStr(OldHeap->rd_att->attrs[colno - 1]->attname),
!                             recheck ? ",\nor use ALTER TABLE ... SET WITHOUT CLUSTER to remove the cluster\n"
!                                     "specification from the table." : ".")));
          }
          else if (colno < 0)
          {
              /* system column --- okay, always non-null */
          }
          else
              /* index expression, lose... */
              ereport(ERROR,
                      (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
!                      errmsg("cannot cluster on expressional index \"%s\" because its index access\n"
!                             "method does not handle null values",
!                         RelationGetRelationName(OldIndex))));
      }

      /*
Index: src/backend/commands/tablecmds.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/tablecmds.c,v
retrieving revision 1.156
diff -c -c -r1.156 tablecmds.c
*** src/backend/commands/tablecmds.c    6 May 2005 17:24:53 -0000    1.156
--- src/backend/commands/tablecmds.c    10 May 2005 00:26:51 -0000
***************
*** 5464,5470 ****
                          indexName, RelationGetRelationName(rel))));

      /* Check index is valid to cluster on */
!     check_index_is_clusterable(rel, indexOid);

      /* And do the work */
      mark_index_clustered(rel, indexOid);
--- 5464,5470 ----
                          indexName, RelationGetRelationName(rel))));

      /* Check index is valid to cluster on */
!     check_index_is_clusterable(rel, indexOid, false);

      /* And do the work */
      mark_index_clustered(rel, indexOid);
Index: src/include/commands/cluster.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/commands/cluster.h,v
retrieving revision 1.27
diff -c -c -r1.27 cluster.h
*** src/include/commands/cluster.h    31 Dec 2004 22:03:28 -0000    1.27
--- src/include/commands/cluster.h    10 May 2005 00:26:53 -0000
***************
*** 19,25 ****

  extern void cluster(ClusterStmt *stmt);

! extern void check_index_is_clusterable(Relation OldHeap, Oid indexOid);
  extern void mark_index_clustered(Relation rel, Oid indexOid);
  extern Oid make_new_heap(Oid OIDOldHeap, const char *NewName,
                Oid NewTableSpace);
--- 19,26 ----

  extern void cluster(ClusterStmt *stmt);

! extern void check_index_is_clusterable(Relation OldHeap, Oid indexOid,
!               bool recheck);
  extern void mark_index_clustered(Relation rel, Oid indexOid);
  extern Oid make_new_heap(Oid OIDOldHeap, const char *NewName,
                Oid NewTableSpace);