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

> On Oct 15, 2024, at 7:25 PM, David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
>
> On Wed, 16 Oct 2024 at 03:40, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>>
>> On Sat, Oct 12, 2024 at 5:56 AM Shayon Mukherjee <shayonj(at)gmail(dot)com> wrote:
>>> Thank you for sharing this Robert. I like the idea behind "allowing extensions to control planner behavior” overall and I think it does help towards a powerful extension ecosystem too. I wonder if there is a reality where we can achieve both the outcomes here
>>>
>>> - Support disabling of indexes [1] through ALTER command
>>> - While also building on "allowing extensions to control planner behavior” for the reasons above?
>>>
>>> [1] https://www.postgresql.org/message-id/ABD42A12-4DCF-4EE4-B903-4C657903CECE%40gmail.com
>>
>> Yes, I think we can do both things.
>
> I think so too. I imagine there'd be cases where even hints global to
> all queries running on the server wouldn't result in the index being
> completely disabled. For example, a physical replica might not be
> privy to the hints defined on the primary and it might just be the
> queries running on the physical replica that are getting the most use
> out of the given index. Having the change made in pg_index would mean
> physical replicas have the index disabled too. For the primary use
> case I have in mind (test disabling indexes you're considering
> dropping), having the disabledness replicate would be very useful.
>

+1 and I agree.

That said - Thank you everyone for the discussions and pointers. I now have a new patch that introduces the ability to enable or disable indexes using ALTER INDEX and CREATE INDEX commands, and updating get_relation_info in plancat.c to skip disabled indexes entirely by baking in the concept into IndexOptInfo structure. Below are all the relevant details.

Original motivation for the problem and proposal for a patch can be found at [1].

This patch passes all the existing specs and the newly added regression tests. The patch is ready for review and test. It compiles, so the can patch can be applied for testing as well.

Implementation details:

- New Grammar:
- ALTER INDEX ... ENABLE/DISABLE
- CREATE INDEX ... DISABLE

- Default state is enabled. Indexes are only disabled when explicitly
instructed via CREATE INDEX ... DISABLE or ALTER INDEX ... DISABLE.

- Primary Key and Unique constraint indexes are always enabled as well. The
ENABLE/DISABLE grammar is not supported for these types of indexes. They can
be later disabled via ALTER INDEX ... ENABLE/DISABLE if needed.

- ALTER INDEX ... ENABLE/DISABLE performs an in-place update of the pg_index
catalog to protect against indcheckxmin [2] (older unrelated thread).

- pg_get_indexdef() support the new functionality and grammar. This change is
reflected in \d output for tables and pg_dump. We show the DISABLE syntax accordingly.

- Updated create_index.sql regression test to cover the new grammar and verify
that disabled indexes are not used in queries. The test CATALOG_VERSION_NO

- Basic single-column and multi-column indexes
- Partial indexes
- Expression indexes
- Join indexes
- GIN and GiST indexes
- Covering indexes
- Range indexes
- Unique indexes and constraints

- Adds a new enabled attribute to the IndexOptInfo structure.

- Modifies get_relation_info in plancat.c to skip disabled indexes entirely, thus reducing the number of places we need to check if an index is disabled or not. Inspired by the conversations start at [3].
- I chose to modify the logic within get_relation_info as compared to, say, reducing the cost to make the planner not consider an index during planning, mostly to keep the number of changes being introduced to a minimum and also the logic itself being self-contained and easier to under perhaps (?).

- No changes are made to stop the index from getting rebuilt. This way we ensure no
data miss or corruption when index is re-nabled.

- TOAST indexes are supported and enabled by default as well.

- REINDEX CONCURRENTLY is supported as well and existing state of pg_index.indisenabled
is carried over accordingly.

- See the changes in create_index.sql to get an idea of the grammar and sql statements.

- See the changes in create_index.out to get an idea of the catalogue states and EXPLAIN
output to see when an index is getting used or isn't (when disabled).

- Incorporated DavidR's feedback from [4] around documentation and also you will see that by skip disabled indexes entirely from get_relation_info in plancat.c (as mentioned above), we address the other mentioned issues as well.

Looking forward to any and all feedback on this patch, including but not limited to code quality, tests, fundamental logic.

[1] https://www.postgresql.org/message-id/CANqtF-oXKe0M%3D0QOih6H%2BsZRjE2BWAbkW_1%2B9nMEAMLxUJg5jA%40mail.gmail.com
[2] https://www.postgresql.org/message-id/20180618215635.m5vrnxdxhxytvmcm@alap3.anarazel.de
[3] https://www.postgresql.org/message-id/3465209.1727202064%40sss.pgh.pa.us
[4] https://www.postgresql.org/message-id/CAApHDvpUNu%3DiVcdJ74sypvgeaCF%2Btfpyb8VRhZaF7DTd1xVr7g%40mail.gmail.com

Thanks
Shayon