Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX

hi.
The following is a review of version 17.

ATExecSetIndexVisibility
if (indexForm->indisvisible != visible)
{
indexForm->indisvisible = visible;
CatalogTupleUpdate(pg_index, &indexTuple->t_self, indexTuple);
CacheInvalidateRelcache(heapRel);
InvokeObjectPostAlterHook(IndexRelationId, indexOid, 0);
CommandCounterIncrement();
}
I am slightly confused. if we already used
CommandCounterIncrement();
then we don't need CacheInvalidateRelcache?

doc/src/sgml/ref/alter_index.sgml
<para>
<command>ALTER INDEX</command> changes the definition of an existing index.
There are several subforms described below. Note that the lock level required
may differ for each subform. An <literal>ACCESS EXCLUSIVE</literal>
lock is held
unless explicitly noted. When multiple subcommands are listed, the lock
held will be the strictest one required from any subcommand.

per the above para, we need mention that ALTER INDEX SET INVISIBLE|INVISIBLE
only use ShareUpdateExclusiveLock?

index_create is called in several places,
most of the time, we use INDEX_CREATE_VISIBLE.
if we define it as INDEX_CREATE_INVISIBLE rather than INDEX_CREATE_VISIBLE
then argument flags required code changes would be less, (i didn't try
it myself)

Similar to get_index_isclustered,
We can place GetIndexVisibility in
src/backend/utils/cache/lsyscache.c,
make it an extern function, so others can use it;
to align with other function names,
maybe rename it as get_index_visibility.

create index v2_idx on v1(data) visible;
is allowed,
doc/src/sgml/ref/create_index.sgml
<synopsis> section need to change to
[ VISIBLE | INVISIBLE ]

?