Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

hi.
the following reviews based on
v10-0001-Introduce-the-ability-to-set-index-visibility-us.patch.

in src/test/regress/sql/create_index.sql
seems there are no sql tests for "create index ... invisible"?

<varlistentry>
<term><literal>VISIBLE</literal></term>
<listitem>
<para>
Make the specified index visible. The index will be used for queries.
</para>
</listitem>
</varlistentry>
here it should be
"Make the specified index visible. The index can be used for query planning"
?

Do we need to add GUC use_invisible_index to postgresql.conf.sample?

CREATE TABLE t(id INT PRIMARY KEY, data TEXT,num INT, vector INT[],
range INT4RANGE);
ALTER INDEX t_pkey INVISIBLE;
alter table t alter column id set data type bigint;
\d t

after ALTER TABLE SET DATA TYPE, the "visible" status should not change?
but here it changed.
you may check ATPostAlterTypeParse to make the "visible" status not change.

@@ -3449,6 +3451,7 @@ typedef struct IndexStmt
bool if_not_exists; /* just do nothing if index already
exists? */
bool reset_default_tblspc; /* reset default_tablespace prior to
* executing */
+ bool isvisible; /* true if VISIBLE (default), false
if INVISIBLE */
} IndexStmt;
the indentation level is not right?

+opt_index_visibility:
+ VISIBLE_P { $$ = true; }
+ | INVISIBLE_P { $$ = false; }
+ | /*EMPTY*/ { $$ = true; }
+ ;
+
the indentation level seems also not right?

+ createFlags = INDEX_CREATE_SKIP_BUILD | INDEX_CREATE_CONCURRENT;
+ if (indexForm->indisvisible)
+ createFlags |= INDEX_CREATE_VISIBLE;
the indentation level seems also not right?

INVISIBLE, VISIBLE is not special words, in gram.y, you don't need
"VISIBLE_P", "INVISIBLE_P", you can just use "INVISIBLE", "VISIBLE"
?

\d t3
Table "public.t3"
Column | Type | Collation | Nullable | Default
--------+-----------+-----------+----------+---------
id | integer | | not null |
data | text | | |
num | integer | | |
vector | integer[] | | |
range | int4range | | |
a | box | | |
Indexes:
"t3_pkey" PRIMARY KEY, btree (id) INVISIBLE
"grect2ind" gist (a) INVISIBLE
"t3_1" gist (a) INVISIBLE
"t3_2" gin (vector) WITH (fastupdate='on',
gin_pending_list_limit='128') INVISIBLE
"t3_4" spgist (data) INVISIBLE
"t3_6" hash (id) INVISIBLE

pg_dump will dump as
--
-- Name: t3 t3_pkey; Type: CONSTRAINT; Schema: public; Owner: jian
--
ALTER TABLE ONLY public.t3
ADD CONSTRAINT t3_pkey PRIMARY KEY (id);

after dump, restore index (primary key: t3_pkey) INVISIBLE will not be restored.
We need extra work for restoring the INVISIBLE flag for the primary key index.

I am not sure if we need to change index_concurrently_swap or not.
but many other pg_index columns changed.