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 <shay...@gmail.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 <dgrowle...@gmail.com> wrote: > >> On Wed, 11 Sept 2024 at 03:12, Nathan Bossart <nathandboss...@gmail.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
v2-0001-Introduce-the-ability-to-enable-disable-indexes.patch
Description: Binary data
