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

Hello!

One more thing (maybe I missed it in the patch, but anyway) - should we
add some migration to ensure what old databases will get enabled=true by
default after upgrade?

Best regards,
Mikhail.

> Should this not behave like if you drop (or create) an index
> during a prepared statement? I have not yet looked closely at
> this code to see what could be done.
>
> Regards,

I looked at this a bit more and ATExecEnableDisableIndex
needs some tweaks.

What should be getting invalidated in the heap relation
that the index is on and not the index relation as it is in
the current patch.

You can retrieve the heap relation oid
IndexGetRelation(indexOid, false) and the
CacheInvalidateRelcache should be on the heap relation.

The planner needs to only care about the heap relation
invalidation to re-plan across multiple executions of
a prepared statement.

There should be a test for this scenario as well.

Regards,

Sami

+       This is the
+      default state for newly created indexes.

This is not needed in the ALTER INDEX docs, IMO.ss

+     <para>
+      Disable the specified index. A disabled index is not used for
queries, but it
+      is still updated when the underlying table data changes and will still be
+      used to enforce constraints (such as UNIQUE, or PRIMARY KEY constraints).
+      This can be useful for testing query performance with and
without specific
+      indexes. If performance degrades after disabling an index, it
can be easily
+      re-enabled using <literal>ENABLE</literal>. Before disabling,
it's recommended
+      to check
<structname>pg_stat_user_indexes</structname>.<structfield>idx_scan</structfield>
+      to identify potentially unused indexes.
+     </para>

This got me thinking if dropping the index is the only
use case we really care about. For example, you may want
to prevent an index that is enforcing a constraint from
being used by the planner, but you probably don't want to
drop it. In fact, I also think that you may want the index
from being used in one part of your application but could
potentially benefit other parts of your application. In that
case, I can see a GUC that allows you to force the use of a
an index that has been CREATED or ALTERED as DISABLED.
UNlike the GUC suggested earlier in the thread, this GUC
can simply be a boolean to allow the force usage of a
DISABLED index. FWIW, Oracle has a similar parameter called
OPTIMIZER_USE_INVISIBLE_INDEXES.

+        underlying table data changes. This can be useful when you
want to create
+        an index without immediately impacting query performance,
allowing you to

c/performance/planning ??

I have also been thinking about DISABLE as the keyword,
and I really don't like it. DISABLE indicates, at least ot me,
that the index is not available for either reads or writes.

Looking at other engines, Sqlserver uses DISABLE to drop
the index data, but keeps the index metadata around.

Oracle uses INVISIBLE and MariabDB uses IGNORABLE to
provide similar functionality to that being discussed here. I
find those keywords to be more appropriate for this purpose.

What about if we use HIDDEN instead of DISABLE as the keyword?

Regards,

Sami

I did some additional testing with this command within transactions.

I had the "BEGIN; ALTER INDEX; EXPLAIN; ROLLBACK;" scenario in mind, but
didn't realise we acquire an AccessExclusiveLock on the index. Therefore, it's
not possible to change the visibility within a single transaction....
unless you don’t mind blocking all access to the relation.

I read the comments at the top of "AlterTableGetLockLevel" and in the
documentation and I understand that this behavior seems unavoidable.
I suppose this is what was meant by the "globally visible effects" of an ALTER
INDEX in the old discussion ? [1]

Being able to rollback the changes is nice, but in this case there is
not much to alter
back anyway. This is probably not the intended use case (hence the
discussion about
GUCs and hints).

[1] https://www.postgresql.org/message-id/30558.1529359929%40sss.pgh.pa.us

I went back to look at this patch and a few things. I noticed it did
not have correct
indentation, so I ran pgindent. I also removed some extra lines added and made
some slight adjustments to the docs. Attached my edited patch as a txt. If you
agree, please revise into a v14.

I also noticed that between v12 and v13, the GUC use_invisible_index
was removed,
but I don't see a discussion as to why. I feel it's a good GUC to have
[0], and we should
at least have it as a separate patch as part of this set.

I will continue reviewing the patch, but i feel this may be close to
be marked RFC, although
not sure if it will get a committer review before code freeze.

[0]
https://www.postgresql.org/message-id/flat/CAA5RZ0udzydObMDi65C59-oq54B9ZmjSZ1wVH3h%2Bv4XiVm6QDA%40mail.gmail.com#cfea240ffd73e947f9edd1ef1c762dae


--
Sami Imseih
Amazon Web Services (AWS)