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

Hello,

I realized there were some white spaces in the diff and a compiler warning
error from CI, so I have fixed those and the updated patch with v2 is now
attached.

Shayon

On Sun, Sep 22, 2024 at 1:42 PM Shayon Mukherjee <shayonj(at)gmail(dot)com> wrote:

> Hello,
>
> Thank you for all the feedback and insights. Work was busy, so I didn't
> get to follow up earlier.
>
> This patch introduces the ability to enable or disable indexes using ALTER
> INDEX
> and CREATE INDEX commands.
>
> Original motivation for the problem and proposal for a patch
> can be found here[0]
>
> This patch contains the relevant implementation details, new regression
> tests and documentation.
> It passes all the existing specs and the newly added regression tests. It
> compiles, so the
> patch can be applied for testing as well.
>
> I have attached the patch in this email, and have also shared it on my
> Github fork[1]. Mostly so
> that I can ensure the full CI passes.
>
>
> *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.
>
> - ALTER INDEX ... ENABLE/DISABLE performs an in-place update of the
> pg_index
> catalog to protect against indcheckxmin.
>
> - pg_get_indexdef() support for 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.
>
> - Modified get_index_paths() and build_index_paths() to exclude disabled
> indexes from consideration during query planning.
>
> - No changes are made to stop the index from getting rebuilt. This way we
> ensure no
> data miss or corruption when index is re-enabled.
>
> - TOAST indexes are supported and enabled by default.
>
> - REINDEX CONCURRENTLY is supported as well and the existing state of
> pg_index.indisenabled
> is carried over accordingly.
>
> - catversion.h is updated with a new CATALOG_VERSION_NO to reflect change
> in pg_index
> schema.
>
> - 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).
>
> I am looking forward to any and all feedback on this patch, including but
> not limited to
> code quality, tests, and fundamental logic.
>
> Thank you for the reviews and feedback.
>
> [0]
> https://www.postgresql.org/message-id/CANqtF-oXKe0M%3D0QOih6H%2BsZRjE2BWAbkW_1%2B9nMEAMLxUJg5jA%40mail.gmail.com
> [1] https://github.com/shayonj/postgres/pull/1
>
> Best,
> Shayon
>
> On Tue, Sep 10, 2024 at 5:35 PM David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
>
>> On Wed, 11 Sept 2024 at 03:12, Nathan Bossart <nathandbossart(at)gmail(dot)com>
>> wrote:
>> >
>> > On Tue, Sep 10, 2024 at 10:16:34AM +1200, David Rowley wrote:
>> > > If we get the skip scan feature for PG18, then there's likely going to
>> > > be lots of people with indexes that they might want to consider
>> > > removing after upgrading. Maybe this is a good time to consider this
>> > > feature as it possibly won't ever be more useful than it will be after
>> > > we get skip scans.
>> >
>> > +1, this is something I've wanted for some time. There was some past
>> > discussion, too [0].
>> >
>> > [0]
>> https://postgr.es/m/flat/ed8c9ed7-bb5d-aaec-065b-ad4893645deb%402ndQuadrant.com
>>
>> Thanks for digging that up. I'd forgotten about that. I see there was
>> pushback from having this last time, which is now over 6 years ago.
>> In the meantime, we still have nothing to make this easy for people.
>>
>> I think the most important point I read in that thread is [1]. Maybe
>> what I mentioned in [2] is a good workaround.
>>
>> Additionally, I think there will need to be syntax in CREATE INDEX for
>> this. Without that pg_get_indexdef() might return SQL that does not
>> reflect the current state of the index. MySQL seems to use "CREATE
>> INDEX name ON table (col) [VISIBLE|INVISIBLE]".
>>
>> David
>>
>> [1]
>> https://www.postgresql.org/message-id/20180618215635.m5vrnxdxhxytvmcm%40alap3.anarazel.de
>> [2]
>> https://www.postgresql.org/message-id/CAKJS1f_L7y_BTGESp5Qd6BSRHXP0mj3x9O9C_U27GU478UwpBw%40mail.gmail.com
>>
>
>
> --
> Kind Regards,
> Shayon Mukherjee
>

--
Kind Regards,
Shayon Mukherjee