On 2020-04-06 21:44, Justin Pryzby wrote:
> On Mon, Apr 06, 2020 at 08:43:46PM +0300, Alexey Kondratov wrote:
>>
>> +/* XXX: reusing reindex_option_list */
>> + | CLUSTER opt_verbose '(' reindex_option_list ')' qualified_name
>> cluster_index_specification
>>
>> Could we actually simply reuse vac_analyze_option_list? From the first
>> sight
>> it does just the right thing, excepting the special handling of
>> spelling
>> ANALYZE/ANALYSE, but it does not seem to be a problem.
>
> Hm, do you mean to let cluster.c reject the other options like
> "analyze" ?
> I'm not sure why that would be better than reusing reindex?
> I think the suggestion will probably be to just copy+paste the reindex
> option
> list and rename it to cluster (possibly with the explanation that
> they're
> separate and independant and so their behavior shouldn't be tied
> together).
>
I mean to literally use vac_analyze_option_list for reindex and cluster
as well. Please, check attached 0007. Now, vacuum, reindex and cluster
filter options list and reject everything that is not supported, so it
seems completely fine to just reuse vac_analyze_option_list, doesn't it?
>>
>> ReindexRelationConcurrently is used for all cases, but it hits
>> different
>> code paths in the case of database, table and index. I have not
>> checked yet,
>> but are you sure it is safe removing these validations in the case of
>> REINDEX CONCURRENTLY?
>
> You're right about the pg_global case, fixed. System catalogs can't be
> reindexed CONCURRENTLY, so they're already caught by that check.
>
>> > XXX: for cluster/vacuum, it might be more friendly to check before
>> > clustering
>> > the table, rather than after clustering and re-indexing.
>>
>> Yes, I think it would be much more user-friendly.
>
> I realized it's not needed or useful to check indexes in advance of
> clustering,
> since 1) a mapped index will be on a mapped relation, which is already
> checked;
> 2) a system index will be on a system relation. Right ?
>
Yes, it seems that you are right. I have tried to create user index on
system relation with allow_system_table_mods=1, but this new index
appeared to become system as well. That way, we do not have to check
indexes in advance.
--
Alexey Kondratov
Postgres Professional https://www.postgrespro.com
Russian Postgres Company