Re: REINDEX backend filtering

On Mon, Mar 15, 2021 at 08:56:00AM +0900, Michael Paquier wrote:
> On Sun, Mar 14, 2021 at 10:57:37PM +0800, Julien Rouhaud wrote:
> >
> > Is there really a use case for reindexing a specific index and at the same time
> > asking for possibly ignoring it? I think we should just forbid REINDEX
> > (OUTDATED) INDEX. What do you think?
>
> I think that there would be cases to be able to handle that, say if a
> user wants to works on a specific set of indexes one-by-one.

If a user want to work on a specific set of indexes one at a time, then the
list of indexes is probably already retrieved from some SQL query and there's
already all necessary infrastructure to properly filter the non oudated
indexes.

> There is
> also the argument of inconsistency with the other commands.

Yes, but the other commands dynamically construct a list of indexes.

The only use case I see would be to process a partitioned index if some of the
underlying indexes have already been processed. IMO this is better addressed
by REINDEX TABLE.

Anyway I'll make REINDEX (OUTDATED) INDEX to maybe reindex the explicitely
stated index name since you think it's a better behavior.

>
> > I was thinking that users would be more interested in the list of indexes being
> > processed rather than the full list of indexes and a mention of whether it was
> > processed or not. I can change that if you prefer.
>
> How invasive do you think it would be to add a note in the verbose
> output when indexes are skipped?

Probably not too invasive, but the verbose output is already inconsistent:

# reindex (verbose) table tt;
NOTICE: 00000: table "tt" has no indexes to reindex

But a REINDEX (VERBOSE) DATABASE won't emit such message. I'm assuming that
it's because it doesn't make sense to warn in that case as the user didn't
explicitly specified the table name. We have the same behavior for now when
using the OUTDATED option if no indexes are processed. Should that be changed
too?

> > Did you mean index_has_outdated_collation() and
> > index_has_outdated_dependency()? It's just to keep things separated, mostly
> > for future improvements on that infrastructure. I can get rid of that function
> > and put back the code in index_has_outadted_dependency() if that's overkill.
>
> Yes, sorry. I meant index_has_outdated_collation() and
> index_has_outdated_dependency().

And are you ok with this function?