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)