Re: REINDEX filtering in the backend

On Thu, Aug 29, 2019 at 2:09 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Wed, Aug 28, 2019 at 10:22:07AM +0200, Julien Rouhaud wrote:
> >>> The filtering is done at table level (with and without the
> >>> concurrently option), so SCHEMA, DATABASE and SYSTEM automatically
> >>> benefit from it. If this clause is used with a REINDEX INDEX, the
> >>> statement errors out, as I don't see a valid use case for providing a
> >>> single index name and asking to possibly filter it at the same time.
> >>
> >> Supporting that case would not be that much amount of work, no?
> >
> > Probably not, but I'm dubious about the use case. Adding the lib
> > version in the catalog would be more useful for people who want to
> > write their own rules to reindex specific set of indexes.
>
> Hearing from others here would be helpful. My take is that having a
> simple option doing the filtering, without the need to store anything
> in the catalogs, would be helpful enough for users mixing both index
> types on a single table. Others may not agree.

That was already suggested by Thomas and seconded by Peter E., see
https://www.postgresql.org/message-id/2b1504ac-3d6c-11ec-e1ce-3daf132b3d37%402ndquadrant.com.

I personally think that it's a sensible approach, and I'll be happy to
propose a patch for that too if no one worked on this yet.

> >> I think that it would be nice to be
> >> able to do both, generating an error for REINDEX INDEX if the index
> >> specified is not compatible, and a LOG if the index is not filtered
> >> out when a list is processed.
> >
> > Do you mean having an error if the index does not contain any
> > collation based type? Also, REINDEX only accept a single name, so
> > there shouldn't be any list processing for REINDEX INDEX? I'm not
> > really in favour of adding extra code the filtering when user asks for
> > a specific index name to be reindexed.
>
> I was referring to adding an error if trying to reindex an index with
> the filtering enabled. That's useful to inform the user that what he
> intends to do is not compatible with the options provided.

Ok, I can add it if needed.

> > That's what I did when I first submitted the feature in reindexdb. I
> > didn't use them because it means switching to TAP tests. I can drop
> > the simple regression test (especially since I now realize than one is
> > quite broken) and use the TAP one if, or should I keep both?
>
> There is no need for TAP I think. You could for example store the
> relid and its relfilenode in a temporary table before running the
> reindex, run the REINDEX and then compare with what pg_class stores.
> And that's much cheaper than setting a new instance for a TAP test.

Oh indeed, good point! I'll work on better tests using this approach then.