Re: CLUSTER not lose indexes

Tom Lane dijo:

> I think you probably want a CommandCounterIncrement at the bottom of the
> loop (after setRelhasindex).  If it works as-is it's just by chance,
> ie due to internal CCI calls in index_create.

Done.


> +         tuple = SearchSysCache(RELOID, ObjectIdGetDatum(attrs->indexOID),
> +                 0, 0, 0);
> +         if (!HeapTupleIsValid(tuple))
> +                 break;
>
> Breaking out of the loop hardly seems an appropriate response to this
> failure condition.  Not finding the index' pg_class entry is definitely
> an error.

Sure.  elog(ERROR) now.  I'm not sure what was I thinking when I wrote
that.

> I'd also suggest more-liberal commenting, as well as more attention to
> updating the existing comments to match new reality.

I'm afraid I cannot get too verbose no matter how hard I try.  I hope
this one is OK.

> In general, I'm not thrilled about expending more code on the existing
> fundamentally-broken implementation of CLUSTER.  We need to look at
> making use of the ability to write a new version of a table (or index)
> under a new relfilenode value, without changing the table's OID.
> However, some parts of your patch will probably still be needed when
> someone gets around to making that happen, so I won't object for now.

Will try to do this.

--
Alvaro Herrera (<alvherre[a]atentus.com>)
Licensee shall have no right to use the Licensed Software
for productive or commercial use. (Licencia de StarOffice 6.0 beta)