Re: [PATCHES] Dealing with CLUSTER failures
Christopher Kings-Lynne wrote: > Seems like an idea to me... > > On another note, what about the problem I pointed out where it's not > possible to drop the default on a serial column after you alter it to a > varchar, for example... Uh, should we allow a columns data type to be changed if it has a default, and if so, how? It seems a conversion should have to apply to the default value as well. -- 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 ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] Dealing with CLUSTER failures
Tom Lane wrote:
> Bruce Momjian 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 - 1.137
--- src/backend/commands/cluster.c 10 May 2005 00:26:48 -
***
*** 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)
{
RelationOldIndex;
--- 309,315
* definition can't change under us.
*/
void
! check_index_is_clusterable(Relation OldHeap, Oid indexOid, bool recheck)
{
RelationOldIndex;
***
*** 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
Re: [PATCHES] Dealing with CLUSTER failures
Bruce Momjian 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 regards, tom lane ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [PATCHES] Dealing with CLUSTER failures
Seems like an idea to me... On another note, what about the problem I pointed out where it's not possible to drop the default on a serial column after you alter it to a varchar, for example... Chris On Sat, 7 May 2005, Bruce Momjian wrote: Christopher Kings-Lynne wrote: I don't think that's a bug. You may not intend ever to cluster on that index again, and if you try it will tell you about the problem. Except it breaks the 'cluster everything' case: test=# cluster; ERROR: cannot cluster when index access method does not handle null values HINT: You may be able to work around this by marking column "a" NOT NULL. I looked over this item, originally posted as: http://archives.postgresql.org/pgsql-hackers/2005-03/msg01055.php It seems that if you use an index method that doesn't support NULLs, you can use ALTER to set the column as NOT NULL, then CLUSTER, and then set it to allow NULLs, and when you CLUSTER all tables, the cluster errors out on that table. I thought about removing the cluster bit when you do the alter or something like that, but it seems too confusing and error-prone to be sure we get every case. I think the main problem is that while cluster is a performance-only feature, we error out if we can't cluster one table, basically treating it as though it needs transaction semantics. It doesn't. This patch throws an ERROR of you cluster a specific index that can't be clustered, but issues only a WARNING if you are clustering all tables. This allows it to report the failed cluster but keep going. I also modified the code to print the index name in case of failure, because without that the user doesn't know the failing index name in a database-wide cluster failure. Here is an example: 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; WARNING: 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. CLUSTER You can see the ERROR for a specific index, and WARNING for full database cluster. -- 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 ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
