Thread

  1. Proposal to Enable/Disable Index using ALTER INDEX

    Shayon Mukherjee <shayonj@gmail.com> — 2024-09-09T21:38:35Z

    Hello hackers,
    
    This is my first time posting here, and I’d like to propose a new feature
    related to PostgreSQL indexes. If this idea resonates, I’d be happy to
    follow up with a patch as well.
    
    *Problem*:
    Adding and removing indexes is a common operation in PostgreSQL. On larger
    databases, however, these operations can be resource-intensive. When
    evaluating the performance impact of one or more indexes, dropping them
    might not be ideal since as a user you may want a quicker way to test their
    effects without fully committing to removing & adding them back again.
    Which can be a time taking operation on larger tables.
    
    *Proposal*:
    I propose adding an ALTER INDEX command that allows for enabling or
    disabling an index globally. This could look something like:
    
    ALTER INDEX index_name ENABLE;
    ALTER INDEX index_name DISABLE;
    
    A disabled index would still receive updates and enforce constraints as
    usual but would not be used for queries. This allows users to assess
    whether an index impacts query performance before deciding to drop it
    entirely.
    
    *Implementation*:
    To keep this simple, I suggest toggling the indisvalid flag in pg_index
    during the enable/disable operation.
    
    *Additional Considerations*:
    - Keeping the index up-to-date while it’s disabled seems preferable, as it
    avoids the need to rebuild the index if it’s re-enabled later. The
    alternative would be dropping and rebuilding the index upon re-enabling,
    which I believe would introduce additional overhead in terms of application
    logic & complexity.
    - I am also proposing to reuse the existing indisvalid flag to avoid adding
    new state and the maintenance that comes with it, but I’m open to feedback
    if this approach has potential downsides.
    - To keep the scope minimal for now, I propose that we only allow enabling
    and disabling indexes globally, and not locally, by supporting it
    exclusively in ALTER INDEX. I would love to know if this would break any
    SQL grammar promises that I might be unaware of.
    
    I would love to learn if this sounds like a good idea and how it can be
    improved further. Accordingly, as a next step I would be very happy to
    propose a patch as well.
    
    Best regards,
    Shayon Mukherjee
    
  2. Re: Proposal to Enable/Disable Index using ALTER INDEX

    David Rowley <dgrowleyml@gmail.com> — 2024-09-09T22:16:34Z

    On Tue, 10 Sept 2024 at 09:39, Shayon Mukherjee <shayonj@gmail.com> wrote:
    > Adding and removing indexes is a common operation in PostgreSQL. On larger databases, however, these operations can be resource-intensive. When evaluating the performance impact of one or more indexes, dropping them might not be ideal since as a user you may want a quicker way to test their effects without fully committing to removing & adding them back again. Which can be a time taking operation on larger tables.
    >
    > Proposal:
    > I propose adding an ALTER INDEX command that allows for enabling or disabling an index globally. This could look something like:
    >
    > ALTER INDEX index_name ENABLE;
    > ALTER INDEX index_name DISABLE;
    >
    > A disabled index would still receive updates and enforce constraints as usual but would not be used for queries. This allows users to assess whether an index impacts query performance before deciding to drop it entirely.
    
    I personally think having some way to alter an index to stop it from
    being used in query plans would be very useful for the reasons you
    mentioned.  I don't have any arguments against the syntax you've
    proposed.  We'd certainly have to clearly document that constraints
    are still enforced. Perhaps there is some other syntax which would
    self-document slightly better. I just can't think of it right now.
    
    > Implementation:
    > To keep this simple, I suggest toggling the indisvalid flag in pg_index during the enable/disable operation.
    
    That's not a good idea as it would allow ALTER INDEX ... ENABLE; to be
    used to make valid a failed concurrently created index.  I think this
    would need a new flag and everywhere in the planner would need to be
    adjusted to ignore indexes when that flag is false.
    
    > Additional Considerations:
    > - Keeping the index up-to-date while it’s disabled seems preferable, as it avoids the need to rebuild the index if it’s re-enabled later. The alternative would be dropping and rebuilding the index upon re-enabling, which I believe would introduce additional overhead in terms of application logic & complexity.
    
    I think the primary use case here is to assist in dropping useless
    indexes in a way that can very quickly be undone if the index is more
    useful than thought. If you didn't keep the index up-to-date then that
    would make the feature useless for that purpose.
    
    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.
    
    David
    
    
    
    
  3. Re: Proposal to Enable/Disable Index using ALTER INDEX

    wenhui qiu <qiuwenhuifx@gmail.com> — 2024-09-10T10:06:47Z

    Hi Shayon
       Thank you for  your work on this , I think it's great to have this
    feature implemented ,I checked the doucment  on other databases,It seems
    both MySQL 8.0  and oracle supports it, sql server need to rebuild indexes
    after  disabled,It seems  disable the index, it's equivalent to deleting
    the index, except that the index's metadata is still retained:
    https://docs.oracle.com/cd/E17952_01/mysql-8.0-en/invisible-indexes.html
    https://learn.microsoft.com/en-us/sql/t-sql/statements/alter-index-transact-sql?view=sql-server-ver16
    https://docs.oracle.com/en/database/oracle/oracle-database/19/sqlrf/ALTER-INDEX.html
    ->A disabled index would still receive updates and enforce constraints as
    usual but would not be used for queries. This allows users to assess ->
    ->whether an index impacts query performance before deciding to drop it
    entirely.
    MySQL 8.0 and oracle settings are not visible, index information is always
    updated, I would then suggest that the statement be changed to set the
    index invisible and visible.
    
    
    
    Thanks
    
    David Rowley <dgrowleyml@gmail.com> 于2024年9月10日周二 06:17写道:
    
    > On Tue, 10 Sept 2024 at 09:39, Shayon Mukherjee <shayonj@gmail.com> wrote:
    > > Adding and removing indexes is a common operation in PostgreSQL. On
    > larger databases, however, these operations can be resource-intensive. When
    > evaluating the performance impact of one or more indexes, dropping them
    > might not be ideal since as a user you may want a quicker way to test their
    > effects without fully committing to removing & adding them back again.
    > Which can be a time taking operation on larger tables.
    > >
    > > Proposal:
    > > I propose adding an ALTER INDEX command that allows for enabling or
    > disabling an index globally. This could look something like:
    > >
    > > ALTER INDEX index_name ENABLE;
    > > ALTER INDEX index_name DISABLE;
    > >
    > > A disabled index would still receive updates and enforce constraints as
    > usual but would not be used for queries. This allows users to assess
    > whether an index impacts query performance before deciding to drop it
    > entirely.
    >
    > I personally think having some way to alter an index to stop it from
    > being used in query plans would be very useful for the reasons you
    > mentioned.  I don't have any arguments against the syntax you've
    > proposed.  We'd certainly have to clearly document that constraints
    > are still enforced. Perhaps there is some other syntax which would
    > self-document slightly better. I just can't think of it right now.
    >
    > > Implementation:
    > > To keep this simple, I suggest toggling the indisvalid flag in pg_index
    > during the enable/disable operation.
    >
    > That's not a good idea as it would allow ALTER INDEX ... ENABLE; to be
    > used to make valid a failed concurrently created index.  I think this
    > would need a new flag and everywhere in the planner would need to be
    > adjusted to ignore indexes when that flag is false.
    >
    > > Additional Considerations:
    > > - Keeping the index up-to-date while it’s disabled seems preferable, as
    > it avoids the need to rebuild the index if it’s re-enabled later. The
    > alternative would be dropping and rebuilding the index upon re-enabling,
    > which I believe would introduce additional overhead in terms of application
    > logic & complexity.
    >
    > I think the primary use case here is to assist in dropping useless
    > indexes in a way that can very quickly be undone if the index is more
    > useful than thought. If you didn't keep the index up-to-date then that
    > would make the feature useless for that purpose.
    >
    > 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.
    >
    > David
    >
    >
    >
    
  4. Re: Proposal to Enable/Disable Index using ALTER INDEX

    Michael Banck <mbanck@gmx.net> — 2024-09-10T10:46:15Z

    Hi,
    
    On Tue, Sep 10, 2024 at 10:16:34AM +1200, David Rowley wrote:
    > On Tue, 10 Sept 2024 at 09:39, Shayon Mukherjee <shayonj@gmail.com> wrote:
    > > Adding and removing indexes is a common operation in PostgreSQL. On
    > > larger databases, however, these operations can be
    > > resource-intensive. When evaluating the performance impact of one or
    > > more indexes, dropping them might not be ideal since as a user you
    > > may want a quicker way to test their effects without fully
    > > committing to removing & adding them back again. Which can be a time
    > > taking operation on larger tables.
    > >
    > > Proposal:
    > > I propose adding an ALTER INDEX command that allows for enabling or disabling an index globally. This could look something like:
    > >
    > > ALTER INDEX index_name ENABLE;
    > > ALTER INDEX index_name DISABLE;
    > >
    > > A disabled index would still receive updates and enforce constraints
    > > as usual but would not be used for queries. This allows users to
    > > assess whether an index impacts query performance before deciding to
    > > drop it entirely.
    > 
    > I personally think having some way to alter an index to stop it from
    > being used in query plans would be very useful for the reasons you
    > mentioned.  I don't have any arguments against the syntax you've
    > proposed.  We'd certainly have to clearly document that constraints
    > are still enforced. Perhaps there is some other syntax which would
    > self-document slightly better. I just can't think of it right now.
    > 
    > > Implementation:
    > > To keep this simple, I suggest toggling the indisvalid flag in
    > > pg_index during the enable/disable operation.
    > 
    > That's not a good idea as it would allow ALTER INDEX ... ENABLE; to be
    > used to make valid a failed concurrently created index.  I think this
    > would need a new flag and everywhere in the planner would need to be
    > adjusted to ignore indexes when that flag is false.
    
    How about the indislive flag instead? I haven't looked at the code, but
    from the documentation ("If false, the index is in process of being
    dropped, and
    should be ignored for all purposes") it sounds like we made be able to
    piggy-back on that instead?
    
    
    Michael
    
    
    
    
  5. Re: Proposal to Enable/Disable Index using ALTER INDEX

    David Rowley <dgrowleyml@gmail.com> — 2024-09-10T12:02:34Z

    On Tue, 10 Sept 2024 at 22:46, Michael Banck <mbanck@gmx.net> wrote:
    > How about the indislive flag instead? I haven't looked at the code, but
    > from the documentation ("If false, the index is in process of being
    > dropped, and
    > should be ignored for all purposes") it sounds like we made be able to
    > piggy-back on that instead?
    
    Doing that could cause an UPDATE which would ordinarily not be
    eligible for a HOT-update to become a HOT-update. That would cause
    issues if the index is enabled again as the index wouldn't have been
    updated during the UPDATE.
    
    I don't see the big deal with adding a new flag. There's even a free
    padding byte to put this flag in after indisreplident, so we don't
    have to worry about using more memory.
    
    David
    
    
    
    
  6. Re: Proposal to Enable/Disable Index using ALTER INDEX

    Shayon Mukherjee <shayonj@gmail.com> — 2024-09-10T12:15:19Z

    +1 for the new flag as well, since it'd be nice to be able to
    enable/disable indexes without having to worry about the missed updates /
    having to rebuild it.
    Shayon
    
    On Tue, Sep 10, 2024 at 8:02 AM David Rowley <dgrowleyml@gmail.com> wrote:
    
    > On Tue, 10 Sept 2024 at 22:46, Michael Banck <mbanck@gmx.net> wrote:
    > > How about the indislive flag instead? I haven't looked at the code, but
    > > from the documentation ("If false, the index is in process of being
    > > dropped, and
    > > should be ignored for all purposes") it sounds like we made be able to
    > > piggy-back on that instead?
    >
    > Doing that could cause an UPDATE which would ordinarily not be
    > eligible for a HOT-update to become a HOT-update. That would cause
    > issues if the index is enabled again as the index wouldn't have been
    > updated during the UPDATE.
    >
    > I don't see the big deal with adding a new flag. There's even a free
    > padding byte to put this flag in after indisreplident, so we don't
    > have to worry about using more memory.
    >
    > David
    >
    
    
    -- 
    Kind Regards,
    Shayon Mukherjee
    
  7. Re: Proposal to Enable/Disable Index using ALTER INDEX

    Shayon Mukherjee <shayonj@gmail.com> — 2024-09-10T12:25:37Z

    Hello,
    
    Thank you for the detailed information and feedback David. Comments inline.
    
    P.S Re-sending it to the mailing list, because I accidentally didn't select
    reply-all on the last reply.
    
    On Mon, Sep 9, 2024 at 6:16 PM David Rowley <dgrowleyml@gmail.com> wrote:
    
    > On Tue, 10 Sept 2024 at 09:39, Shayon Mukherjee <shayonj@gmail.com> wrote:
    > > Adding and removing indexes is a common operation in PostgreSQL. On
    > larger databases, however, these operations can be resource-intensive. When
    > evaluating the performance impact of one or more indexes, dropping them
    > might not be ideal since as a user you may want a quicker way to test their
    > effects without fully committing to removing & adding them back again.
    > Which can be a time taking operation on larger tables.
    > >
    > > Proposal:
    > > I propose adding an ALTER INDEX command that allows for enabling or
    > disabling an index globally. This could look something like:
    > >
    > > ALTER INDEX index_name ENABLE;
    > > ALTER INDEX index_name DISABLE;
    > >
    > > A disabled index would still receive updates and enforce constraints as
    > usual but would not be used for queries. This allows users to assess
    > whether an index impacts query performance before deciding to drop it
    > entirely.
    >
    > I personally think having some way to alter an index to stop it from
    > being used in query plans would be very useful for the reasons you
    > mentioned.  I don't have any arguments against the syntax you've
    > proposed.  We'd certainly have to clearly document that constraints
    > are still enforced. Perhaps there is some other syntax which would
    > self-document slightly better. I just can't think of it right now.
    >
    
    Thank you and likewise. I was thinking of piggy backing off of VALID / NOT
    VALID, but that might have similar issues (if not more confusion) to the
    current proposed syntax. Will be sure to update the documentation.
    
    
    
    >
    > > Implementation:
    > > To keep this simple, I suggest toggling the indisvalid flag in pg_index
    > during the enable/disable operation.
    >
    > That's not a good idea as it would allow ALTER INDEX ... ENABLE; to be
    > used to make valid a failed concurrently created index.  I think this
    > would need a new flag and everywhere in the planner would need to be
    > adjusted to ignore indexes when that flag is false.
    >
    
    That is a great call and I wasn't thinking of the semantics with the
    existing usage of concurrently created indexes.
    
    
    >
    > > Additional Considerations:
    > > - Keeping the index up-to-date while it’s disabled seems preferable, as
    > it avoids the need to rebuild the index if it’s re-enabled later. The
    > alternative would be dropping and rebuilding the index upon re-enabling,
    > which I believe would introduce additional overhead in terms of application
    > logic & complexity.
    >
    > I think the primary use case here is to assist in dropping useless
    > indexes in a way that can very quickly be undone if the index is more
    > useful than thought. If you didn't keep the index up-to-date then that
    > would make the feature useless for that purpose.
    >
    
    +1
    
    
    >
    > 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.
    >
    > David
    >
    
    Thank you for the feedback again, I will look into the changes required and
    accordingly propose a PATCH.
    
    -- 
    Kind Regards,
    Shayon Mukherjee
    
  8. Re: Proposal to Enable/Disable Index using ALTER INDEX

    Nathan Bossart <nathandbossart@gmail.com> — 2024-09-10T15:12:22Z

    On Tue, Sep 10, 2024 at 10:16:34AM +1200, David Rowley wrote:
    > I think the primary use case here is to assist in dropping useless
    > indexes in a way that can very quickly be undone if the index is more
    > useful than thought. If you didn't keep the index up-to-date then that
    > would make the feature useless for that purpose.
    > 
    > 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
    
    -- 
    nathan
    
    
    
    
  9. Re: Proposal to Enable/Disable Index using ALTER INDEX

    David Rowley <dgrowleyml@gmail.com> — 2024-09-10T21:35:16Z

    On Wed, 11 Sept 2024 at 03:12, Nathan Bossart <nathandbossart@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
    
    
    
    
  10. [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX

    Shayon Mukherjee <shayonj@gmail.com> — 2024-09-22T17:42:48Z

    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@gmail.com> wrote:
    
    > On Wed, 11 Sept 2024 at 03:12, Nathan Bossart <nathandbossart@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
    
  11. Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX

    Shayon Mukherjee <shayonj@gmail.com> — 2024-09-22T18:20:53Z

    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@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 <dgrowleyml@gmail.com> wrote:
    >
    >> On Wed, 11 Sept 2024 at 03:12, Nathan Bossart <nathandbossart@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
    
  12. Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX

    David Rowley <dgrowleyml@gmail.com> — 2024-09-22T22:44:58Z

    On Mon, 23 Sept 2024 at 05:43, Shayon Mukherjee <shayonj@gmail.com> wrote:
    > - Modified get_index_paths() and build_index_paths() to exclude disabled
    >   indexes from consideration during query planning.
    
    There are quite a large number of other places you also need to modify.
    
    Here are 2 places where the index should be ignored but isn't:
    
    1. expression indexes seem to still be used for statistical estimations:
    
    create table b as select generate_series(1,1000)b;
    create index on b((b%10));
    analyze b;
    explain select distinct b%10 from b;
    -- HashAggregate  (cost=23.00..23.12 rows=10 width=4)
    
    alter index b_expr_idx disable;
    explain select distinct b%10 from b;
    -- HashAggregate  (cost=23.00..23.12 rows=10 width=4) <-- should be 1000 rows
    
    drop index b_expr_idx;
    explain select distinct b%10 from b;
    -- HashAggregate  (cost=23.00..35.50 rows=1000 width=4)
    
    2. Indexes seem to still be used for join removals.
    
    create table c (c int primary key);
    explain select c1.* from c c1 left join c c2 on c1.c=c2.c; --
    correctly removes join.
    alter index c_pkey disable;
    explain select c1.* from c c1 left join c c2 on c1.c=c2.c; -- should
    not remove join.
    
    Please carefully look over all places that RelOptInfo.indexlist is
    looked at and consider skipping disabled indexes. Please also take
    time to find SQL that exercises each of those places so you can verify
    that the behaviour is correct after your change. This is also a good
    way to learn exactly all cases where indexes are used. Using this
    method would have led you to find places like
    rel_supports_distinctness(), where you should be skipping disabled
    indexes.
    
    The planner should not be making use of disabled indexes for any
    optimisations at all.
    
    > - catversion.h is updated with a new CATALOG_VERSION_NO to reflect change in pg_index
    >   schema.
    
    Please leave that up to the committer. Patch authors doing this just
    results in the patch no longer applying as soon as someone commits a
    version bump.
    
    Also, please get rid of these notices. The command tag serves that
    purpose. It's not interesting that the index is already disabled.
    
    # alter index a_pkey disable;
    NOTICE:  index "a_pkey" is now disabled
    ALTER INDEX
    # alter index a_pkey disable;
    NOTICE:  index "a_pkey" is already disabled
    ALTER INDEX
    
    I've only given the code a very quick glance. I don't quite understand
    why you're checking the index is enabled in create_index_paths() and
    get_index_paths(). I think the check should be done only in
    create_index_paths().  Primarily, you'll find code such as "if
    (index->indpred != NIL && !index->predOK)" in the locations you need
    to consider skipping the disabled index. I think your new code should
    be located very close to those places or perhaps within the same if
    condition unless it makes it overly complex for the human reader.
    
    I think the documents should also mention that disabling an index is a
    useful way to verify an index is not being used before dropping it as
    the index can be enabled again at the first sign that performance has
    been effected. (It might also be good to mention that checking
    pg_stat_user_indexes.idx_scan should be the first port of call when
    checking for unused indexes)
    
    David
    
    
    
    
  13. Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX

    Shayon Mukherjee <shayonj@gmail.com> — 2024-09-23T11:07:11Z

    Hi David,
    
    Thank you so much for the review and pointers. I totally missed expression indexes. I am going to do another proper pass along with your feedback and follow up with an updated patch and any questions. 
    
    Excited to be learning so much about the internals. 
    Shayon
    
    > On Sep 22, 2024, at 6:44 PM, David Rowley <dgrowleyml@gmail.com> wrote:
    > 
    > On Mon, 23 Sept 2024 at 05:43, Shayon Mukherjee <shayonj@gmail.com> wrote:
    >> - Modified get_index_paths() and build_index_paths() to exclude disabled
    >>  indexes from consideration during query planning.
    > 
    > There are quite a large number of other places you also need to modify.
    > 
    > Here are 2 places where the index should be ignored but isn't:
    > 
    > 1. expression indexes seem to still be used for statistical estimations:
    > 
    > create table b as select generate_series(1,1000)b;
    > create index on b((b%10));
    > analyze b;
    > explain select distinct b%10 from b;
    > -- HashAggregate  (cost=23.00..23.12 rows=10 width=4)
    > 
    > alter index b_expr_idx disable;
    > explain select distinct b%10 from b;
    > -- HashAggregate  (cost=23.00..23.12 rows=10 width=4) <-- should be 1000 rows
    > 
    > drop index b_expr_idx;
    > explain select distinct b%10 from b;
    > -- HashAggregate  (cost=23.00..35.50 rows=1000 width=4)
    > 
    > 2. Indexes seem to still be used for join removals.
    > 
    > create table c (c int primary key);
    > explain select c1.* from c c1 left join c c2 on c1.c=c2.c; --
    > correctly removes join.
    > alter index c_pkey disable;
    > explain select c1.* from c c1 left join c c2 on c1.c=c2.c; -- should
    > not remove join.
    > 
    > Please carefully look over all places that RelOptInfo.indexlist is
    > looked at and consider skipping disabled indexes. Please also take
    > time to find SQL that exercises each of those places so you can verify
    > that the behaviour is correct after your change. This is also a good
    > way to learn exactly all cases where indexes are used. Using this
    > method would have led you to find places like
    > rel_supports_distinctness(), where you should be skipping disabled
    > indexes.
    > 
    > The planner should not be making use of disabled indexes for any
    > optimisations at all.
    > 
    >> - catversion.h is updated with a new CATALOG_VERSION_NO to reflect change in pg_index
    >>  schema.
    > 
    > Please leave that up to the committer. Patch authors doing this just
    > results in the patch no longer applying as soon as someone commits a
    > version bump.
    > 
    > Also, please get rid of these notices. The command tag serves that
    > purpose. It's not interesting that the index is already disabled.
    > 
    > # alter index a_pkey disable;
    > NOTICE:  index "a_pkey" is now disabled
    > ALTER INDEX
    > # alter index a_pkey disable;
    > NOTICE:  index "a_pkey" is already disabled
    > ALTER INDEX
    > 
    > I've only given the code a very quick glance. I don't quite understand
    > why you're checking the index is enabled in create_index_paths() and
    > get_index_paths(). I think the check should be done only in
    > create_index_paths().  Primarily, you'll find code such as "if
    > (index->indpred != NIL && !index->predOK)" in the locations you need
    > to consider skipping the disabled index. I think your new code should
    > be located very close to those places or perhaps within the same if
    > condition unless it makes it overly complex for the human reader.
    > 
    > I think the documents should also mention that disabling an index is a
    > useful way to verify an index is not being used before dropping it as
    > the index can be enabled again at the first sign that performance has
    > been effected. (It might also be good to mention that checking
    > pg_stat_user_indexes.idx_scan should be the first port of call when
    > checking for unused indexes)
    > 
    > David
    
    
    
    
    
  14. Re: Proposal to Enable/Disable Index using ALTER INDEX

    Peter Eisentraut <peter@eisentraut.org> — 2024-09-23T15:14:15Z

    On 09.09.24 23:38, Shayon Mukherjee wrote:
    > *Problem*:
    > Adding and removing indexes is a common operation in PostgreSQL. On 
    > larger databases, however, these operations can be resource-intensive. 
    > When evaluating the performance impact of one or more indexes, dropping 
    > them might not be ideal since as a user you may want a quicker way to 
    > test their effects without fully committing to removing & adding them 
    > back again. Which can be a time taking operation on larger tables.
    > 
    > *Proposal*:
    > I propose adding an ALTER INDEX command that allows for enabling or 
    > disabling an index globally. This could look something like:
    > 
    > ALTER INDEX index_name ENABLE;
    > ALTER INDEX index_name DISABLE;
    > 
    > A disabled index would still receive updates and enforce constraints as 
    > usual but would not be used for queries. This allows users to assess 
    > whether an index impacts query performance before deciding to drop it 
    > entirely.
    
    I think a better approach would be to make the list of disabled indexes 
    a GUC setting, which would then internally have an effect similar to 
    enable_indexscan, meaning it would make the listed indexes unattractive 
    to the planner.
    
    This seems better than the proposed DDL command, because you'd be able 
    to use this per-session, instead of forcing a global state, and even 
    unprivileged users could use it.
    
    (I think we have had proposals like this before, but I can't find the 
    discussion I'm thinking of right now.)
    
    
    
    
    
  15. Re: Proposal to Enable/Disable Index using ALTER INDEX

    Shayon Mukherjee <shayonj@gmail.com> — 2024-09-23T20:51:57Z

    That's a good point.
    
    +1 for the idea of the GUC setting, especially since, as you mentioned, it allows unprivileged users to access it and being per-session..
    
    I am happy to draft a patch for this as well. I think I have a working idea so far of where the necessary checks might go. However if you don’t mind, can you elaborate further on how the effect would be similar to enable_indexscan? 
    
    I was thinking we could introduce a new GUC option called `disabled_indexes` and perform a check against in all places for each index being considered with its OID via get_relname_relid through a helper function in the various places we need to prompt the planner to not use the index (like in indxpath.c as an example).
    
    Curious to learn if you have a different approach in mind perhaps?
    
    Thank you,
    Shayon
    
    
    > On Sep 23, 2024, at 11:14 AM, Peter Eisentraut <peter@eisentraut.org> wrote:
    > 
    > On 09.09.24 23:38, Shayon Mukherjee wrote:
    >> *Problem*:
    >> Adding and removing indexes is a common operation in PostgreSQL. On larger databases, however, these operations can be resource-intensive. When evaluating the performance impact of one or more indexes, dropping them might not be ideal since as a user you may want a quicker way to test their effects without fully committing to removing & adding them back again. Which can be a time taking operation on larger tables.
    >> *Proposal*:
    >> I propose adding an ALTER INDEX command that allows for enabling or disabling an index globally. This could look something like:
    >> ALTER INDEX index_name ENABLE;
    >> ALTER INDEX index_name DISABLE;
    >> A disabled index would still receive updates and enforce constraints as usual but would not be used for queries. This allows users to assess whether an index impacts query performance before deciding to drop it entirely.
    > 
    > I think a better approach would be to make the list of disabled indexes a GUC setting, which would then internally have an effect similar to enable_indexscan, meaning it would make the listed indexes unattractive to the planner.
    > 
    > This seems better than the proposed DDL command, because you'd be able to use this per-session, instead of forcing a global state, and even unprivileged users could use it.
    > 
    > (I think we have had proposals like this before, but I can't find the discussion I'm thinking of right now.)
    > 
    
    
    
    
    
  16. Re: Proposal to Enable/Disable Index using ALTER INDEX

    Shayon Mukherjee <shayonj@gmail.com> — 2024-09-23T21:03:41Z

    I found an old thread here [0].
    
    Also, a question: If we go with the GUC approach, how do we expect `pg_get_indexdef` to behave?
    
    I suppose it would behave no differently than it otherwise would, because there's no new SQL grammar to support and, given its GUC status, it seems reasonable that `pg_get_indexdef` doesn’t reflect whether an index is enabled or not. 
    
    If so, then I wonder if using a dedicated `ALTER` command and keeping the state in `pg_index` would be better for consistency's sake?
    
    [0]https://postgrespro.com/list/id/20151212.112536.1628974191058745674.t-ishii@sraoss.co.jp
    
    Thank you
    Shayon
    
    > On Sep 23, 2024, at 4:51 PM, Shayon Mukherjee <shayonj@gmail.com> wrote:
    > 
    > That's a good point.
    > 
    > +1 for the idea of the GUC setting, especially since, as you mentioned, it allows unprivileged users to access it and being per-session..
    > 
    > I am happy to draft a patch for this as well. I think I have a working idea so far of where the necessary checks might go. However if you don’t mind, can you elaborate further on how the effect would be similar to enable_indexscan? 
    > 
    > I was thinking we could introduce a new GUC option called `disabled_indexes` and perform a check against in all places for each index being considered with its OID via get_relname_relid through a helper function in the various places we need to prompt the planner to not use the index (like in indxpath.c as an example).
    > 
    > Curious to learn if you have a different approach in mind perhaps?
    > 
    > Thank you,
    > Shayon
    > 
    > 
    >> On Sep 23, 2024, at 11:14 AM, Peter Eisentraut <peter@eisentraut.org> wrote:
    >> 
    >> On 09.09.24 23:38, Shayon Mukherjee wrote:
    >>> *Problem*:
    >>> Adding and removing indexes is a common operation in PostgreSQL. On larger databases, however, these operations can be resource-intensive. When evaluating the performance impact of one or more indexes, dropping them might not be ideal since as a user you may want a quicker way to test their effects without fully committing to removing & adding them back again. Which can be a time taking operation on larger tables.
    >>> *Proposal*:
    >>> I propose adding an ALTER INDEX command that allows for enabling or disabling an index globally. This could look something like:
    >>> ALTER INDEX index_name ENABLE;
    >>> ALTER INDEX index_name DISABLE;
    >>> A disabled index would still receive updates and enforce constraints as usual but would not be used for queries. This allows users to assess whether an index impacts query performance before deciding to drop it entirely.
    >> 
    >> I think a better approach would be to make the list of disabled indexes a GUC setting, which would then internally have an effect similar to enable_indexscan, meaning it would make the listed indexes unattractive to the planner.
    >> 
    >> This seems better than the proposed DDL command, because you'd be able to use this per-session, instead of forcing a global state, and even unprivileged users could use it.
    >> 
    >> (I think we have had proposals like this before, but I can't find the discussion I'm thinking of right now.)
    >> 
    > 
    
    
  17. Re: Proposal to Enable/Disable Index using ALTER INDEX

    David Rowley <dgrowleyml@gmail.com> — 2024-09-24T00:30:59Z

    On Tue, 24 Sept 2024 at 03:14, Peter Eisentraut <peter@eisentraut.org> wrote:
    >
    > On 09.09.24 23:38, Shayon Mukherjee wrote:
    > > ALTER INDEX index_name ENABLE;
    > > ALTER INDEX index_name DISABLE;
    >
    > I think a better approach would be to make the list of disabled indexes
    > a GUC setting, which would then internally have an effect similar to
    > enable_indexscan, meaning it would make the listed indexes unattractive
    > to the planner.
    
    I understand the last discussion went down that route too. For me, it
    seems strange that adding some global variable is seen as cleaner than
    storing the property in the same location as all the other index
    properties.
    
    How would you ensure no cached plans are still using the index after
    changing the GUC?
    
    > This seems better than the proposed DDL command, because you'd be able
    > to use this per-session, instead of forcing a global state, and even
    > unprivileged users could use it.
    
    That's true.
    
    > (I think we have had proposals like this before, but I can't find the
    > discussion I'm thinking of right now.)
    
    I think it's the one that was already linked by Nathan.  [1]? The GUC
    seems to have been first suggested on the same thread in [2].
    
    David
    
    [1] https://www.postgresql.org/message-id/ed8c9ed7-bb5d-aaec-065b-ad4893645deb%402ndQuadrant.com
    [2] https://www.postgresql.org/message-id/29800.1529359024%40sss.pgh.pa.us
    
    
    
    
  18. Re: Proposal to Enable/Disable Index using ALTER INDEX

    Shayon Mukherjee <shayonj@gmail.com> — 2024-09-24T01:44:17Z

    On Mon, Sep 23, 2024 at 8:31 PM David Rowley <dgrowleyml@gmail.com> wrote:
    
    > On Tue, 24 Sept 2024 at 03:14, Peter Eisentraut <peter@eisentraut.org>
    > wrote:
    > >
    > > On 09.09.24 23:38, Shayon Mukherjee wrote:
    > > > ALTER INDEX index_name ENABLE;
    > > > ALTER INDEX index_name DISABLE;
    > >
    > > I think a better approach would be to make the list of disabled indexes
    > > a GUC setting, which would then internally have an effect similar to
    > > enable_indexscan, meaning it would make the listed indexes unattractive
    > > to the planner.
    >
    > I understand the last discussion went down that route too. For me, it
    > seems strange that adding some global variable is seen as cleaner than
    > storing the property in the same location as all the other index
    > properties.
    >
    >
    That was my first instinct as well. Although, being able to control this
    setting on a per session level and as an unprivileged user is somewhat
    attractive.
    
    
    > How would you ensure no cached plans are still using the index after
    > changing the GUC?
    >
    
    Could we call ResetPlanCache() to invalidate all plan caches from the
    assign_ hook on GUC when it's set (and doesn't match the old value).
    Something like this (assuming the GUC is called `disabled_indexes`)
    
    void
    assign_disabled_indexes(const char *newval, void *extra)
    {
    if (disabled_indexes != newval)
    ResetPlanCache();
    }
    
    A bit heavy-handed, but perhaps it's OK, since it's not meant to be used
    frequently also ?
    
    
    > > This seems better than the proposed DDL command, because you'd be able
    > > to use this per-session, instead of forcing a global state, and even
    > > unprivileged users could use it.
    >
    > That's true.
    >
    > > (I think we have had proposals like this before, but I can't find the
    > > discussion I'm thinking of right now.)
    >
    > I think it's the one that was already linked by Nathan.  [1]? The GUC
    > seems to have been first suggested on the same thread in [2].
    >
    > David
    >
    > [1]
    > https://www.postgresql.org/message-id/ed8c9ed7-bb5d-aaec-065b-ad4893645deb%402ndQuadrant.com
    > [2] https://www.postgresql.org/message-id/29800.1529359024%40sss.pgh.pa.us
    >
    
    
    Shayon
    
  19. Re: Proposal to Enable/Disable Index using ALTER INDEX

    Maciek Sakrejda <m.sakrejda@gmail.com> — 2024-09-24T04:37:58Z

    If one of the use cases is soft-dropping indexes, would a GUC approach
    still support that? ALTER TABLE?
    
  20. Re: Proposal to Enable/Disable Index using ALTER INDEX

    Shayon Mukherjee <shayonj@gmail.com> — 2024-09-24T13:19:45Z

    Hello,
    
    Regarding GUC implementation for index disabling, I was imagining something
    like the attached PATCH. The patch compiles and can be applied for testing.
    It's not meant to be production ready, but I am sharing it as a way to get
    a sense of the nuts and bolts. It requires more proper test cases and docs,
    etc. Example towards the end of the email.
    
    That said, I am still quite torn between GUC setting or having a dedicated
    ALTER grammar. My additional thoughts which is mostly a summary of what
    David and Peter have already very nicely raised earlier are:
    
    - GUC allows a non-privileged user to disable one or more indexes per
    session.
    
    - If we think of the task of disabling indexes temporarily (without
    stopping any updates to the index), then it feels more in the territory of
    query tuning than index maintenance. In which case, a GUC setting makes
    more sense and sits well with others in the team like enable_indexscan,
    enable_indexonlyscan and so on.
    
    - At the same time, as David pointed out earlier, GUC is also a global
    setting and perhaps storing the state of whether or not an index is being
    used is perhaps better situated along with other index properties in
    pg_index.
    
    - One of my original motivations for the proposal was also that we can
    disable an index for _all_ sessions quickly without it impacting index
    build and turn it back on quickly as well. To do so with GUC, we would need
    to do something like the following, if I am not mistaken, in which case
    that is not something an unprivileged user may be able to perform, so just
    calling it out.
    
      ALTER USER example_user SET disabled_indexes = 'idx_foo_bar';
    
    - For an ALTER statement, I think an ALTER INDEX makes more sense than
    ALTER TABLE, especially since we have the existing ALTER INDEX grammar and
    functionality. But let me know if I am missing something here.
    
    - Resetting plan cache could still be an open question for GUC. I was
    wondering if we can reset the plan cache local to the session for GUC (like
    the one in the PATCH attached) and if that is enough? This concern doesn't
    apply with managing property in pg_index.
    
    - With a GUC attribute, the state of an index being enabled/disabled won't
    be captured in pg_get_indexdef(), and that is likely OK, but maybe that
    would need to be made explicit through docs.
    
    Example 1
    
    CREATE TABLE b AS SELECT generate_series(1,1000) AS b;
    CREATE INDEX ON b((b%10));
    ANALYZE b;
    EXPLAIN SELECT DISTINCT b%10 FROM b;
    
    SET disabled_indexes = 'b_expr_idx';
    
    EXPLAIN SELECT DISTINCT b%10 FROM b; -- HashAggregate rows=10000
    
    Example 2
    
    CREATE TABLE disabled_index_test(id int PRIMARY KEY, data text);
    INSERT INTO disabled_index_test SELECT g, 'data ' || g FROM
    generate_series(1, 1000) g;
    CREATE INDEX disabled_index_idx1 ON disabled_index_test(data);
    EXPLAIN (COSTS OFF) SELECT * FROM disabled_index_test WHERE data = 'data
    500';
    
    SET disabled_indexes = 'b_expr_idx, disabled_index_idx1';
    
    EXPLAIN SELECT * FROM disabled_index_test WHERE data = 'data 500'; -- no
    index is used
    
    Wrapping up...
    
    I am sure there are things I am missing or unintentionally overlooking.
    Since this would be a nice feature to have, I'd love some guidance on which
    approach seems like a good next step to take. I am happy to work
    accordingly on the patch.
    
    Thank you
    Shayon
    
    On Tue, Sep 24, 2024 at 12:38 AM Maciek Sakrejda <m.sakrejda@gmail.com>
    wrote:
    
    > If one of the use cases is soft-dropping indexes, would a GUC approach
    > still support that? ALTER TABLE?
    >
    
    
    -- 
    Kind Regards,
    Shayon Mukherjee
    
  21. Re: Proposal to Enable/Disable Index using ALTER INDEX

    Peter Eisentraut <peter@eisentraut.org> — 2024-09-24T18:08:10Z

    On 23.09.24 22:51, Shayon Mukherjee wrote:
    > I am happy to draft a patch for this as well. I think I have a working
    > idea so far of where the necessary checks might go. However if you don’t
    >   mind, can you elaborate further on how the effect would be similar to
    > enable_indexscan?
    
    Planner settings like enable_indexscan used to just add a large number 
    (disable_cost) to the estimated plan node costs.  It's a bit more 
    sophisticated in PG17.  But in any case, I imagine "disabling an index" 
    could use the same mechanism.  Or maybe not, maybe the setting would 
    just completely ignore the index.
    
    
    
    
  22. Re: Proposal to Enable/Disable Index using ALTER INDEX

    Peter Eisentraut <peter@eisentraut.org> — 2024-09-24T18:11:23Z

    On 24.09.24 02:30, David Rowley wrote:
    > I understand the last discussion went down that route too. For me, it
    > seems strange that adding some global variable is seen as cleaner than
    > storing the property in the same location as all the other index
    > properties.
    
    It's arguably not actually a property of the index, it's a property of 
    the user's session.  Like, kind of, the search path is a session 
    property, not a property of a schema.
    
    > How would you ensure no cached plans are still using the index after
    > changing the GUC?
    
    Something for the patch author to figure out. ;-)
    
    
    
    
  23. Re: Proposal to Enable/Disable Index using ALTER INDEX

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-09-24T18:21:04Z

    Peter Eisentraut <peter@eisentraut.org> writes:
    > On 23.09.24 22:51, Shayon Mukherjee wrote:
    >> I am happy to draft a patch for this as well. I think I have a working
    >> idea so far of where the necessary checks might go. However if you don’t
    >> mind, can you elaborate further on how the effect would be similar to
    >> enable_indexscan?
    
    > Planner settings like enable_indexscan used to just add a large number 
    > (disable_cost) to the estimated plan node costs.  It's a bit more 
    > sophisticated in PG17.  But in any case, I imagine "disabling an index" 
    > could use the same mechanism.  Or maybe not, maybe the setting would 
    > just completely ignore the index.
    
    Yeah, I'd be inclined to implement this by having create_index_paths
    just not make any paths for rejected indexes.  Or you could back up
    another step and keep plancat.c from building IndexOptInfos for them.
    The latter might have additional effects, such as preventing the plan
    from relying on a uniqueness condition enforced by the index.  Not
    clear to me if that's desirable or not.
    
    [ thinks... ]  One good reason for implementing it in plancat.c is
    that you'd have the index relation open and be able to see its name
    for purposes of matching to the filter.  Anywhere else, getting the
    name would involve additional overhead.
    
    			regards, tom lane
    
    
    
    
  24. Re: Proposal to Enable/Disable Index using ALTER INDEX

    Shayon Mukherjee <shayonj@gmail.com> — 2024-09-24T19:38:08Z

    Thank you for the historical context and working, I understand what you were referring to before now. 
    
    Shayon
    
    > On Sep 24, 2024, at 2:08 PM, Peter Eisentraut <peter@eisentraut.org> wrote:
    > 
    > On 23.09.24 22:51, Shayon Mukherjee wrote:
    >> I am happy to draft a patch for this as well. I think I have a working
    >> idea so far of where the necessary checks might go. However if you don’t
    >>  mind, can you elaborate further on how the effect would be similar to
    >> enable_indexscan?
    > 
    > Planner settings like enable_indexscan used to just add a large number (disable_cost) to the estimated plan node costs.  It's a bit more sophisticated in PG17.  But in any case, I imagine "disabling an index" could use the same mechanism.  Or maybe not, maybe the setting would just completely ignore the index.
    
    
    
    
    
  25. Re: Proposal to Enable/Disable Index using ALTER INDEX

    Shayon Mukherjee <shayonj@gmail.com> — 2024-09-26T17:39:23Z

    Hello,
    
    I am back with a PATCH :). Thanks to everyone in the threads for all the helpful discussions.
    
    This proposal is for a PATCH to introduce a GUC variable to disable specific indexes during query planning.
    
    This is an alternative approach to the previous PATCH I had proposed and is improved upon after some of the recent discussions in the thread. The PATCH contains the relevant changes, regression tests, and documentation.
    
    I went with the GUC approach to introduce a way for a user to disable indexes during query planning over dedicated SQL Grammar and introducing the `isenabled` attribute in `pg_index` for the following reasons:
    
    - Inspired by the discussions brought in earlier about this setting being something that unprivileged users can benefit from versus an ALTER statement.
    - A GUC variable felt more closely aligned with the query tuning purpose, which this feature would serve, over index maintenance, the state of which is more closely reflected in `pg_index`.
    
    Implementation details:
    
    The patch introduces a new GUC parameter `disabled_indexes` that allows users to specify a comma-separated list of indexes to be ignored during query planning. Key aspects:
    
    - Adds a new `isdisabled` 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.
    - Implements GUC hooks for parameter validation and assignment.
    - Resets the plan cache when the `disabled_indexes` list is modified through `ResetPlanCache()`
    
    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 (?).
    
    As mentioned before, this does not impact the building of the index. That still happens.
    
    I have added regression tests for:
    
    - 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
    
    I'd love to hear any feedback on the proposed PATCH and also the overall approach.
    
  26. Re: Proposal to Enable/Disable Index using ALTER INDEX

    Shayon Mukherjee <shayonj@gmail.com> — 2024-10-01T18:17:19Z

    Hello,
    
    Also added this as a post in Commit Fest [0]
    
    [0] https://commitfest.postgresql.org/50/5274/
    
    Thank you
    Shayon
    
    > On Sep 26, 2024, at 1:39 PM, Shayon Mukherjee <shayonj@gmail.com> wrote:
    > 
    > Hello,
    > 
    > I am back with a PATCH :). Thanks to everyone in the threads for all the helpful discussions.
    > 
    > This proposal is for a PATCH to introduce a GUC variable to disable specific indexes during query planning.
    > 
    > This is an alternative approach to the previous PATCH I had proposed and is improved upon after some of the recent discussions in the thread. The PATCH contains the relevant changes, regression tests, and documentation.
    > 
    > I went with the GUC approach to introduce a way for a user to disable indexes during query planning over dedicated SQL Grammar and introducing the `isenabled` attribute in `pg_index` for the following reasons:
    > 
    > - Inspired by the discussions brought in earlier about this setting being something that unprivileged users can benefit from versus an ALTER statement.
    > - A GUC variable felt more closely aligned with the query tuning purpose, which this feature would serve, over index maintenance, the state of which is more closely reflected in `pg_index`.
    > 
    > Implementation details:
    > 
    > The patch introduces a new GUC parameter `disabled_indexes` that allows users to specify a comma-separated list of indexes to be ignored during query planning. Key aspects:
    > 
    > - Adds a new `isdisabled` 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.
    > - Implements GUC hooks for parameter validation and assignment.
    > - Resets the plan cache when the `disabled_indexes` list is modified through `ResetPlanCache()`
    > 
    > 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 (?).
    > 
    > As mentioned before, this does not impact the building of the index. That still happens.
    > 
    > I have added regression tests for:
    > 
    > - 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
    > 
    > I'd love to hear any feedback on the proposed PATCH and also the overall approach.
    > <v1-0001-Ability-to-enable-disable-indexes-through-GUC.patch>
    >> On Sep 24, 2024, at 9:19 AM, Shayon Mukherjee <shayonj@gmail.com> wrote:
    >> 
    >> -- 
    >> Kind Regards,
    >> Shayon Mukherjee
    >> <v1-0001-Proof-of-Concept-Ability-to-enable-disable-indexe.patch>
    > 
    
    
  27. Re: Proposal to Enable/Disable Index using ALTER INDEX

    Robert Haas <robertmhaas@gmail.com> — 2024-10-07T15:52:27Z

    On Mon, Sep 23, 2024 at 11:14 AM Peter Eisentraut <peter@eisentraut.org> wrote:
    > I think a better approach would be to make the list of disabled indexes
    > a GUC setting, which would then internally have an effect similar to
    > enable_indexscan, meaning it would make the listed indexes unattractive
    > to the planner.
    >
    > This seems better than the proposed DDL command, because you'd be able
    > to use this per-session, instead of forcing a global state, and even
    > unprivileged users could use it.
    >
    > (I think we have had proposals like this before, but I can't find the
    > discussion I'm thinking of right now.)
    
    I feel like a given user could want either one of these things. If
    you've discovered that a certain index is causing your production
    application to pick the wrong index, disabling it and thereby
    affecting all backends is what you want. If you're trying to
    experiment with different query plans without changing anything for
    other backends, being able to set some session-local state is better.
    I don't understand the argument that one of these is categorically
    better than the other.
    
    -- 
    Robert Haas
    EDB: http://www.enterprisedb.com
    
    
    
    
  28. Re: Proposal to Enable/Disable Index using ALTER INDEX

    Shayon Mukherjee <shayonj@gmail.com> — 2024-10-09T07:06:50Z

    
    > On Oct 7, 2024, at 4:52 PM, Robert Haas <robertmhaas@gmail.com> wrote:
    > 
    > On Mon, Sep 23, 2024 at 11:14 AM Peter Eisentraut <peter@eisentraut.org> wrote:
    >> I think a better approach would be to make the list of disabled indexes
    >> a GUC setting, which would then internally have an effect similar to
    >> enable_indexscan, meaning it would make the listed indexes unattractive
    >> to the planner.
    >> 
    >> This seems better than the proposed DDL command, because you'd be able
    >> to use this per-session, instead of forcing a global state, and even
    >> unprivileged users could use it.
    >> 
    >> (I think we have had proposals like this before, but I can't find the
    >> discussion I'm thinking of right now.)
    > 
    > I feel like a given user could want either one of these things. If
    > you've discovered that a certain index is causing your production
    > application to pick the wrong index, disabling it and thereby
    > affecting all backends is what you want. If you're trying to
    > experiment with different query plans without changing anything for
    > other backends, being able to set some session-local state is better.
    > I don't understand the argument that one of these is categorically
    > better than the other.
    
    Makes sense to me and it’s something I am somewhat split on as well. I suppose with a GUC you can still do some thing like
    
    ALTER USER foobar SET disabled_indexes to ‘idx_test_table_id’ 
    
    [thinking…] This way all new sessions will start to not consider the index when query planning. Of course it does not help existing sessions, so one may need to kill those backends, which could be heavy handed.
    
    Both these options clearly serve slightly different purposes with good pros and I am currently thinking if GUC is that good middle ground solution.
    
    Curious if someone has a stronger opinion on which one of these might make more sense perhaps :-D.
    
    [thinking…] Unless - we try to do support both a GUC and the ALTER INDEX ENABLE/DISABLE grammar + isdisabled attribute on pg_index?
    
    I can see that both implementations (GUC and the new attribute on pg_index via ALTER) have the primary logic managed by `get_relation_info` in `plancat.c`. Here, we set `isdisabled` (new attribute) on `IndexOptInfo` and compare it against `disabled_indexes` in the GUC (from the previous GUC patch). Similarly, for `pg_index`, which is already open in `get_relation_info`, we can read from `pg_index.isdisabled` and accordingly update `IndexOptInfo.isdisabled`.
    
    [0] https://www.postgresql.org/message-id/6CE345C1-6FFD-4E4C-8775-45DA659C57CF@gmail.com
    
    Thanks
    Shayon 
    
    
    
  29. Re: Proposal to Enable/Disable Index using ALTER INDEX

    David Rowley <dgrowleyml@gmail.com> — 2024-10-09T08:19:34Z

    On Wed, 9 Oct 2024 at 20:07, Shayon Mukherjee <shayonj@gmail.com> wrote:
    > [thinking…] Unless - we try to do support both a GUC and the ALTER INDEX ENABLE/DISABLE grammar + isdisabled attribute on pg_index?
    
    I just wanted to explain my point of view on this.  This is my opinion
    and is by no means authoritative.
    
    I was interested in this patch when you proposed it as an ALTER INDEX
    option. I know other committers seem interested, but I personally
    don't have any interest in the GUC option.  I think the reason I
    dislike it is that it's yet another not even half-baked take on
    planner hints (the other one being enable* GUCs). I often thought that
    if we ever did planner hints that it would be great to have multiple
    ways to specify the hints. Ordinarily, I'd expect some special comment
    type as the primary method to specify hints, but equally, it would be
    nice to be able to specify them in other ways. e.g. a GUC to have them
    apply to more than just 1 query. Useful for things such as "don't use
    index X".
    
    Now, I'm not suggesting you go off and code up planner hints. That's a
    huge project. I'm just concerned that we've already got a fair bit of
    cruft that will be left remaining if we ever get core planner hints
    and a disabled_indexes GUC will just add to that. I don't feel like
    the ALTER INDEX method would be leftover cruft from us gaining core
    planner hints. Others might feel differently on that one. I feel the
    ALTER INDEX option is less controversial.
    
    I'll also stand by what I said earlier on this thread. If PeterG gets
    index skip scans done for PG18, then it's likely there's going to be
    lots of users considering if they still need a certain index or not
    after upgrading to PG18.
    
    David
    
    
    
    
  30. Re: Proposal to Enable/Disable Index using ALTER INDEX

    Robert Haas <robertmhaas@gmail.com> — 2024-10-09T12:41:10Z

    On Wed, Oct 9, 2024 at 4:19 AM David Rowley <dgrowleyml@gmail.com> wrote:
    > On Wed, 9 Oct 2024 at 20:07, Shayon Mukherjee <shayonj@gmail.com> wrote:
    > > [thinking…] Unless - we try to do support both a GUC and the ALTER INDEX ENABLE/DISABLE grammar + isdisabled attribute on pg_index?
    >
    > I just wanted to explain my point of view on this.  This is my opinion
    > and is by no means authoritative.
    >
    > I was interested in this patch when you proposed it as an ALTER INDEX
    > option. I know other committers seem interested, but I personally
    > don't have any interest in the GUC option.  I think the reason I
    > dislike it is that it's yet another not even half-baked take on
    > planner hints (the other one being enable* GUCs). I often thought that
    > if we ever did planner hints that it would be great to have multiple
    > ways to specify the hints. Ordinarily, I'd expect some special comment
    > type as the primary method to specify hints, but equally, it would be
    > nice to be able to specify them in other ways. e.g. a GUC to have them
    > apply to more than just 1 query. Useful for things such as "don't use
    > index X".
    
    +1. A GUC can be done as a contrib module using existing hooks, and I
    think that's already been done outside of core, perhaps multiple
    times. That certainly doesn't mean we CAN'T add it as an in-core
    feature, but I do think "yet another not even half-baked take on
    planner hints" is a fair description. What I would personally like to
    see is for us to ship one or possibly more than one contrib module
    that let people do hint-like things in useful ways, and this could be
    a part of that. But I think we need better infrastructure for
    controlling the planner behavior first, hence the "allowing extensions
    to control planner behavior" thread.
    
    -- 
    Robert Haas
    EDB: http://www.enterprisedb.com
    
    
    
    
  31. Re: Proposal to Enable/Disable Index using ALTER INDEX

    Vinicius Abrahao <vinnix.bsd@gmail.com> — 2024-10-12T09:40:30Z

    On Wed, Oct 9, 2024 at 1:41 PM Robert Haas <robertmhaas@gmail.com> wrote:
    
    > On Wed, Oct 9, 2024 at 4:19 AM David Rowley <dgrowleyml@gmail.com> wrote:
    > > On Wed, 9 Oct 2024 at 20:07, Shayon Mukherjee <shayonj@gmail.com> wrote:
    > > > [thinking…] Unless - we try to do support both a GUC and the ALTER
    > INDEX ENABLE/DISABLE grammar + isdisabled attribute on pg_index?
    > >
    > > I just wanted to explain my point of view on this.  This is my opinion
    > > and is by no means authoritative.
    > >
    > > I was interested in this patch when you proposed it as an ALTER INDEX
    > > option. I know other committers seem interested, but I personally
    > > don't have any interest in the GUC option.  I think the reason I
    > > dislike it is that it's yet another not even half-baked take on
    > > planner hints (the other one being enable* GUCs). I often thought that
    > > if we ever did planner hints that it would be great to have multiple
    > > ways to specify the hints. Ordinarily, I'd expect some special comment
    > > type as the primary method to specify hints, but equally, it would be
    > > nice to be able to specify them in other ways. e.g. a GUC to have them
    > > apply to more than just 1 query. Useful for things such as "don't use
    > > index X".
    >
    > +1. A GUC can be done as a contrib module using existing hooks, and I
    > think that's already been done outside of core, perhaps multiple
    > times. That certainly doesn't mean we CAN'T add it as an in-core
    > feature, but I do think "yet another not even half-baked take on
    > planner hints" is a fair description. What I would personally like to
    > see is for us to ship one or possibly more than one contrib module
    > that let people do hint-like things in useful ways, and this could be
    > a part of that. But I think we need better infrastructure for
    > controlling the planner behavior first, hence the "allowing extensions
    > to control planner behavior" thread.
    >
    >
    What's the strategy here in this discussion?
    This topic is older than PostgreSQL itself - everytime WE talk about
    so-called "hints" we see procrastination in the name of trademarks.
    Lack of definition of what can and can't be done with hooks and what is the
    infra-estructural code that is necessary to allow it from core.
    
    Take for example the need of disabling an index. What does it mean
    practically and for which component of the code?
    You are going to disable the index but not the update of it? Why? Does it
    imply that when you are going to re-enable it you are going to recreate it?
    So that's more observed from the point of syntax and facilities.
    Also distributed into partitions and why not the opposite: the creation of
    a global index for all partitions. Also in discussion elsewhere.
    Otherwise it will appear that people will need hints and contribs to
    "outsource" the main role of what is strategic to the team to companies
    elsewhere.
    
    Regards,
    Vinícius
    
  32. Re: Proposal to Enable/Disable Index using ALTER INDEX

    Shayon Mukherjee <shayonj@gmail.com> — 2024-10-12T09:53:42Z

    Hi David, 
    
    Answered below
    
    > On Oct 9, 2024, at 9:19 AM, David Rowley <dgrowleyml@gmail.com> wrote:
    > 
    > On Wed, 9 Oct 2024 at 20:07, Shayon Mukherjee <shayonj@gmail.com> wrote:
    >> [thinking…] Unless - we try to do support both a GUC and the ALTER INDEX ENABLE/DISABLE grammar + isdisabled attribute on pg_index?
    > 
    > I just wanted to explain my point of view on this.  This is my opinion
    > and is by no means authoritative.
    > 
    > I was interested in this patch when you proposed it as an ALTER INDEX
    > option. I know other committers seem interested, but I personally
    > don't have any interest in the GUC option.  I think the reason I
    > dislike it is that it's yet another not even half-baked take on
    > planner hints (the other one being enable* GUCs). I often thought that
    > if we ever did planner hints that it would be great to have multiple
    > ways to specify the hints. Ordinarily, I'd expect some special comment
    > type as the primary method to specify hints, but equally, it would be
    > nice to be able to specify them in other ways. e.g. a GUC to have them
    > apply to more than just 1 query. Useful for things such as "don't use
    > index X".
    
    Thank you so much this context, as someone new to psql-hackers, having this insight is super useful. Also getting a sense of how folks feel about controlling different behaviors like planner hints through GUC and SQL grammar.  
    
    For instance: I wasn’t quite able to figure out the how to properly distinguish + reason  between the enable* GUCs and ALTER index for this case, and patches are per my limited understand of the historical context as well.
      
    > 
    > Now, I'm not suggesting you go off and code up planner hints. That's a
    > huge project. I'm just concerned that we've already got a fair bit of
    > cruft that will be left remaining if we ever get core planner hints
    > and a disabled_indexes GUC will just add to that. I don't feel like
    > the ALTER INDEX method would be leftover cruft from us gaining core
    > planner hints. Others might feel differently on that one. I feel the
    > ALTER INDEX option is less controversial.
    > 
    > I'll also stand by what I said earlier on this thread. If PeterG gets
    > index skip scans done for PG18, then it's likely there's going to be
    > lots of users considering if they still need a certain index or not
    > after upgrading to PG18.
    
    
    Likewise, I personally feel that the ability to disable indexes quickly and reverse the disabling (also quickly) is super useful, especially from an operational POV (point of view). So, I am very keen on getting this landed and happy to iterate on as many patches as it takes. :D
    
    At this point, I am indifferent to each of the approaches (GUC or SQL grammar) based on the pros/cons I shared earlier in the thread & discussions in the thread. However, I would like us to make progress on getting _something_ out since the topic of disabling indexes has come up many times on pgsql-hackers in the past years and there is no easy way to toggle this behavior yet. 
    
    “yet another not even half-baked take on planner hints" is a good way to put things about enable* GUCs, so I am very much on board with proposing an updated PATCH to support disabling of indexes through ALTER. The original PATCH was here for context [1]. 
    
    I am also curious about supporting this ([1]) through the ALTER grammar and not having the planner consider indexes by updating `get_relation_info` in `plancat.c`. Basically, through `pg_index.isdisabled`, which is already open in `get_relation_info`, we can read from `pg_index.isdisabled` and accordingly update `IndexOptInfo.isdisabled`. So, I'm happy to explore that as well and share my findings. 
    
    
    [1] https://www.postgresql.org/message-id/CANqtF-oBaBtRfw9O7GAoHN3nNEZQYsW3oaGfD%2BwJfG8R29nZYw%40mail.gmail.com
    
    Thanks
    Shayon
    
    
  33. Re: Proposal to Enable/Disable Index using ALTER INDEX

    Shayon Mukherjee <shayonj@gmail.com> — 2024-10-12T09:56:24Z

    > On Oct 9, 2024, at 1:41 PM, Robert Haas <robertmhaas@gmail.com> wrote:
    > 
    > On Wed, Oct 9, 2024 at 4:19 AM David Rowley <dgrowleyml@gmail.com <mailto:dgrowleyml@gmail.com>> wrote:
    >> On Wed, 9 Oct 2024 at 20:07, Shayon Mukherjee <shayonj@gmail.com> wrote:
    >>> [thinking…] Unless - we try to do support both a GUC and the ALTER INDEX ENABLE/DISABLE grammar + isdisabled attribute on pg_index?
    >> 
    >> I just wanted to explain my point of view on this.  This is my opinion
    >> and is by no means authoritative.
    >> 
    >> I was interested in this patch when you proposed it as an ALTER INDEX
    >> option. I know other committers seem interested, but I personally
    >> don't have any interest in the GUC option.  I think the reason I
    >> dislike it is that it's yet another not even half-baked take on
    >> planner hints (the other one being enable* GUCs). I often thought that
    >> if we ever did planner hints that it would be great to have multiple
    >> ways to specify the hints. Ordinarily, I'd expect some special comment
    >> type as the primary method to specify hints, but equally, it would be
    >> nice to be able to specify them in other ways. e.g. a GUC to have them
    >> apply to more than just 1 query. Useful for things such as "don't use
    >> index X".
    > 
    > +1. A GUC can be done as a contrib module using existing hooks, and I
    > think that's already been done outside of core, perhaps multiple
    > times. That certainly doesn't mean we CAN'T add it as an in-core
    > feature, but I do think "yet another not even half-baked take on
    > planner hints" is a fair description. What I would personally like to
    > see is for us to ship one or possibly more than one contrib module
    > that let people do hint-like things in useful ways, and this could be
    > a part of that. But I think we need better infrastructure for
    > controlling the planner behavior first, hence the "allowing extensions
    > to control planner behavior" thread.
    > 
    
    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
    
    Thanks
    Shayon
    
    
  34. Re: Proposal to Enable/Disable Index using ALTER INDEX

    Robert Haas <robertmhaas@gmail.com> — 2024-10-15T14:40:38Z

    On Sat, Oct 12, 2024 at 5:56 AM Shayon Mukherjee <shayonj@gmail.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.
    
    -- 
    Robert Haas
    EDB: http://www.enterprisedb.com
    
    
    
    
  35. Re: Proposal to Enable/Disable Index using ALTER INDEX

    David Rowley <dgrowleyml@gmail.com> — 2024-10-15T23:16:48Z

    On Sat, 12 Oct 2024 at 22:41, Vinícius Abrahão <vinnix.bsd@gmail.com> wrote:
    > You are going to disable the index but not the update of it? Why? Does it imply that when you are going to re-enable it you are going to recreate it?
    
    It might be worth you reading the discussion and proposed patches. I
    think either of those would answer your questions.
    
    I don't recall anyone ever proposing that re-enabling the index would
    result in it having to be rebuilt. If that was a requirement, then I'd
    say there does not seem much point in the feature. You might as well
    just drop the index and recreate it if you change your mind.
    
    David
    
    
    
    
  36. Re: Proposal to Enable/Disable Index using ALTER INDEX

    David Rowley <dgrowleyml@gmail.com> — 2024-10-15T23:25:38Z

    On Wed, 16 Oct 2024 at 03:40, Robert Haas <robertmhaas@gmail.com> wrote:
    >
    > On Sat, Oct 12, 2024 at 5:56 AM Shayon Mukherjee <shayonj@gmail.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.
    
    David
    
    
    
    
  37. Re: Proposal to Enable/Disable Index using ALTER INDEX

    Peter Eisentraut <peter@eisentraut.org> — 2024-10-16T12:33:24Z

    On 24.09.24 20:21, Tom Lane wrote:
    > Peter Eisentraut <peter@eisentraut.org> writes:
    >> On 23.09.24 22:51, Shayon Mukherjee wrote:
    >>> I am happy to draft a patch for this as well. I think I have a working
    >>> idea so far of where the necessary checks might go. However if you don’t
    >>> mind, can you elaborate further on how the effect would be similar to
    >>> enable_indexscan?
    > 
    >> Planner settings like enable_indexscan used to just add a large number
    >> (disable_cost) to the estimated plan node costs.  It's a bit more
    >> sophisticated in PG17.  But in any case, I imagine "disabling an index"
    >> could use the same mechanism.  Or maybe not, maybe the setting would
    >> just completely ignore the index.
    > 
    > Yeah, I'd be inclined to implement this by having create_index_paths
    > just not make any paths for rejected indexes.  Or you could back up
    > another step and keep plancat.c from building IndexOptInfos for them.
    > The latter might have additional effects, such as preventing the plan
    > from relying on a uniqueness condition enforced by the index.  Not
    > clear to me if that's desirable or not.
    
    Yes, I think you'd want that, because one of the purposes of this 
    feature would be to test whether it's okay to drop an index.  So you 
    don't want the planner to take any account of the index for its decisions.
    
    
    
    
    
  38. Re: Proposal to Enable/Disable Index using ALTER INDEX

    Robert Haas <robertmhaas@gmail.com> — 2024-10-16T12:54:02Z

    On Wed, Oct 16, 2024 at 8:33 AM Peter Eisentraut <peter@eisentraut.org> wrote:
    > > Yeah, I'd be inclined to implement this by having create_index_paths
    > > just not make any paths for rejected indexes.  Or you could back up
    > > another step and keep plancat.c from building IndexOptInfos for them.
    > > The latter might have additional effects, such as preventing the plan
    > > from relying on a uniqueness condition enforced by the index.  Not
    > > clear to me if that's desirable or not.
    >
    > Yes, I think you'd want that, because one of the purposes of this
    > feature would be to test whether it's okay to drop an index.  So you
    > don't want the planner to take any account of the index for its decisions.
    
    I think this is right. I think we want to avoid invalidating the
    index, so we still need to consider it in determining where HOT
    updates must be performed, but we don't want to "improve" the plan
    using the index if it's disabled.
    
    -- 
    Robert Haas
    EDB: http://www.enterprisedb.com
    
    
    
    
  39. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Shayon Mukherjee <shayonj@gmail.com> — 2024-10-16T16:19:47Z

    > On Oct 15, 2024, at 7:25 PM, David Rowley <dgrowleyml@gmail.com> wrote:
    > 
    > On Wed, 16 Oct 2024 at 03:40, Robert Haas <robertmhaas@gmail.com> wrote:
    >> 
    >> On Sat, Oct 12, 2024 at 5:56 AM Shayon Mukherjee <shayonj@gmail.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
    
    
    
  40. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Shayon Mukherjee <shayonj@gmail.com> — 2024-10-16T18:15:51Z

    > On Oct 16, 2024, at 12:19 PM, Shayon Mukherjee <shayonj@gmail.com> wrote:
    > 
    > - ALTER INDEX ... ENABLE/DISABLE performs an in-place update of the pg_index 
    >   catalog to protect against indcheckxmin [2] (older unrelated thread).
    
    Performing the in place update of the pg_index row from ATExecEnableDisableIndex using systable_inplace_update_begin was failing in CI weirdly but not on my local MacBook when running spec suite. I am also wondering what is causing the requirement [1] to update the row in-place vs say using CatalogTupleUpdate since using the later is working well locally + CI? 
    
    I have attached a v2 patch (following from the last v1 patch [1]) that uses CatalogTupleUpdate and local + CI [2] is passing. 
    
    [1] https://www.postgresql.org/message-id/20180618215635.m5vrnxdxhxytvmcm@alap3.anarazel.de
    [2] https://www.postgresql.org/message-id/EF2313B8-A017-4869-9B7F-A24EDD8795DE%40gmail.com
    [3]  https://github.com/shayonj/postgres/pull/1
    
    Thanks
    Shayon
    
    
    
  41. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Shayon Mukherjee <shayonj@gmail.com> — 2024-10-16T22:01:13Z

    > On Oct 16, 2024, at 2:15 PM, Shayon Mukherjee <shayonj@gmail.com> wrote:
    > 
    > 
    >> On Oct 16, 2024, at 12:19 PM, Shayon Mukherjee <shayonj@gmail.com> wrote:
    >> 
    >> - ALTER INDEX ... ENABLE/DISABLE performs an in-place update of the pg_index 
    >>   catalog to protect against indcheckxmin [2] (older unrelated thread).
    > 
    > Performing the in place update of the pg_index row from ATExecEnableDisableIndex using systable_inplace_update_begin was failing in CI weirdly but not on my local MacBook when running spec suite. I am also wondering what is causing the requirement [1] to update the row in-place vs say using CatalogTupleUpdate since using the later is working well locally + CI? 
    > 
    
    
    I believe I somewhat understand why the issue might be occurring, where CI is failing the create_index regression test and crashing (signal 6 in postmaster logs). I suspect it might be due to a segmentation fault or a similar issue.
    
    IsInplaceUpdateOid is used in systable_inplace_update_begin (recently introduced), but it doesn’t yet support pg_index. Based on check_lock_if_inplace_updateable_rel in heapam.c and IsInplaceUpdateOid in catalog.c, introducing support for in-place updates via this new mechanism might not be straightforward for pg_index. It would require updating the callers to handle in-place updates and locks, I presume?
    
    I did confirm that, based on the v2 PATCH, when not doing in-place updates on pg_index - it means that the xmin for pg_index would increment. I suppose there’s a risk of indcheckxmin being marked as TRUE when xmin exceeds, potentially causing an index to be accidentally skipped ? (I am not 100% sure)
    
    I'll take some time to think this through and familiarize myself with the new systable_inplace_update_begin. In the meantime, aside from the in-place updates on pg_index, I would love to learn any first impressions or feedback on the patch folks may have.
    
    Thank you,
    Shayon
    
    
  42. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Shayon Mukherjee <shayonj@gmail.com> — 2024-10-17T13:59:34Z

    On Oct 16, 2024, at 6:01 PM, Shayon Mukherjee <shayonj@gmail.com> wrote:
    
    I'll take some time to think this through and familiarize myself with the
    new systable_inplace_update_begin. In the meantime, aside from the in-place
    updates on pg_index, I would love to learn any first impressions or
    feedback on the patch folks may have.
    
    
    My take away from whether or not an in-place update is needed on pg_index
    [1]
    
    - It’s unclear to me why it’s needed.
    - I understand the xmin would get incremented when using CatalogTupleUpdate
    to update indisenabled.
    - However, I haven’t been able to replicate any odd behavior locally or CI.
    - FWIW - REINDEX CONCURRENTLY (via index_swap),  index_constraint_create
    and few other places perform CatalogTupleUpdate to update the relevant
    attributes as well.
    
    Based on the above summary and after my testing I would like to propose a
    v3 of the patch. The only difference between this and v1 [2] is that the
    update of pg_index row happens via CatalogTupleUpdate.
    
    [1]
    https://www.postgresql.org/message-id/20180618215635.m5vrnxdxhxytvmcm@alap3.anarazel.de
    [2]
    https://www.postgresql.org/message-id/EF2313B8-A017-4869-9B7F-A24EDD8795DE%40gmail.com
    
    Thank you for bearing with me on this :D
    Shayon
    
  43. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Rafia Sabih <rafia.pghackers@gmail.com> — 2024-11-05T11:32:34Z

    Interesting idea.
    This patch needs a rebase.
    
    On Thu, 17 Oct 2024 at 15:59, Shayon Mukherjee <shayonj@gmail.com> wrote:
    
    >
    >
    > On Oct 16, 2024, at 6:01 PM, Shayon Mukherjee <shayonj@gmail.com> wrote:
    >
    > I'll take some time to think this through and familiarize myself with the
    > new systable_inplace_update_begin. In the meantime, aside from the in-place
    > updates on pg_index, I would love to learn any first impressions or
    > feedback on the patch folks may have.
    >
    >
    > My take away from whether or not an in-place update is needed on pg_index
    > [1]
    >
    > - It’s unclear to me why it’s needed.
    > - I understand the xmin would get incremented when
    > using CatalogTupleUpdate to update indisenabled.
    > - However, I haven’t been able to replicate any odd behavior locally or
    > CI.
    > - FWIW - REINDEX CONCURRENTLY (via index_swap),  index_constraint_create
    > and few other places perform CatalogTupleUpdate to update the relevant
    > attributes as well.
    >
    > Based on the above summary and after my testing I would like to propose a
    > v3 of the patch. The only difference between this and v1 [2] is that the
    > update of pg_index row happens via CatalogTupleUpdate.
    >
    > [1]
    > https://www.postgresql.org/message-id/20180618215635.m5vrnxdxhxytvmcm@alap3.anarazel.de
    > [2]
    > https://www.postgresql.org/message-id/EF2313B8-A017-4869-9B7F-A24EDD8795DE%40gmail.com
    >
    > Thank you for bearing with me on this :D
    > Shayon
    >
    
    
    -- 
    Regards,
    Rafia Sabih
    CYBERTEC PostgreSQL International GmbH
    
  44. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Robert Haas <robertmhaas@gmail.com> — 2024-11-05T15:55:12Z

    On Thu, Oct 17, 2024 at 9:59 AM Shayon Mukherjee <shayonj@gmail.com> wrote:
    > My take away from whether or not an in-place update is needed on pg_index [1]
    >
    > - It’s unclear to me why it’s needed.
    > - I understand the xmin would get incremented when using CatalogTupleUpdate to update indisenabled.
    > - However, I haven’t been able to replicate any odd behavior locally or CI.
    > - FWIW - REINDEX CONCURRENTLY (via index_swap),  index_constraint_create and few other places perform CatalogTupleUpdate to update the relevant attributes as well.
    >
    > Based on the above summary and after my testing I would like to propose a v3 of the patch. The only difference between this and v1 [2] is that the update of pg_index row happens via CatalogTupleUpdate.
    >
    > [1]  https://www.postgresql.org/message-id/20180618215635.m5vrnxdxhxytvmcm@alap3.anarazel.de
    > [2] https://www.postgresql.org/message-id/EF2313B8-A017-4869-9B7F-A24EDD8795DE%40gmail.com
    
    In-place updates are generally bad news, so I think this patch
    shouldn't use them. However, someone will need to investigate whether
    that breaks the indcheckxmin thing that Andres mentions in your [1],
    and if it does, figure out what to do about it. Creating a test case
    to show the breakage would probably be a good first step, to frame the
    discussion.
    
    -- 
    Robert Haas
    EDB: http://www.enterprisedb.com
    
    
    
    
  45. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Shayon Mukherjee <shayonj@gmail.com> — 2024-11-25T22:34:38Z

    > On Nov 5, 2024, at 10:55 AM, Robert Haas <robertmhaas@gmail.com> wrote:
    > 
    > On Thu, Oct 17, 2024 at 9:59 AM Shayon Mukherjee <shayonj@gmail.com> wrote:
    >> My take away from whether or not an in-place update is needed on pg_index [1]
    >> 
    >> - It’s unclear to me why it’s needed.
    >> - I understand the xmin would get incremented when using CatalogTupleUpdate to update indisenabled.
    >> - However, I haven’t been able to replicate any odd behavior locally or CI.
    >> - FWIW - REINDEX CONCURRENTLY (via index_swap),  index_constraint_create and few other places perform CatalogTupleUpdate to update the relevant attributes as well.
    >> 
    >> Based on the above summary and after my testing I would like to propose a v3 of the patch. The only difference between this and v1 [2] is that the update of pg_index row happens via CatalogTupleUpdate.
    >> 
    >> [1]  https://www.postgresql.org/message-id/20180618215635.m5vrnxdxhxytvmcm@alap3.anarazel.de
    >> [2] https://www.postgresql.org/message-id/EF2313B8-A017-4869-9B7F-A24EDD8795DE%40gmail.com
    > 
    > In-place updates are generally bad news, so I think this patch
    > shouldn't use them. However, someone will need to investigate whether
    > that breaks the indcheckxmin thing that Andres mentions in your [1],
    > and if it does, figure out what to do about it. Creating a test case
    > to show the breakage would probably be a good first step, to frame the
    > discussion.
    
    Hello,
    
    Thank you for the guidance and tips. I was wondering if updating in-place is preferable or not, since my first instinct was to avoid it. I did not notice any breakage last time, unless I was looking in the wrong place (possibly?). I did notice the min update when a not-in-place update was performed, but I didn't notice any issues (as mentioned in [1]) from it, via logs, index usage, or other common operations. Let me write up some more test cases to check if there is a risk of indexcheckxmin running out or other issues, and I'll get back here.
    
    Thank you
    Shayon
    
    [1] https://www.postgresql.org/message-id/20180618215635.m5vrnxdxhxytvmcm@alap3.anarazel.de
    
    > 
    > -- 
    > Robert Haas
    > EDB: http://www.enterprisedb.com
    
    
    
    
    
  46. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    David Rowley <dgrowleyml@gmail.com> — 2024-11-25T23:18:48Z

    On Tue, 26 Nov 2024 at 11:34, Shayon Mukherjee <shayonj@gmail.com> wrote:
    > Thank you for the guidance and tips. I was wondering if updating in-place is preferable or not, since my first instinct was to avoid it. I did not notice any breakage last time, unless I was looking in the wrong place (possibly?). I did notice the min update when a not-in-place update was performed, but I didn't notice any issues (as mentioned in [1]) from it, via logs, index usage, or other common operations. Let me write up some more test cases to check if there is a risk of indexcheckxmin running out or other issues, and I'll get back here.
    
    Another safer option could be to disallow the enable/disable ALTER
    TABLE if indcheckxmin is true. We do have and use
    ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE for these sorts of issues.
    There are other existing failure modes relating to indexes that can
    depend on what another session has done, e.g. MarkInheritDetached()
    can fail if another session detached an index concurrently. I could
    respect an argument that this one might be worse than that as its
    timing dependent rather than dependent on what another session has
    done.
    
    David
    
    
    
    
  47. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Shayon Mukherjee <shayonj@gmail.com> — 2024-12-06T16:24:41Z

    On Mon, Nov 25, 2024 at 6:19 PM David Rowley <dgrowleyml@gmail.com> wrote:
    
    > Another safer option could be to disallow the enable/disable ALTER
    > TABLE if indcheckxmin is true. We do have and use
    > ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE for these sorts of issues.
    > There are other existing failure modes relating to indexes that can
    > depend on what another session has done, e.g. MarkInheritDetached()
    > can fail if another session detached an index concurrently. I could
    > respect an argument that this one might be worse than that as its
    > timing dependent rather than dependent on what another session has
    > done.
    >
    
    Thank you for that feedback David. It's very useful.
    
    I have attached a v4 patch that now raises an error if indcheckxmin is true
    before attempting to enable/disable an index, and asks the caller to wait
    until indcheckxmin is balanced again via log message.
    
    I couldn't come up with any reliable (non-flaky) way of getting
    indcheckxmin to report true in regression specs for extra coverage. So, I
    ended up "simulating" it by directly updating the relevant row for an index
    and marking indcheckxmin as true in specs and accordingly asserting. The
    specs now cover all the previous cases and also the new case where the
    error would be raised if the caller attempts to enable/disable an index
    while indcheckxmin is true.
    
    The patch is also rebased against the latest master and passing in CI.
    Would love to receive any further feedback on it.
    
    Thank you everyone!
    Shayon
    
  48. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Shayon Mukherjee <shayonj@gmail.com> — 2024-12-28T16:09:42Z

    On Fri, Dec 6, 2024 at 11:24 AM Shayon Mukherjee <shayonj@gmail.com> wrote:
    
    > The patch is also rebased against the latest master and passing in CI.
    > Would love to receive any further feedback on it.
    >
    >
    Rebased the last patch against the latest master from today as a v5. No
    other changes since last post.
    
    Shayon
    
  49. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Mihail Nikalayeu <michail.nikolaev@gmail.com> — 2024-12-30T16:08:18Z

    Hello.
    
    A few comments on patch:
    
    > + temporarily reducing the overhead of index maintenance
    > +      during bulk data loading operations
    
    >
    But tuples are still inserted, where the difference come from?
    
    > or verifying an index is not being used
    > +      before dropping it
    
    Hm, it does not provide the guarantee - index may also be used as an
    arbiter for INSERT ON CONFLICT, for example. For that case, "update
    pg_index set indisvalid = false" should be used before the DROP, probably.
    Also index may also be used for constraint, part of partitioned table, etc.
    
    Also, I think it is better to move check to indisvalid as if
    (!index->indisvalid || !index->indisenabled).
    
    Best regards,
    Mikhail.
    
  50. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Shayon Mukherjee <shayonj@gmail.com> — 2024-12-30T17:33:02Z

    On Mon, Dec 30, 2024 at 11:08 AM Michail Nikolaev <
    michail.nikolaev@gmail.com> wrote:
    
    > Hello.
    >
    > A few comments on patch:
    >
    
    Thank you for the feedback.
    
    >
    > > + temporarily reducing the overhead of index maintenance
    > > +      during bulk data loading operations
    >
    >>
    >
    But tuples are still inserted, where the difference come from?
    >
    >
    I think in retrospect this wasn't needed. I likely conflated a few
    different use cases of this feature in the docs and I can see how it may
    get confusing. I was originally thinking of the scenario where the use case
    of this feature could allow users to identify indexes no longer needed and
    once dropped, it would simplify the above tasks. I have simplified
    the documentation now and removed that reference.
    
    
    
    > > or verifying an index is not being used
    > > +      before dropping it
    >
    > Hm, it does not provide the guarantee - index may also be used as an
    > arbiter for INSERT ON CONFLICT, for example. For that case, "update
    > pg_index set indisvalid = false" should be used before the DROP, probably.
    > Also index may also be used for constraint, part of partitioned table, etc.
    >
    
    Good catch, thank you! The feature is primarily geared towards the query
    planner, so we are intentionally not changing the enforcements of say
    constraints, including uniqueness, and the ones you mentioned above. I have
    updated the documentation to call out the distinction. Let me know if you
    think we can tweak the wording further.
    
    
    >
    > Also, I think it is better to move check to indisvalid as if
    > (!index->indisvalid || !index->indisenabled).
    >
    >
    Thank you!  I was a bit split in terms of code maintenance and quality, so
    originally I went with a dedicated block and a return/continue statement
    for index ->indisenabled . I have now updated the patch to perform if
    (!index->indisvalid || !index->indisenabled) and also updated the code
    comment accordingly.
    
    Rebased with the latest master as well.
    
    Thank you
    Shayon
    
  51. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Sami Imseih <samimseih@gmail.com> — 2024-12-30T19:56:01Z

    > Rebased with the latest master as well.
    
    Hi,
    
    This is a great, long needed feature. Thanks for doing this.
    
    I am late to this thread, but I took a look at the current patch
    and have some comments as I continue to look.
    
    1/
    instead of
    
    +       If true, the index is currently enabled and should be used for queries.
    +       If false, the index is disabled and should not be used for queries,
    
    how about?
    
    "If true, the index is currently enabled and may be used for queries.
    If false, the index is disabled and may not be used for queries,"
    
    "may" is more accurate than "should" in this context.
    
    2/
    instead of
    
    +       but is still maintained when the table is modified. Default is true.
    
    how about?
    
    "but is still updated when the table is modified. Default is true."
    
    "update" of an index is the current verb used. See bottom of
    https://www.postgresql.org/docs/current/indexes-intro.html
    
    3/
    instead of saying "used by the query planner for query optimization",
    can it just be "The index will be used for queries."
    
    +     <para>
    +      Enable the specified index. The index will be used by the query planner
    +      for query optimization. This is the default state for newly
    created indexes.
    +     </para>
    
    Same for
    
    +    <listitem>
    +     <para>
    +      Disable the specified index. A disabled index is not used by
    the query planner
    +      for query optimization, but it is still maintained when the
    underlying table
    +      data changes and will still be used to enforce constraints
    (such as UNIQUE,
    +      or PRIMARY KEY constraints).
    
    4/ Should documentation recommend a direct catalog update?
    
    +      to identify potentially unused indexes. Note that if you want
    to completely
    +      prevent an index from being used, including for constraint
    enforcement, you
    +      would need to mark it as invalid using a direct update to the
    system catalogs
    +      (e.g., <literal>UPDATE pg_index SET indisvalid = false WHERE
    indexrelid = 'index_name'::regclass</literal>).
    
    "indisvalid" does not control constraint enforcement in this case. It will be
    "indisready" being set to false that will.
    
    But even then, this goes against the general principle ( also documnted )
    of not updating the catalog directly. See [1]
    
    I think this part should be removed.
    
    5/
    
    In a case of a prepared statement, disabling the index
    has no effect.
    
    postgres=# create table foo ( id int primary key );
    CREATE TABLE
    postgres=# prepare prp as select * from foo where id = 1;
    PREPARE
    postgres=# explain analyze execute prp;
                                                        QUERY PLAN
    -------------------------------------------------------------------------------------------------------------------
     Index Only Scan using foo_pkey on foo  (cost=0.15..8.17 rows=1
    width=4) (actual time=0.018..0.019 rows=0 loops=1)
       Index Cond: (id = 1)
       Heap Fetches: 0
       Buffers: shared hit=2
     Planning:
       Buffers: shared hit=15 read=7
     Planning Time: 2.048 ms
     Execution Time: 0.071 ms
    (8 rows)
    
    postgres=# alter index foo_pkey disable ;
    ALTER INDEX
    postgres=# explain analyze execute prp;
                                                        QUERY PLAN
    -------------------------------------------------------------------------------------------------------------------
     Index Only Scan using foo_pkey on foo  (cost=0.15..8.17 rows=1
    width=4) (actual time=0.035..0.036 rows=0 loops=1)
       Index Cond: (id = 1)
       Heap Fetches: 0
       Buffers: shared hit=2
     Planning Time: 0.012 ms
     Execution Time: 0.320 ms
    (6 rows)
    
    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,
    
    Sami Imseih
    Amazon Web Services (AWS)
    
    
    [1] https://www.postgresql.org/docs/current/catalogs.html
    
    
    
    
  52. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Mihail Nikalayeu <michail.nikolaev@gmail.com> — 2024-12-30T20:48:39Z

    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.
    
    
    
    
  53. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Sami Imseih <samimseih@gmail.com> — 2024-12-31T03:52:12Z

    > 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
    
    
    
    
  54. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Shayon Mukherjee <shayonj@gmail.com> — 2024-12-31T16:52:16Z

    On Mon, Dec 30, 2024 at 2:56 PM Sami Imseih <samimseih@gmail.com> wrote:
    
    > > Rebased with the latest master as well.
    >
    > Hi,
    >
    > This is a great, long needed feature. Thanks for doing this.
    >
    > I am late to this thread, but I took a look at the current patch
    > and have some comments as I continue to look.
    >
    >
    Thank you so much for such detailed and useful feedback.
    
    
    > 1/
    > instead of
    >
    > +       If true, the index is currently enabled and should be used for
    > queries.
    > +       If false, the index is disabled and should not be used for queries,
    >
    > how about?
    > ....
    >
    
    It all makes sense to me. Thank you for the pointers to the docs and
    existing practices as well. I have updated the patch accordingly.
    
    
    >
    > "indisvalid" does not control constraint enforcement in this case. It will
    > be
    > "indisready" being set to false that will.
    >
    > But even then, this goes against the general principle ( also documnted )
    > of not updating the catalog directly. See [1]
    >
    > I think this part should be removed.
    >
    
    Makes sense and is fair. I also did not see examples of sharing queries in
    the docs either. Updated the patch accordingly.
    
    >
    > 5/
    >
    > In a case of a prepared statement, disabling the index
    > has no effect.
    >
    > ...
    
    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.
    >
    >
    oof! Great catch, can't believe I missed prepared statements 😅. I have
    updated the patch to ensure we are invalidating the heap relation the index
    is on from ATExecEnableDisableIndex and also backed it up with tests as
    well. It should also address your feedback and suggestion in [1]
    
    Thank you once again for the pointers and guidance. Attached v7 patch
    (rebased with latest master).
    Shayon
    
    [1]
    https://www.postgresql.org/message-id/CAA5RZ0sWpQZxjO9NEchwPeDbtw2HFWF-v8MscBDcm00FB6N_3w%40mail.gmail.com
    
  55. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Shayon Mukherjee <shayonj@gmail.com> — 2024-12-31T16:59:37Z

    On Mon, Dec 30, 2024 at 3:48 PM Michail Nikolaev <michail.nikolaev@gmail.com>
    wrote:
    
    > 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?
    >
    
    Hi!
    
    Thank you! I tested this by manually upgrading (using pg_upgrade) from
    master to the build from the branch, which ensures that post-upgrade the
    column for indisenabled is true by default. I also backed it up with bool
    indisenabled BKI_DEFAULT(t); in pg_index.h. Additionally, I tested
    upgrading from an old data directory to the new one (both on this patch) to
    ensure indexes with DISABLE properties are carried over as well on the new
    data directory/upgrade. For reference the latest patch now is in [1].
    
    Given this is working as expected, would we still need a migration step?
    (Let me know if I missed something ofc).
    
    For reference here is the setup from my local testing (for reference)
    
    rm -Rf /tmp/pg_data && rm -Rf /tmp/pg_data_new
    ./configure --prefix=/tmp/pg_install_old && make clean && make -j8 && make
    install
    
    # Create and init old cluster
    /tmp/pg_install_old/bin/initdb -D /tmp/pg_data
    /tmp/pg_install_old/bin/pg_ctl -D /tmp/pg_data start
    
    # Create test data
    /tmp/pg_install_old/bin/createdb test
    /tmp/pg_install_old/bin/psql test -c "CREATE TABLE foo (id int); CREATE
    INDEX idx_foo ON foo(id) DISABLE;"
    
    # Stop old cluster
    /tmp/pg_install_old/bin/pg_ctl -D /tmp/pg_data stop
    
    # Switch branch and build new version
    git checkout s/enable-disable-index
    ./configure --prefix=/tmp/pg_install_new && make clean && make -j8 && make
    install
    
    # Create new cluster directory
    /tmp/pg_install_new/bin/initdb -D /tmp/pg_data_new
    
    # Now run upgrade with different binary locations
    /tmp/pg_install_new/bin/pg_upgrade \
      -b /tmp/pg_install_old/bin \
      -B /tmp/pg_install_new/bin \
      -d /tmp/pg_data \
      -D /tmp/pg_data_new \
      -p 5432 \
      -P 5433
    
    /tmp/pg_install_new/bin/pg_ctl -D /tmp/pg_data_new start
    $ SELECT * FROM pg_index WHERE indexrelid = 'idx_foo'::regclass;
    
    Thank you
    Shayon
    [1]
    https://www.postgresql.org/message-id/CANqtF-qOtDDktykqSFQO%3DUrDyRuF4fKPBQFaYuY1Eo4M0J8cpA%40mail.gmail.com
    
  56. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Mihail Nikalayeu <michail.nikolaev@gmail.com> — 2025-01-01T14:58:11Z

    Hello!
    
    > Given this is working as expected, would we still need a migration step?
    
    No, it is clear now. Thanks for explaining.
    
    Best regards,
    Mikhail.
    
  57. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Sami Imseih <samimseih@gmail.com> — 2025-01-03T21:09:18Z

    +       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
    
    
    
    
  58. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Shayon Mukherjee <shayonj@gmail.com> — 2025-01-11T21:10:18Z

    On Fri, Jan 3, 2025 at 4:09 PM Sami Imseih <samimseih@gmail.com> wrote:
    
    > +       This is the
    > +      default state for newly created indexes.
    >
    > This is not needed in the ALTER INDEX docs, IMO.ss
    >
    > Updated and attached the patch.
    
    
    > 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.
    >
    
    I totally see where you are coming from. Some rough thoughts/notes:
    
    - The patch/proposed feature today doesn't disable constraints, like
    uniqueness. It only impacts query planning. Maybe it should ?
    - I was imagining this feature as being short-lived in production - that
    is, you disable a potential index to collect data on query performance and
    then make a decision on whether you need the index permanently. However,
    yes, one can always keep an index disabled for longer, and conditionally
    use it in another part of an application in which case a GUC to bypass the
    disabled/invisible index would come in handy as you mentioned.
    - I don't have a strong opinion either way, but I do wonder - considering
    that this GUC is an additive feature - if it's something worth implementing
    once we have more feedback from the usage (in v18 pre release, alpha, ec)
    of marking an index as disabled/invisible first? Or perhaps as a follow-up
    patch?
    
    If we do go with a GUC - is FORCE_INVISIBLE_INDEX a good name?
    
    
    >
    > +        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 ??
    >
    
    Updated and attached the patch.
    
    
    > 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?
    >
    >
    I agree. DISABLE doesn't sit right. I noticed INVISIBLE in MariaDB. I like
    HIDDEN/VISIBLE or ACTIVE/INACTIVE as well, since it impacts query planning.
    
    Happy to propose a patch with HIDDEN/VISIBLE if that sounds like a good
    idea.
    
    Thank you
    Shayon
    
  59. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Sami Imseih <samimseih@gmail.com> — 2025-01-11T22:50:29Z

    Thanks for the updates patch!
    
    >> 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.
    >
    >
    > I totally see where you are coming from. Some rough thoughts/notes:
    >
    > - The patch/proposed feature today doesn't disable constraints, like uniqueness. It only impacts query planning. Maybe it should ?
    > - I was imagining this feature as being short-lived in production - that is, you disable a potential index to collect data on query performance and then make a decision on whether you need the index permanently. However, yes, one can always keep an index disabled for longer, and conditionally use it in another part of an application in which case a GUC to bypass the disabled/invisible index would come in handy as you mentioned.
    > - I don't have a strong opinion either way, but I do wonder - considering that this GUC is an additive feature - if it's something worth implementing once we have more feedback from the usage (in v18 pre release, alpha, ec) of marking an index as disabled/invisible first? Or perhaps as a follow-up patch?
    >
    > If we do go with a GUC - is FORCE_INVISIBLE_INDEX a good name?
    >
    >>
    
    Here is a use-case where the GUC may be useful. I can see a user
    wanting to try out the index before committing to using it across the
    board. They can create the index as invisible and force using
    it in a specific part of the application. If they are happy with the results,
    they can make it visible. This is similar to but not exactly what HypoPG [1]
    does. HypoPG does not actually create the index and can only be used
    with EXPLAIN ( not EXPLAIN ANALYZE ) in a specific session. I see the
    ability to test on a real index may be more useful. Maybe others have
    other thoughts on this?
    
    > I agree. DISABLE doesn't sit right. I noticed INVISIBLE in MariaDB. I like HIDDEN/VISIBLE or ACTIVE/INACTIVE as well, since it impacts query planning.
    
    Let's see if other have an opinion on this, but VISIBLE/INVISIBLE
    seem the best way to indicate that the indexes are visible or invisible
    from the optimizer. ACTIVE/INACTIVE sound a lot like ENABLE/DISABLE.
    
    [1] https://github.com/HypoPG/hypopg
    
    Regards,
    
    Sami
    
    
    
    
  60. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Shayon Mukherjee <shayonj@gmail.com> — 2025-01-12T19:58:14Z

    On Sat, Jan 11, 2025 at 5:50 PM Sami Imseih <samimseih@gmail.com> wrote:
    
    > Here is a use-case where the GUC may be useful. I can see a user
    > wanting to try out the index before committing to using it across the
    > board. They can create the index as invisible and force using
    > it in a specific part of the application. If they are happy with the
    > results,
    > they can make it visible. This is similar to but not exactly what HypoPG
    > [1]
    > does. HypoPG does not actually create the index and can only be used
    > with EXPLAIN ( not EXPLAIN ANALYZE ) in a specific session. I see the
    > ability to test on a real index may be more useful. Maybe others have
    > other thoughts on this?
    >
    >
    That's a good call out, thank you. I was mostly interested in the other use
    cases and I like how this bool GUC unlocks the use case you mentioned.
    Plus, also the ability to conditionally use indexes in session that are
    globally marked as invisible.
    
    
    > > I agree. DISABLE doesn't sit right. I noticed INVISIBLE in MariaDB. I
    > like HIDDEN/VISIBLE or ACTIVE/INACTIVE as well, since it impacts query
    > planning.
    >
    > Let's see if other have an opinion on this, but VISIBLE/INVISIBLE
    > seem the best way to indicate that the indexes are visible or invisible
    > from the optimizer. ACTIVE/INACTIVE sound a lot like ENABLE/DISABLE.
    >
    
    I like VISIBLE/INVISIBLE. Proposing a new patch with the following changes
    
    - We now have moved away from DISABLE/ENABLE grammar to VISIBLE/INVISIBLE.
    No change in functionality, it's all the same as before, just new grammar.
    Backed by regression specs like before too.
    - The column in pg_index is now called indisvisible.
    - Introduced a new GUC - use_invisible_index. When set to on, it will not
    respect the visibility state in pg_index (related to the above
    conversation).
    - When GUC is assigned/updated we accordingly reset plan cache. Backed this
    specs with.
    
    The patch is rebased against master and passes in CI. Happy to iterate on
    any feedback received.
    
    Thanks
    Shayon
    
  61. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Benoit Lobréau <benoit.lobreau@gmail.com> — 2025-01-24T10:32:52Z

    Hi,
    
    Thank you for the patch! I've had a need for this feature several times,
    so I appreciate the work you’ve put into it.
    
    I like the new name VISIBLE/INVISIBLE and the fact that it's a separate flag in
    pg_index (it's easy to monitor).
    
    I don’t feel qualified to provide an opinion on the code itself just yet.
    
    I did notice something in the command prototype:
    
    +ALTER INDEX [ IF EXISTS ] <replaceable
    class="parameter">name</replaceable> VISIBLE
    +ALTER INDEX [ IF EXISTS ] <replaceable
    class="parameter">name</replaceable> INVISIBLE
    
    it would probably be better as:
    
     +ALTER INDEX [ IF EXISTS ] <replaceable
    class="parameter">name</replaceable> {VISIBLE|INVISIBLE}
    
    The completion for the INVISIBLE / VISIBLE keyword is missing in psql.
    
    
    I also tested the ALTER command within a transaction, and it worked as I
    expected: the changes are transactional (possibly because you didn’t use
    systable_inplace_update_begin?).
    
    
    Additionally, I tried using the ALTER command on an index that supports
    a foreign key. As expected, delete and update operations on the referenced
    table became significantly slower. I was wondering if this behavior should
    be documented here.
    
    +      Make the specified index invisible. The index will not be used
    for queries.
    +      This can be useful for testing query performance with and
    without specific
    +      indexes.
    
    Maybe something like :
    
        The index will not be used for user or system queries (e.g., an index
        supporting foreign keys).
    
    I noticed that you mentionned checking pg_stat_user_indexes before using
    the query but it might not be enough?
    
    
    
    
  62. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Benoit Lobréau <benoit.lobreau@gmail.com> — 2025-01-24T14:20:19Z

    On Fri, Jan 24, 2025 at 11:32 AM Benoit Lobréau
    <benoit.lobreau@gmail.com> wrote:
    > The completion for the INVISIBLE / VISIBLE keyword is missing in psql.
    
    I think this should to the trick ?
    
    diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
    index 81cbf10aa28..43ea8e55fd0 100644
    --- a/src/bin/psql/tab-complete.in.c
    +++ b/src/bin/psql/tab-complete.in.c
    @@ -2393,7 +2393,8 @@ match_previous_words(int pattern_id,
            else if (Matches("ALTER", "INDEX", MatchAny))
                    COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME TO", "SET",
                                              "RESET", "ATTACH PARTITION",
    -                                         "DEPENDS ON EXTENSION", "NO
    DEPENDS ON EXTENSION");
    +                                         "DEPENDS ON EXTENSION", "NO
    DEPENDS ON EXTENSION",
    +                                         "INVISIBLE", "VISIBLE");
            else if (Matches("ALTER", "INDEX", MatchAny, "ATTACH"))
                    COMPLETE_WITH("PARTITION");
            else if (Matches("ALTER", "INDEX", MatchAny, "ATTACH", "PARTITION"))
    
    
    
    
  63. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Benoit Lobréau <benoit.lobreau@gmail.com> — 2025-01-24T15:26:23Z

    I also noticed that \d on an index doesn't warn about the invisible state
    whereas \d on a table does:
    
    [local]:5444 postgres@postgres=# SELECT indexrelid::regclass,
    indisvalid, indisvisible FROM pg_index WHERE indexrelid =
    'repli_pkey'::regclass \gx
    -[ RECORD 1 ]+-----------
    indexrelid   | repli_pkey
    indisvalid   | f
    indisvisible | f
    
    [local]:5444 postgres@postgres=# \d repli_pkey
          Index "public.repli_pkey"
     Column |  Type   | Key? | Definition
    --------+---------+------+------------
     i      | integer | yes  | i
    primary key, btree, for table "public.repli", invalid
    
    [local]:5444 postgres@postgres=# \d repli
                   Table "public.repli"
     Column |  Type   | Collation | Nullable | Default
    --------+---------+-----------+----------+---------
     i      | integer |           | not null |
     t      | text    |           |          |
    Indexes:
        "repli_pkey" PRIMARY KEY, btree (i) INVISIBLE INVALID
    Publications:
        "pub"
    
    The attached patch adds the flag.
    
    [local]:5444 postgres@postgres=# \d repli_pkey
          Index "public.repli_pkey"
     Column |  Type   | Key? | Definition
    --------+---------+------+------------
     i      | integer | yes  | i
    primary key, btree, for table "public.repli", invalid, invisible
    
  64. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Benoit Lobréau <benoit.lobreau@gmail.com> — 2025-01-30T13:51:24Z

    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
    
    
    
    
  65. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Sami Imseih <samimseih@gmail.com> — 2025-01-31T04:48:18Z

    > 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]
    
    What is being discussed here is different from what I can tell. This
    is referring
    to the index changing status ( visible/invisible ) and those changes being
    visible by another transaction.
    
    However, the current patch may be too restrictive. Why do we need
    an AccessExclusiveLock on the table to perform the change. We are
    only changing the catalog and not the underlying data. This is a lot like
    ALTER INDEX RENAME, which only takes a ShareUpdateExclusiveLock.
    Can we do the same here?
    
    I am also still reviewing and have a few comments on v9
    
    1/ Missing ATSimpleRecursion call in PrepCmd for
            case AT_SetIndexVisible:
            case AT_SetIndexInvisible:
    Without the recursion call, the visibility changes on a
    parent will not apply to the partitions. We are also
    missing tests for partitions.
    
    2/ In ATExecSetIndexVisibility
    
    Change:
    elog(ERROR, "could not find tuple for index %u", indexOid);
    
    To:
    elog(ERROR, "cache lookup failed for index %u", indexOid);
    
    I see both message formats are used all over the place,
    but in tablecmds.c, the "cache lookup" variant is the one
    used, so let's do that for consistency.
    
    3/ In ATExecSetIndexVisibility
    
            if (indexForm->indcheckxmin)
    +       {
    +               heap_freetuple(indexTuple);
    +               table_close(pg_index, RowExclusiveLock);
    +                ereport(ERROR,
    
    There is no need to close the table or free the tuple, as
    these are "cleaned up" when the transaction aborts.
    
    4/ In ATExecSetIndexVisibility
    
    I have a suggestion below:
    
    What about we open both heapOid and IndexRelationId
    at the start and close them at the end. Also,
    inside the block for indexForm->indisvisible != visible,
    do the work to invalidate the cache, invoke the post
    alter hook and increment the command counter. This will
    also allow us to get rid of the update boolean as well.
    
    What do you think?
    
    Regards,
    
    Sami
    
    
    
    
  66. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Shayon Mukherjee <shayonj@gmail.com> — 2025-02-01T07:35:25Z

    On Fri, Jan 31, 2025 at 10:18 AM Sami Imseih <samimseih@gmail.com> wrote:
    
    >
    > What is being discussed here is different from what I can tell. This
    > is referring
    > to the index changing status ( visible/invisible ) and those changes being
    > visible by another transaction.
    >
    >
    +1. My vote would be to keep the behavior as is, at least for the first
    roll out of this feature.
    
    
    > However, the current patch may be too restrictive. Why do we need
    > an AccessExclusiveLock on the table to perform the change. We are
    > only changing the catalog and not the underlying data. This is a lot like
    > ALTER INDEX RENAME, which only takes a ShareUpdateExclusiveLock.
    > Can we do the same here?
    >
    >
    Makes sense, I think this ended up being a remananent from one of the first
    iterations. Updated it in the attached v10 patch.
    
    
    > 1/ Missing ATSimpleRecursion call in PrepCmd for
    >         case AT_SetIndexVisible:
    >         case AT_SetIndexInvisible:
    > Without the recursion call, the visibility changes on a
    > parent will not apply to the partitions. We are also
    > missing tests for partitions.
    >
    >
    Great catch! Thank you!! Updated the patch with the support for partitions
    and backed by regression specs.
    
    2/ In ATExecSetIndexVisibility
    >
    > Change:
    > elog(ERROR, "could not find tuple for index %u", indexOid);
    >
    > To:
    > elog(ERROR, "cache lookup failed for index %u", indexOid);
    >
    
    Good eye! I missed this :).
    
    
    >
    > 3/ In ATExecSetIndexVisibility
    >
    >         if (indexForm->indcheckxmin)
    > +       {
    > +               heap_freetuple(indexTuple);
    > +               table_close(pg_index, RowExclusiveLock);
    > +                ereport(ERROR,
    >
    > There is no need to close the table or free the tuple, as
    > these are "cleaned up" when the transaction aborts.
    >
    > 4/ In ATExecSetIndexVisibility
    >
    > I have a suggestion below:
    >
    > What about we open both heapOid and IndexRelationId
    > at the start and close them at the end. Also,
    > inside the block for indexForm->indisvisible != visible,
    > do the work to invalidate the cache, invoke the post
    > alter hook and increment the command counter. This will
    > also allow us to get rid of the update boolean as well.
    >
    > What do you think?
    >
    
    Thank you for the suggestions!! I was err'ing a bit on the "defensive" side
    being new to this area, however your suggestions are much more simpler. I
    have updated the patch accordingly.
    
    I have also added support for suggestions from the earlier messages, I will
    respond there accordingly.
    
    Thank you!
    Shayon
    
  67. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Shayon Mukherjee <shayonj@gmail.com> — 2025-02-01T07:41:00Z

    On Fri, Jan 24, 2025 at 4:03 PM Benoit Lobréau <benoit.lobreau@gmail.com>
    wrote:
    
    > I did notice something in the command prototype:
    >
    > +ALTER INDEX [ IF EXISTS ] <replaceable
    > class="parameter">name</replaceable> VISIBLE
    > +ALTER INDEX [ IF EXISTS ] <replaceable
    > class="parameter">name</replaceable> INVISIBLE
    >
    > it would probably be better as:
    >
    >  +ALTER INDEX [ IF EXISTS ] <replaceable
    > class="parameter">name</replaceable> {VISIBLE|INVISIBLE}
    >
    >
    Thank you for your review, really appreciate it! I have updated with your
    feedback in v10 patch [1]
    
    
    > The completion for the INVISIBLE / VISIBLE keyword is missing in psql.
    >
    >
    Also updated in v10 patch [1]
    
    
    > Additionally, I tried using the ALTER command on an index that supports
    > a foreign key. As expected, delete and update operations on the referenced
    > table became significantly slower. I was wondering if this behavior should
    > be documented here.
    >
    > +      Make the specified index invisible. The index will not be used
    > for queries.
    > +      This can be useful for testing query performance with and
    > without specific
    > +      indexes.
    >
    > Maybe something like :
    >
    >     The index will not be used for user or system queries (e.g., an index
    >     supporting foreign keys).
    >
    > I noticed that you mentionned checking pg_stat_user_indexes before using
    > the query but it might not be enough?
    >
    
    This part of the documentation has gone through some changes, and I have
    sensed it's hard to convey the details without complicating or breaking
    precedence. By saying "The index will not be used for queries", I (as a
    PostgreSQL user) was assuming this would apply to both user and system
    queries, and hence the distinction was implicit. However, I don't have a
    strong opinion and am happy to make the changes. I am also curious if
    anyone else has thoughts on this as well?
    
    [1]
    https://www.postgresql.org/message-id/CANqtF-rs5N4ZepiH9YV1VVi6YNtyFDzDbJabhcZKqh-hNgLmgw%40mail.gmail.com
    
    
    Thank you
    Shayon
    
  68. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Shayon Mukherjee <shayonj@gmail.com> — 2025-02-01T07:44:10Z

    On Fri, Jan 24, 2025 at 8:56 PM Benoit Lobréau <benoit.lobreau@gmail.com>
    wrote:
    
    > I also noticed that \d on an index doesn't warn about the invisible state
    > whereas \d on a table does:
    >
    
    Thank you for the review + patch (v9-002) [1]. Your patch looks good to me.
    I have not incorporated this in my v10 patch [2]. Mostly to make sure you
    are credited and also being new here and not knowing whether or not I
    should :) (to help with the reviewing process, etc). Open to suggestions
    and feedback.
    
    [1]
    https://www.postgresql.org/message-id/CAPE8EZ5G%2BCZiw%3Dp1Cs7DOZ2MGLa1yTS8Tk%3DThzi1F14N2A%3D1oQ%40mail.gmail.com
    
    [2]
    https://www.postgresql.org/message-id/CANqtF-rs5N4ZepiH9YV1VVi6YNtyFDzDbJabhcZKqh-hNgLmgw%40mail.gmail.com
    
    Thank you
    Shayon
    
  69. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    jian he <jian.universality@gmail.com> — 2025-02-02T09:41:15Z

    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.
    
    
    
    
  70. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    jian he <jian.universality@gmail.com> — 2025-02-03T02:45:43Z

    hi.
    ....
    modules/test_ddl_deparse/test_ddl_deparse.so.p/test_ddl_deparse.c.o -c
    ../../Desktop/pg_src/src7/postgres/src/test/modules/test_ddl_deparse/test_ddl_deparse.c
    ../../Desktop/pg_src/src7/postgres/src/test/modules/test_ddl_deparse/test_ddl_deparse.c:
    In function ‘get_altertable_subcmdinfo’:
    ../../Desktop/pg_src/src7/postgres/src/test/modules/test_ddl_deparse/test_ddl_deparse.c:111:17:
    error: enumeration value ‘AT_SetIndexVisible’ not handled in switch
    [-Werror=switch]
      111 |                 switch (subcmd->subtype)
          |                 ^~~~~~
    ../../Desktop/pg_src/src7/postgres/src/test/modules/test_ddl_deparse/test_ddl_deparse.c:111:17:
    error: enumeration value ‘AT_SetIndexInvisible’ not handled in switch
    [-Werror=switch]
    cc1: all warnings being treated as errors
    
    
    so we need to change test_ddl_deparse.c.
    The attached patch fixes the indentation and test_ddl_deparse.c issue.
    Maybe we can add some tests on src/test/modules/test_ddl_deparse.
    
  71. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Shayon Mukherjee <shayonj@gmail.com> — 2025-02-04T09:44:43Z

    On Sun, Feb 2, 2025 at 3:11 PM jian he <jian.universality@gmail.com> wrote:
    
    > hi.
    > the following reviews based on
    > v10-0001-Introduce-the-ability-to-set-index-visibility-us.patch.
    >
    > Thank you for the amazing review!!
    
    
    > in src/test/regress/sql/create_index.sql
    > seems there are no sql tests for "create index ... invisible"?
    >
    >
    > Good call, added in v11 patch (attached)
    
    
    > "Make the specified index visible. The index can be used for query
    > planning"
    > ?
    >
    
    Done in v11 patch.
    
    
    >
    > Do we need to add GUC use_invisible_index to postgresql.conf.sample?
    >
    >
    I wasn't sure at first, hence opted for GUC_NOT_IN_SAMPLE when introducing
    the GUC. I have added the new GUC in postgresql.conf.sample as part of the
    v11 patch.
    
    
    > 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.
    >
    >
    Thank you! I was relying on the existing specs to guide me on cases like
    this. That said - now I have fixed this in the v11 patch and also added
    regression specs for the same (future proofing and all).
    
    
    > ....
    > +    createFlags = INDEX_CREATE_SKIP_BUILD | INDEX_CREATE_CONCURRENT;
    > +    if (indexForm->indisvisible)
    > +            createFlags |= INDEX_CREATE_VISIBLE;
    > the indentation level seems also not right?
    >
    >
    >
    Thank you. I have struggled with indentation in the project a bit. I have
    gone ahead and fixed these, but I would love to know how do folks generally
    solve this and if they use any linting tools? I just use VScode, so my
    tooling may not be right for local dev.
    
    
    > INVISIBLE, VISIBLE is not special words, in gram.y, you don't need
    > "VISIBLE_P", "INVISIBLE_P", you can just use "INVISIBLE", "VISIBLE"
    > ?
    >
    > Got it, thank you and updated!
    
    
    > ....
    >
    > 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.
    >
    >
    Great catch! I am learning that handling of primary keys and constraint +
    indexes behave slightly differently in terms of logic across the codebase.
    I have updated the v11 patch with the fixes to ensure that pg_dump will
    respect the index visibility status on primary keys and also followed up
    with specs in 002_pg_dump.pl as a way to future proof the behavior. It was
    nice to learn more on how testing inside the .pl specs work as well.
    
    
    > I am not sure if we need to change index_concurrently_swap or not.
    > but many other pg_index columns changed.
    >
    
    My apologies, but I didn't fully follow this feedback. There are some specs
    in create_index.sql for REINDEX behavior when index visibility and I didn't
    notice any change in behavior in terms of query planning or the columns in
    pg_index. The only change I noticed was the rel id, which makes sense given
    the behavior of REINDEX. Once I understand the issue more, happy to follow
    up with fixes/specs accordingly.
    
    Few additional notes
    
    - The v11 patch now shows the index invisibility status when you do \d
    index_name. h/t to Benoit Lobréau for the patch [1]
    - The v11 patch also updated test_ddl_deparse.c as mentioned in [2] and
    also brings in new specs in
    src/test/modules/test_ddl_deparse/expected/alter_index.out as a follow up.
      - I found it interesting that this wasn't caught in specs in CI or
    anywhere else and I think it is dependent on the clang flags (?). Anyways,
    sharing for posterity. For now there is spec coverage for future cases in
    v11 patch.
    
    [1]
    https://www.postgresql.org/message-id/CAPE8EZ5G%2BCZiw%3Dp1Cs7DOZ2MGLa1yTS8Tk%3DThzi1F14N2A%3D1oQ%40mail.gmail.com
    [2]
    https://www.postgresql.org/message-id/CACJufxHROE2pDRYecnts9u12K-2R3AGhFADEw_C-GNiRWKZ6ig%40mail.gmail.com
    
  72. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    jian he <jian.universality@gmail.com> — 2025-02-08T05:40:52Z

    hi.
    ```
    drop table if exists idxpart;
    create table idxpart (a int, b int, c text) partition by range (a);
    create table idxpart1 (like idxpart);
    alter table idxpart attach partition idxpart1 for values from (0) to (10);
    
    create index idxpart_c on only idxpart (c) invisible;
    create index idxpart1_c on idxpart1 (c);
    
    alter index idxpart_c attach partition idxpart1_c;
    ```
    In this case, should ALTER INDEX ATTACH PARTITION change the attached
    partition(idxpart1_c)'s "visible" status?
    
    
    attached is a minor patch about indentation change and meson.build change.
    
  73. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Shayon Mukherjee <shayonj@gmail.com> — 2025-02-23T15:41:11Z

    On Sat, Feb 8, 2025 at 12:41 AM jian he <jian.universality@gmail.com> wrote:
    
    > hi.
    > ```
    > drop table if exists idxpart;
    > create table idxpart (a int, b int, c text) partition by range (a);
    > create table idxpart1 (like idxpart);
    > alter table idxpart attach partition idxpart1 for values from (0) to (10);
    >
    > create index idxpart_c on only idxpart (c) invisible;
    > create index idxpart1_c on idxpart1 (c);
    >
    > alter index idxpart_c attach partition idxpart1_c;
    > ```
    > In this case, should ALTER INDEX ATTACH PARTITION change the attached
    > partition(idxpart1_c)'s "visible" status?
    >
    >
    Hi,
    That is a great question and I have really gone back and forth on this one
    and here's my reasoning so far
    
    1. When you don't use ONLY:
       - The index of child table inherits the visibility of the parent table's
    index
       - This applies whether the parent index is set as INVISIBLE or VISIBLE
       - This automatic inheritance is expected behavior and feels natural
    
    2. When you use ONLY:
       - You as a user/developer are explicitly taking control of index
    management
       - Creating an index for parent as INVISIBLE and another for child as
    VISIBLE represents conscious, deliberate choices
       - When attaching these indexes, it makes sense to respect these explicit
    visibility settings
       - Silently overriding the child index's visibility could violate the
    Principle of Least Surprise
       - Lastly, this model also allows more granular control over index
    visibility for each partition
    
    I am not strongly tied to either of these options and very much open to
    changing my mind. Also happy to try and document this for more clarity.
    
    I have rebased the patch on top of master (resolving some merge conflicts),
    along with the meson changes (thank you for that).
    
    Thanks,
    Shayon
    
  74. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Shayon Mukherjee <shayonj@gmail.com> — 2025-03-07T07:21:34Z

    On Sun, Feb 23, 2025 at 3:41 PM Shayon Mukherjee <shayonj@gmail.com> wrote:
    
    > I have rebased the patch on top of master (resolving some merge
    > conflicts), along with the meson changes (thank you for that).
    >
    
    Rebased against the latest master and attaching the v13 patch.
    
    Thank you
    Shayon
    
  75. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Shayon Mukherjee <shayonj@gmail.com> — 2025-03-16T15:04:56Z

    Hi Hackers,
    
    Just leaving a quick note: I know this patch has through a lot of variations. I am keen on shipping this for v18 if possible, and while it’s a learning process for me, I am more than happy to iterate on any feedback. Let me know if there is anything I can do to help to keep the momentum going on this (absolutely no rush however). Just seeking feedback :).
    
    Thank you
    Shayon
    
    > On Mar 7, 2025, at 2:21 AM, Shayon Mukherjee <shayonj@gmail.com> wrote:
    > 
    > On Sun, Feb 23, 2025 at 3:41 PM Shayon Mukherjee <shayonj@gmail.com <mailto:shayonj@gmail.com>> wrote:
    >> I have rebased the patch on top of master (resolving some merge conflicts), along with the meson changes (thank you for that).
    > 
    > Rebased against the latest master and attaching the v13 patch.
    > 
    > Thank you
    > Shayon 
    > <v13-0001-Introduce-the-ability-to-enable-disable-indexes-.patch>
    
    
  76. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Sami Imseih <samimseih@gmail.com> — 2025-04-01T18:41:17Z

    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)
    
  77. Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)

    Shayon Mukherjee <shayonj@gmail.com> — 2025-04-01T22:50:28Z

    On Tue, Apr 1, 2025 at 2:41 PM Sami Imseih <samimseih@gmail.com> wrote:
    
    > 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.
    >
    >
    My apologies, I rebased off an old commit and that's how we lost the GUC
    change and also the rename from ENABLE/DISABLE to VISIBLE/INVISIBLE. I have
    brought it all back, in addition to the specs and other changes mentioned
    in v12.  Let me know if you have any feedback, more than happy to
    incorporate them.
    
    Also, thank you for letting me know of pgindent, very handy!
    
    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.
    >
    
    I really appreciate the reviews and guidance, thank you! Would love if this
    can become part of v18 release. No worries if not of course. I am
    definitely around to help get this production ready and also if any issues
    arise after.
    
    Thank you
    Shayon
    
  78. Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX

    Gurjeet Singh <gurjeet@singh.im> — 2025-04-02T05:55:57Z

    On Sun, Sep 22, 2024 at 3:45 PM David Rowley <dgrowleyml@gmail.com> wrote:
    
    > I think the documents should also mention that disabling an index is a
    > useful way to verify an index is not being used before dropping it as
    > the index can be enabled again at the first sign that performance has
    > been effected. (It might also be good to mention that checking
    > pg_stat_user_indexes.idx_scan should be the first port of call when
    > checking for unused indexes)
    
    While reviewing Shayon's v14 patch, I had removed text (quoted below) from the
    ALTER INDEX docs that did not feel right in a command reference. I thought
    of reading up on the history/discussion of the patch, and now I see why Shayon
    chose to include an advice in ALTER INDEX docs.
    
    > + indexes. If performance degrades after making an index invisible, it can be easily
    > + be made visible using <literal>VISIBLE</literal>. Before making an index invisible, it's recommended
    > + to check <structname>pg_stat_user_indexes</structname>.<structfield>idx_scan</structfield>
    > + to identify potentially unused indexes.
    
    I feel ALTER INDEX command reference doc is not the right place for this kind of
    operational advice. Is there a better place in documentation for this kind of
    advice? Or maybe it needs to be reworded to fit the command reference style?
    
    Best regards,
    Gurjeet
    http://Gurje.et
    
    
    
    
  79. Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX

    Sami Imseih <samimseih@gmail.com> — 2025-04-03T01:58:49Z

    > > + indexes. If performance degrades after making an index invisible, it can be easily
    > > + be made visible using <literal>VISIBLE</literal>. Before making an index invisible, it's recommended
    > > + to check <structname>pg_stat_user_indexes</structname>.<structfield>idx_scan</structfield>
    > > + to identify potentially unused indexes.
    >
    > I feel ALTER INDEX command reference doc is not the right place for this kind of
    > operational advice. Is there a better place in documentation for this kind of
    > advice? Or maybe it needs to be reworded to fit the command reference style?
    
    I agree with you.
    
    What about we add this wording in the following section [0]? This
    section discusses techniques
    for experimenting with indexes. It says,
    ".... A good deal of experimentation is often necessary. The rest of
    this section gives some tips for that:...."
    
    A discussion about invisible indexes as one of the tools for
    experimentation can be added here.
    What do you think?
    
    [0] https://www.postgresql.org/docs/current/indexes-examine.html
    
    --
    Sami Imseih
    Amazon Web Services (AWS)
    
    
    
    
  80. Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX

    Gurjeet Singh <gurjeet@singh.im> — 2025-04-03T02:16:01Z

    On Wed Apr 2, 2025 at 6:58 PM PDT, Sami Imseih wrote:
    >> > + indexes. If performance degrades after making an index invisible, it can be easily
    >> > + be made visible using <literal>VISIBLE</literal>. Before making an index invisible, it's recommended
    >> > + to check <structname>pg_stat_user_indexes</structname>.<structfield>idx_scan</structfield>
    >> > + to identify potentially unused indexes.
    >>
    >> I feel ALTER INDEX command reference doc is not the right place for this kind of
    >> operational advice. Is there a better place in documentation for this kind of
    >> advice? Or maybe it needs to be reworded to fit the command reference style?
    >
    > I agree with you.
    >
    > What about we add this wording in the following section [0]? This
    > section discusses techniques
    > for experimenting with indexes. It says,
    > ".... A good deal of experimentation is often necessary. The rest of
    > this section gives some tips for that:...."
    >
    > A discussion about invisible indexes as one of the tools for
    > experimentation can be added here.
    > What do you think?
    >
    > [0] https://www.postgresql.org/docs/current/indexes-examine.html
    
    That seems like a very good location for this advice. But the current
    set of bullet points are all directed towards "... a general procedure
    for determining which indexes to create". I propose that a new paragrph,
    not a bullet point, be added towards the end of that section which
    addresses the options of steps to take before dropping an index.
    Something like the following:
    
    Sometimes you may notice that an index is not being used anymore by the
    application queries. In such cases, it is a good idea to investigate if
    such an index can be dropped, because an index that is not being used
    for query optimization still consumes resources and slows down INSERT,
    UPDATE, and DELETE commands. To aid in such an investigation, look at
    the pg_stat_user_indexes.idx_scan count for the index.
    
    To determine the performance effects of dropping the index, without
    actually dropping the said index, you may mark the index invisible to
    the planner by using the ALTER INDEX ... INVISIIBLE command. If it turns
    out that doing so causes a performance degradation, the index can be
    quickly made visible to the planner for query optimization by using the
    ALTER INDEX ... VISIBLE command.
    
    Thoughts?
    
    Best regards,
    Gurjeet
    http://Gurje.et
    
    
    
    
    
  81. Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX

    Sami Imseih <samimseih@gmail.com> — 2025-04-03T02:53:17Z

    > That seems like a very good location for this advice. But the current
    > set of bullet points are all directed towards "... a general procedure
    > for determining which indexes to create". I propose that a new paragrph,
    > not a bullet point, be added towards the end of that section which
    > addresses the options of steps to take before dropping an index.
    > Something like the following:
    
    > Thoughts?
    
    This new feature provides the ability to experiment with indexes to
    create ( or drop ),
    so I don't think it deviates from the purpose of this paragraphs.
    
    This patch will provide the ability for the user to create an index as initially
    invisible and a GUC, use_invisible_index if set to TRUE to experiment with
    the new index to see if it improves performance. So, I think providing this
    pattern to experiment with a new index will fit nicely as a new bulletpoint.
    
    --
    Sami Imseih
    Amazon Web Services (AWS)
    
    
    
    
  82. Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX

    Shayon Mukherjee <shayonj@gmail.com> — 2025-04-05T21:00:59Z

    On Wed, Apr 2, 2025 at 10:53 PM Sami Imseih <samimseih@gmail.com> wrote:
    
    > > That seems like a very good location for this advice. But the current
    > > set of bullet points are all directed towards "... a general procedure
    > > for determining which indexes to create". I propose that a new paragrph,
    > > not a bullet point, be added towards the end of that section which
    > > addresses the options of steps to take before dropping an index.
    > > Something like the following:
    >
    > > Thoughts?
    >
    > This new feature provides the ability to experiment with indexes to
    > create ( or drop ),
    > so I don't think it deviates from the purpose of this paragraphs.
    >
    > This patch will provide the ability for the user to create an index as
    > initially
    > invisible and a GUC, use_invisible_index if set to TRUE to experiment with
    > the new index to see if it improves performance. So, I think providing this
    > pattern to experiment with a new index will fit nicely as a new
    > bulletpoint.
    >
    >
    >
    Thank you for the feedback and pointers Sami and Gurjeet. Good call on [0]
    being a good place for operational advice. I have gone ahead and removed
    the  advice about "pg_stat_user_indexes.idx_scan"
    from doc/src/sgml/ref/alter_index.sgml and
    updated doc/src/sgml/indices.sgml to include a new bullet point with also a
    reference to use_invisible_index. Let me know how it sounds and if there is
    any feedback.
    
    Also, rebased.
    
    Thank you
    Shayon
    
    [0] https://www.postgresql.org/docs/current/indexes-examine.html
    
  83. Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX

    Sami Imseih <samimseih@gmail.com> — 2025-04-07T20:01:33Z

    Thanks for the update!
    
    The changes in v15 look good to me. The patch does need to be rebased,
    and I also think you should add a tab-complete for CREATE INDEX
    
    
    simseih@bcd07415af92 postgresql % git diff
    diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
    index 8e2eb50205e..f1853a68ccc 100644
    --- a/src/bin/psql/tab-complete.in.c
    +++ b/src/bin/psql/tab-complete.in.c
    @@ -3434,6 +3434,8 @@ match_previous_words(int pattern_id,
                             !TailMatches("POLICY", MatchAny, MatchAny,
    MatchAny, MatchAny, MatchAny) &&
                             !TailMatches("FOR", MatchAny, MatchAny, MatchAny))
                    COMPLETE_WITH("(");
    +       else if (TailMatches("*)"))
    +               COMPLETE_WITH("VISIBLE", "INVISIBLE");
    
            /* CREATE OR REPLACE */
            else if (Matches("CREATE", "OR"))
    
    IMO, with the above in place, this patch is RFC.
    
    --
    Sami Imseih
    Amazon Web Services (AWS)
    
    
    
    
  84. Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX

    Shayon Mukherjee <shayonj@gmail.com> — 2025-04-07T20:47:56Z

    On Mon, Apr 7, 2025 at 4:01 PM Sami Imseih <samimseih@gmail.com> wrote:
    
    > Thanks for the update!
    >
    > The changes in v15 look good to me. The patch does need to be rebased,
    > and I also think you should add a tab-complete for CREATE INDEX
    >
    >
    > simseih@bcd07415af92 postgresql % git diff
    > diff --git a/src/bin/psql/tab-complete.in.c
    > b/src/bin/psql/tab-complete.in.c
    > index 8e2eb50205e..f1853a68ccc 100644
    > --- a/src/bin/psql/tab-complete.in.c
    > +++ b/src/bin/psql/tab-complete.in.c
    > @@ -3434,6 +3434,8 @@ match_previous_words(int pattern_id,
    >                          !TailMatches("POLICY", MatchAny, MatchAny,
    > MatchAny, MatchAny, MatchAny) &&
    >                          !TailMatches("FOR", MatchAny, MatchAny, MatchAny))
    >                 COMPLETE_WITH("(");
    > +       else if (TailMatches("*)"))
    > +               COMPLETE_WITH("VISIBLE", "INVISIBLE");
    >
    >         /* CREATE OR REPLACE */
    >         else if (Matches("CREATE", "OR"))
    >
    > IMO, with the above in place, this patch is RFC.
    >
    >
    >
    Thank you Sami, really appreciate it!
    
    Attached v16 with feedback and rebased.
    
    Thanks
    Shayon
    
  85. Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX

    Sami Imseih <samimseih@gmail.com> — 2025-04-07T21:39:46Z

    > Attached v16 with feedback and rebased.
    
    Thanks, and I realized the original tab-complete I proposed
    was not entirely correct. I fixed it and also shortened the
    commit message.
    
    
    --
    Sami Imseih
    Amazon Web Services (AWS)
    
  86. Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX

    Shayon Mukherjee <shayonj@gmail.com> — 2025-04-08T11:48:43Z

    On Mon, Apr 7, 2025 at 5:39 PM Sami Imseih <samimseih@gmail.com> wrote:
    
    > > Attached v16 with feedback and rebased.
    >
    > Thanks, and I realized the original tab-complete I proposed
    > was not entirely correct. I fixed it and also shortened the
    > commit message.
    
    
    I was wondering about if the check needed to be more encompassing. Your
    proposal definitely makes sense, thank you!
    
    + else if (TailMatches("INDEX|CONCURRENTLY", "ON", MatchAny, "USING",
    MatchAny, "(*)")
    
    Shayon
    
  87. Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX

    jian he <jian.universality@gmail.com> — 2025-04-24T04:45:22Z

    hi.
    The following is a review of version 17.
    
    ATExecSetIndexVisibility
        if (indexForm->indisvisible != visible)
        {
            indexForm->indisvisible = visible;
            CatalogTupleUpdate(pg_index, &indexTuple->t_self, indexTuple);
            CacheInvalidateRelcache(heapRel);
            InvokeObjectPostAlterHook(IndexRelationId, indexOid, 0);
            CommandCounterIncrement();
        }
    I am slightly confused. if we already used
    CommandCounterIncrement();
    then we don't need CacheInvalidateRelcache?
    
    
    doc/src/sgml/ref/alter_index.sgml
      <para>
       <command>ALTER INDEX</command> changes the definition of an existing index.
       There are several subforms described below. Note that the lock level required
       may differ for each subform. An <literal>ACCESS EXCLUSIVE</literal>
    lock is held
       unless explicitly noted. When multiple subcommands are listed, the lock
       held will be the strictest one required from any subcommand.
    
    per the above para, we need mention that ALTER INDEX SET INVISIBLE|INVISIBLE
    only use ShareUpdateExclusiveLock?
    
    
    index_create is called in several places,
    most of the time, we use INDEX_CREATE_VISIBLE.
    if we define it as INDEX_CREATE_INVISIBLE rather than INDEX_CREATE_VISIBLE
    then argument flags required code changes would be less, (i didn't try
    it myself)
    
    
    Similar to get_index_isclustered,
    We can place GetIndexVisibility in
    src/backend/utils/cache/lsyscache.c,
    make it an extern function, so others can use it;
    to align with other function names,
    maybe rename it as get_index_visibility.
    
    
    create index v2_idx on v1(data) visible;
    is allowed,
    doc/src/sgml/ref/create_index.sgml
    <synopsis> section need to change to
    [ VISIBLE | INVISIBLE ]
    
    ?
    
    
    
    
  88. Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX

    jian he <jian.universality@gmail.com> — 2025-04-24T08:07:35Z

    hi, two more minor issues.
    
    src/bin/pg_dump/pg_dump.c
    if (fout->remoteVersion >= 180000)
    need change to
    if (fout->remoteVersion >= 190000)
    
    
    +-- Test index visibility with partitioned tables
    +CREATE TABLE part_tbl(id int, data text) PARTITION BY RANGE(id);
    +CREATE TABLE part1 PARTITION OF part_tbl FOR VALUES FROM (1) TO (100);
    +CREATE TABLE part2 PARTITION OF part_tbl FOR VALUES FROM (100) TO (200);
    +INSERT INTO part_tbl
    +SELECT g, 'data ' || g
    +FROM generate_series(1, 199) g;
    +CREATE INDEX idx_part_tbl ON part_tbl(data);
    +SELECT c.relname, i.indisvisible
    +FROM pg_index i
    +JOIN pg_class c ON i.indexrelid = c.oid
    +WHERE c.relname LIKE 'idx_part_tbl%'
    +ORDER BY c.relname;
    +   relname    | indisvisible
    +--------------+--------------
    + idx_part_tbl | t
    +(1 row)
    +
    
    This test seems not that good?
    "idx_part_tbl" is the partitioned index, we also need to show each
    partition index
    pg_index.indisvisible value?
    
    we can change it to
    --------
    CREATE TABLE part_tbl(id int, data text) PARTITION BY RANGE(id);
    CREATE TABLE part_tbl_1 PARTITION OF part_tbl FOR VALUES FROM (1) TO (100);
    CREATE TABLE part_tbl_2 PARTITION OF part_tbl FOR VALUES FROM (100) TO (200);
    CREATE INDEX ON part_tbl(data);
    SELECT c.relname, i.indisvisible
    FROM pg_index i
    JOIN pg_class c ON i.indexrelid = c.oid
    WHERE c.relname LIKE 'part_tbl%'
    ORDER BY c.relname;
    -----
    
    
    
    
  89. Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX

    Shayon Mukherjee <shayonj@gmail.com> — 2025-04-28T11:23:15Z

    On Thu, Apr 24, 2025 at 12:45 AM jian he <jian.universality@gmail.com>
    wrote:
    
    > hi.
    > The following is a review of version 17.
    >
    > ATExecSetIndexVisibility
    >     if (indexForm->indisvisible != visible)
    >     {
    >         indexForm->indisvisible = visible;
    >         CatalogTupleUpdate(pg_index, &indexTuple->t_self, indexTuple);
    >         CacheInvalidateRelcache(heapRel);
    >         InvokeObjectPostAlterHook(IndexRelationId, indexOid, 0);
    >         CommandCounterIncrement();
    >     }
    > I am slightly confused. if we already used
    > CommandCounterIncrement();
    > then we don't need CacheInvalidateRelcache?
    >
    >
    >
    Thank you for this catch. I misunderstood the behavior of the two and was
    performing both to avoid inconsistency between state within a transaction
    and cross session, but as you pointed out CommandCounterIncrement() helps
    achieve both. Updated.
    
    doc/src/sgml/ref/alter_index.sgml
    >   <para>
    >    <command>ALTER INDEX</command> changes the definition of an existing
    > index.
    >    There are several subforms described below. Note that the lock level
    > required
    >    may differ for each subform. An <literal>ACCESS EXCLUSIVE</literal>
    > lock is held
    >    unless explicitly noted. When multiple subcommands are listed, the lock
    >    held will be the strictest one required from any subcommand.
    >
    > per the above para, we need mention that ALTER INDEX SET
    > INVISIBLE|INVISIBLE
    > only use ShareUpdateExclusiveLock?
    >
    >
    I wasn't sure at first where to add the note about
    ShareUpdateExclusiveLock. But it looks like we have a precedent from RENAME
    in doc/src/sgml/ref/alter_index.sgml, so I have done the same for VISIBLE &
    INVISIBLE in doc/src/sgml/ref/alter_index.sgml as well.
    
    
    > index_create is called in several places,
    > most of the time, we use INDEX_CREATE_VISIBLE.
    > if we define it as INDEX_CREATE_INVISIBLE rather than INDEX_CREATE_VISIBLE
    > then argument flags required code changes would be less, (i didn't try
    > it myself)
    >
    
    Looks like the only change we would save is the one in
    src/backend/catalog/toasting.c. Rest of the code change/diffs would still
    be needed IIUC (if I understand correctly). This approach felt a bit
    ergonomical, hence opted for it, but happy to update. Let me know.
    
    
    >
    
    Similar to get_index_isclustered,
    > We can place GetIndexVisibility in
    > src/backend/utils/cache/lsyscache.c,
    > make it an extern function, so others can use it;
    > to align with other function names,
    > maybe rename it as get_index_visibility.
    >
    >
    I was a bit torn on this one and figured I wouldn't introduce it as it
    could be a bit of premature optimization, until there were more use cases
    (or maybe one more). Plus, I figured the next time we need this info, we
    could expose a more public function like get_index_visibility (given N=2, N
    being the number of callers). However, given you mentioned and spotted this
    as well, I have introduced get_index_visibility in the new patch now.
    
    
    
    > create index v2_idx on v1(data) visible;
    > is allowed,
    > doc/src/sgml/ref/create_index.sgml
    > <synopsis> section need to change to
    > [ VISIBLE | INVISIBLE ]
    >
    > ?
    >
    
    Updated to match the same pattern as the one in
    doc/src/sgml/ref/alter_index.sgml.
    
    Thank you for the feedback. I have also updated the feedback from [1] as
    well. Few extra notes:
    
    - Attached v18
    - Rebased against master
    - Updated the commit message
    - Updated the target remote version to now be fout->remoteVersion >= 190000
    - Using a UNION ALL query to show all indexes from part_tbl partitioned
    tables in the specs as noted in [1]. The query suggested in [1] wasn't
    encompassing all the indexes, hence the UNION ALL for WHERE i.indrelid =
    'part_tbl'::regclass::oid.
    
    [1]
    https://www.postgresql.org/message-id/CACJufxFS_M7nGvFiz-dUutaWb7RQxRMO97wC5ZezKW2ZsMQPQg@mail.gmail.com
    
    Thank you
    Shayon
    
  90. Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX

    Robert Treat <rob@xzilla.net> — 2025-06-05T20:14:32Z

    On Mon, Apr 28, 2025 at 7:23 AM Shayon Mukherjee <shayonj@gmail.com> wrote:
    > On Thu, Apr 24, 2025 at 12:45 AM jian he <jian.universality@gmail.com> wrote:
    >>
    > Thank you for the feedback. I have also updated the feedback from [1] as well. Few extra notes:
    >
    > - Attached v18
    > - Rebased against master
    > - Updated the commit message
    > - Updated the target remote version to now be fout->remoteVersion >= 190000
    > - Using a UNION ALL query to show all indexes from part_tbl partitioned tables in the specs as noted in [1]. The query suggested in [1] wasn't encompassing all the indexes, hence the UNION ALL for WHERE i.indrelid = 'part_tbl'::regclass::oid.
    >
    
    Having looked at this patch, I'm a bit surprised that it would be
    considered for commit; not that the work hasn't been done with rigor,
    but the implementation seems extremely obtuse for the common use cases
    that have been envisioned.
    
    As a primary example, imagine you have 2 indexes and you want to test
    if one index can handle the load via skip scans. With this feature, in
    order to do that SAFELY, you would need to first figure out how to
    ensure that the `force_invisible_index` GUC has been set to true
    across all possible backends, even though there seems like a general
    agreement that there isn't an easy way to do this (see comments around
    cached plans), and to make it more complicated, this needs to occur
    across all downstream replicas. Only then would it be safe to run the
    alter index to set your index invisible, at which point you could then
    test at the query/session level to determine which queries will be
    supportable without the risk of having your server(s) tank due to
    overload when you start getting hundreds of queries who plan has gone
    sideways. Ideally you would be able to do this in the opposite
    fashion; start with a session level guc that allows you to test in a
    controlled manner, and then if that works you start to roll that out
    across multiple sessions, and then to multiple servers, before
    eventually dropping the index.
    
    But that isn't the only gap; imagine if you want to test across 3 or
    more indexes; with this implementation, the "use invisible" flag is
    all or nothing, which again makes it difficult to work with;
    especially if you have multiple cases within the system that might
    make use of this feature (and people will surely run invisible indexes
    for weeks in production to ensure some random monthly report doesn't
    come along and cause trouble). I'm also skeptical of the idea that
    users need a way to add invisible indexes they can then test to see if
    they are useful because 1) this is basically how indexes already work,
    meaning if you add an index and it isn't useful, it doesn't get used,
    and 2) we have an extension (hypopg) which arguably provides this
    functionality without causing a bunch of i/o, and there isn't nearly
    the clamor to add this functionality in to core as there is for having
    a way to "soft drop" indexes. TBH, with this implementation, I can see
    people running with all indexes set invisible and
    force_invisible_index set to true, just to enable simple granular
    control when they need it.
    
    I know this thread is rather old and there doesn't seem to be full
    agreement on the ALTER vs GUC implementation idea, and even though I
    agree with the sentiment that the GUC system is little more than the
    "half-baked take on planner hints", the upside of GUC first
    implementations is that they tend to provide better usability than
    most grammer related implementations. Consider that any implementation
    which requires the use of ALTER statements (which this one does)
    undercuts its own usefulness because it adds significant operational
    risk in any attempt to use it just by the nature of ALTER leading to
    system-wide (including multi-server) changes, and while it feels like
    we often dismiss operational risk, those are exactly the folks who
    need this feature the most.
    
    P.S. I really do want to thank Shayon for sticking with this; I
    thought about saying that up front but it felt cliche, but I do think
    it is important to say it.
    
    Robert Treat
    https://xzilla.net
    
    
    
    
  91. Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX

    David Rowley <dgrowleyml@gmail.com> — 2025-06-05T22:48:20Z

    On Fri, 6 Jun 2025 at 08:14, Robert Treat <rob@xzilla.net> wrote:
    > I know this thread is rather old and there doesn't seem to be full
    > agreement on the ALTER vs GUC implementation idea, and even though I
    > agree with the sentiment that the GUC system is little more than the
    > "half-baked take on planner hints", the upside of GUC first
    > implementations is that they tend to provide better usability than
    > most grammer related implementations. Consider that any implementation
    > which requires the use of ALTER statements (which this one does)
    > undercuts its own usefulness because it adds significant operational
    > risk in any attempt to use it just by the nature of ALTER leading to
    > system-wide (including multi-server) changes, and while it feels like
    > we often dismiss operational risk, those are exactly the folks who
    > need this feature the most.
    
    Thanks for weighing in.
    
    In my mind, this feature is for "I'm almost 100% certain this index
    isn't needed, I want to make sure I'm 100% right in a way that I can
    quickly fix the ensuing chaos if I'm wrong".  It sounds like in your
    mind it's "I want to run some experiments to see if this index is
    needed or not". I think both have merit, but I think the former gets
    you closer to 100% certainty, as it'll be replicated to physical
    replica servers.
    
    I'd personally be looking at something like pg_stat_all_indexes
    instead of playing around with session-level GUC setting to figure out
    if an index was being used or not and I'd be looking to the ALTER
    TABLE once I'd seen nothing changing in pg_stat_all_indexes for some
    time period. I mean, what am I really going to do in session-level
    GUC? -- Run all possible queries that the application runs and check
    they're still fast? If I could do that, then I could equally just not
    use the GUC and look at EXPLAIN on all those queries to see if the
    index is picked anywhere.
    
    Maybe we need to hear from a few more people who have recently faced
    the dilemma of removing a seemingly unused index on a critical
    application.
    
    For me, I have been in this situation before. The database wasn't
    massive. I could likely have put the index back in 10 mins or so.
    However, it'd still have been nice to have something else to try
    before trying DROP INDEX. It's quite easy to imagine your finger
    hovering over the [Enter] key for a while before typing that statement
    when the index is large.
    
    > P.S. I really do want to thank Shayon for sticking with this;
    
    +1
    
    David
    
    
    
    
  92. Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX

    Sami Imseih <samimseih@gmail.com> — 2025-06-06T00:16:14Z

    > Thanks for weighing in.
    +1
    
    > In my mind, this feature is for "I'm almost 100% certain this index
    > isn't needed, I want to make sure I'm 100% right in a way that I can
    > quickly fix the ensuing chaos if I'm wrong".
    
    This is the primary use-case. A user performs an ALTER INDEX...
    INVISIBLE, and they monitor the workload and pg_stat_all_indexes
    ( on primary and hot standbys ) until they feel confident enough
    to fully commit to dropping the index. This is the case that many
    users out there want. The bonus is the locking acquired to flip
    the VISIBLE/INVISIBLE flag is a ShareUpdateExclusiveLock
    on the index, so this operation can only be blocked by VACUUM
    or other ALTERs, etc,
    
    > I'm also skeptical of the idea that
    > users need a way to add invisible indexes they can then test to see if
    > they are useful because 1) this is basically how indexes already work,
    > meaning if you add an index and it isn't useful, it doesn't get used,
    
    The GUC will be useful for experimentation or for the safer rollout of
    new indexes. For example, an index can be created as INVISIBLE initially,
    and with use_invisible_index, one can observe how the index may impact
    various queries before fully committing to enabling it. Also, if we allow an
    index to be INVISIBLE initially, we need to provide the user with this
    GUC; otherwise, I can’t see why a user would want to make an
    index INVISIBLE initially.
    
    > and 2) we have an extension (hypopg) which arguably provides this
    > functionality without causing a bunch of i/o, and there isn't nearly
    > the clamor to add this functionality in to core as there is for having
    > a way to "soft drop" indexes.
    
    I have not worked much with HypoPG, but from what I understand,
    it works only at the EXPLAIN level. It is purely an experimentation tool.
    However, the proposed GUC can also be used in more places,
    including, pg_hint_plan ( at least with the SET hint without any changes
    to pg_hint_plan).
    
    > > P.S. I really do want to thank Shayon for sticking with this;
    
    > +1
    +1
    
    --
    Sami Imseih
    Amazon Web Services (AWS)
    
    
    
    
  93. Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX

    Robert Treat <rob@xzilla.net> — 2025-06-06T02:32:39Z

    On Thu, Jun 5, 2025 at 8:16 PM Sami Imseih <samimseih@gmail.com> wrote:
    
    > > Thanks for weighing in.
    > +1
    >
    > > In my mind, this feature is for "I'm almost 100% certain this index
    > > isn't needed, I want to make sure I'm 100% right in a way that I can
    > > quickly fix the ensuing chaos if I'm wrong".
    >
    > This is the primary use-case. A user performs an ALTER INDEX...
    > INVISIBLE, and they monitor the workload and pg_stat_all_indexes
    > ( on primary and hot standbys ) until they feel confident enough
    > to fully commit to dropping the index. This is the case that many
    > users out there want.
    >
    
    To be blunt, the users who think they want this either aren't trying to
    solve the actual hard problem, or they haven't thought about how this
    operation needs to happen that deeply. Don't get me wrong, it would be an
    improvement to have some type of mechanism that can move you from almost
    100% to 100%, but the real problem is how do you SAFELY get to almost 100%
    in the first place? You need to be able to build that confidence through
    smaller incremental changes to your production workload, and ALTER INDEX
    won't help you with that. In production, you aren't watching to see what
    happen with pg_stat_all_indexes, because you will first be watching
    pg_stat_activity to see if the plans have flipped in some way that leads to
    an overloaded server (extra latency, poor caching effects, extra buffers
    usage, etc). And the replicated bit? Sadly someone launched some big DML
    operation so you're waiting for that to finish so the "quick rollback" can
    actually get to those other servers.
    
    
    > > I'm also skeptical of the idea that
    > > users need a way to add invisible indexes they can then test to see if
    > > they are useful because 1) this is basically how indexes already work,
    > > meaning if you add an index and it isn't useful, it doesn't get used,
    >
    > The GUC will be useful for experimentation or for the safer rollout of
    > new indexes. For example, an index can be created as INVISIBLE initially,
    > and with use_invisible_index, one can observe how the index may impact
    > various queries before fully committing to enabling it. Also, if we allow
    > an
    > index to be INVISIBLE initially, we need to provide the user with this
    > GUC; otherwise, I can’t see why a user would want to make an
    > index INVISIBLE initially.
    >
    >
    Again, I can squint enough to see the use case, but the risk with indexes
    is FAR greater in their removal rather than in adding new ones; and to
    whatever degree you think slow rolling out the generally not dangerous
    addition of new indexes is, it's an argument that should really speak to
    how much more important the ability to slow roll index removal is.
    
    
    > > and 2) we have an extension (hypopg) which arguably provides this
    > > functionality without causing a bunch of i/o, and there isn't nearly
    > > the clamor to add this functionality in to core as there is for having
    > > a way to "soft drop" indexes.
    >
    > I have not worked much with HypoPG, but from what I understand,
    > it works only at the EXPLAIN level. It is purely an experimentation tool.
    > However, the proposed GUC can also be used in more places,
    > including, pg_hint_plan ( at least with the SET hint without any changes
    > to pg_hint_plan).
    >
    >
    To be clear, the reason I bring up hypopg is that if slow rolling the
    addition of indexes was a significant customer problem, we'd have people
    clamoring for better tools to do it, and by and large we don't, and I posit
    that by and large that's because adding new indexes is not really that
    dangerous.
    
    I'm not saying there isn't any possible use case that could be solved with
    the above (although mind my example of people running with all indexes and
    the guc always enabled; I don't think thats a sceanrio that anyone thinks
    should be recommended, but it will be a far more common use case given this
    design; and btw it wont work well with pg_hint_plan because the GUC/ALTER
    combo doesn't play well with multiple indexes), but more importantly, if we
    only solve the simple cases at the expense of the hard problem, we're doing
    our users a disservice.
    
    
    Robert Treat
    https://xzilla.net
    
  94. Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX

    David G. Johnston <david.g.johnston@gmail.com> — 2025-06-06T03:41:02Z

    On Thu, Jun 5, 2025 at 7:32 PM Robert Treat <rob@xzilla.net> wrote:
    
    > I'm not saying there isn't any possible use case that could be solved with
    > the above (although mind my example of people running with all indexes and
    > the guc always enabled; I don't think thats a sceanrio that anyone thinks
    > should be recommended, but it will be a far more common use case given this
    > design; and btw it wont work well with pg_hint_plan because the GUC/ALTER
    > combo doesn't play well with multiple indexes), but more importantly, if we
    > only solve the simple cases at the expense of the hard problem, we're doing
    > our users a disservice.
    >
    >
    So, as proposed:
    
    Replicate-able DDL: Enables a holistic picture but is a foot-gun for the
    DBA in the "revert" case.
    
    Boolean GUC: Enables some experimentation; can be used to quickly re-enable
    invisible indexes that are waiting for the DDL to make them visible again.
    Insufficiently granular for quickly exploring various options.
    
    The granularity issue seems overcome-able:
    
    Multi-Valued GUC: Specify explicitly which invisible indexes to make
    visible, eliminating the granularity problem of the boolean option.  Can
    provide a "pg_catalog.pg_all_indexes" magic value impossible to exist in
    reality that would enable the "true" boolean option - false would just be
    an empty setting.
    
    The foot-gun seems safe enough to offer given the benefit the feature
    provides.
    
    Even without the GUC the proposed feature seems an improvement over the
    status quo.  The boolean I'd probably leave on the table; while a bit ugly
    in usage for the expected experimentation the multi-valued text GUC seems
    reasonable (and will effectively prohibit relying on invisible indexes
    generally).
    
    Are there other alternative designs this last bit of discussion is meant to
    promote or are people actively voting for the status quo over the addition
    of the index visibility attribute?  Or, maybe more properly, is index
    replication the dividing line here and any new feature has to make that
    aspect optional?
    
    If we are going to bite on the multi-valued text GUC it could just define
    which indexes to ignore when planning and we'd have the local-only feature
    done.  Which leads then to just implementing this feature (with
    multi-valued GUC) as the option by which the DBA can choose to apply their
    local GUC changes across their secondaries without having to (but can if
    they wish) apply the GUC change to all those machines as well.
    
    David J.
    
  95. Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX

    Sami Imseih <samimseih@gmail.com> — 2025-06-06T15:23:30Z

    > Don't get me wrong, it would be an improvement to have some type of
    > mechanism that can move you from almost 100% to 100%, but the real
    > problem is how do you SAFELY get to almost 100% in the first place?
    
    This big use case is precisely the "almost 100% to 100%"  confidence problem.
    Usually, users have done their homework, they've analyzed
    workloads, tuned queries and maybe created a better index. Now, they see some
    indexes that are unused or underused. In the current state, the only
    option is to drop the
    index. But if that turns out to be a mistake, they have to rebuild it, which
    can be slow and disruptive. With this feature, If making the index
    invisible causes
    problems, they can quickly make it visible again without needing to
    rebuild anything.
    
    Also, users coming from other databases, both commercial and open source, are
    already used to this kind of setup: an ALTER command for visibility, plus a
    parameter to control whether invisible indexes are used on a per session level.
    So we're not inventing something new here; we're following a well-known and
    useful pattern that makes life easier, especially for users migrating to
    Postgres.
    
    I am still trying to understand. Do you think the ALTER command is not useful?
    or, do you think the GUC is all we need and it should be more granular?
    or maybe something different?
    
    --
    Sami
    
    
    
    
  96. Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX

    David Rowley <dgrowleyml@gmail.com> — 2025-06-07T00:03:53Z

    On Fri, 6 Jun 2025 at 14:32, Robert Treat <rob@xzilla.net> wrote:
    > In production, you aren't watching to see what happen with pg_stat_all_indexes, because you will first be watching pg_stat_activity to see if the plans have flipped in some way that leads to an overloaded server (extra latency, poor caching effects, extra buffers usage, etc). And the replicated bit? Sadly someone launched some big DML operation so you're waiting for that to finish so the "quick rollback" can actually get to those other servers.
    
    I think you've misunderstood when you'd be looking at
    pg_stat_all_indexes. The time when you'd want to look at
    pg_stat_all_indexes is *before* you DROP INDEX and before you ALTER
    TABLE INVISIBLE the index. What you'd likely want to look for there
    are indexes that have the last_idx_scan set to something far in the
    past or set to NULL.
    
    I'm curious to know if you've ever had to drop an index out of
    production before? What did you think about when you'd just typed the
    DROP INDEX command and were contemplating your future? How long did
    you pause before pressing [Enter]?
    
    Can you list your proposed series of steps you'd recommend to a DBA
    wishing to remove an index, assuming this feature exists in core as
    you'd like it to?
    
    David
    
    
    
    
  97. Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX

    Robert Treat <rob@xzilla.net> — 2025-06-07T13:35:33Z

    On Fri, Jun 6, 2025 at 8:04 PM David Rowley <dgrowleyml@gmail.com> wrote:
    > On Fri, 6 Jun 2025 at 14:32, Robert Treat <rob@xzilla.net> wrote:
    > > In production, you aren't watching to see what happen with pg_stat_all_indexes, because you will first be watching pg_stat_activity to see if the plans have flipped in some way that leads to an overloaded server (extra latency, poor caching effects, extra buffers usage, etc). And the replicated bit? Sadly someone launched some big DML operation so you're waiting for that to finish so the "quick rollback" can actually get to those other servers.
    >
    > I think you've misunderstood when you'd be looking at
    > pg_stat_all_indexes. The time when you'd want to look at
    > pg_stat_all_indexes is *before* you DROP INDEX and before you ALTER
    > TABLE INVISIBLE the index. What you'd likely want to look for there
    > are indexes that have the last_idx_scan set to something far in the
    > past or set to NULL.
    >
    
    I guess you have never heard of the TREAT method of index management? :-D
    - Test for duplicate indexes
    - Reindex bloated indexes
    - Eliminate unused indexes
    - Add missing indexes
    - Tune indexes for generic queries
    
    The easy part of figuring out what to change, the hard part
    (sometimes) is getting those changes into production safely; that's
    the part I am focused on.
    
    > I'm curious to know if you've ever had to drop an index out of
    > production before? What did you think about when you'd just typed the
    > DROP INDEX command and were contemplating your future? How long did
    > you pause before pressing [Enter]?
    >
    
    ROFL... Uh... yes, I have had to do it at least a few times.
    
    So, years ago I used to say things like "I wish we had a way to make
    indexes invisible like they do in Oracle" on the regular; but as I
    worked through several different implementations and their potential
    effects, and had more and more exposure to more demanding Postgres
    installations, my thinking evolved. I spoke with Sami a bit about this
    off-list and he walked me through some of the Oracle documentation on
    this (I had, at best, forgot the specifics), which I think was helpful
    to better understand some of the allure of the alter index/guc method
    for many people who are used to it (and this current version of the
    implementation is very Oracle like), but it also crystalized my
    feeling that an Oracle-style implementation would be a red herring
    that can keep us from a better solution.
    
    > Can you list your proposed series of steps you'd recommend to a DBA
    > wishing to remove an index, assuming this feature exists in core as
    > you'd like it to?
    >
    
    Well, the series of steps differs depending on the nature of the
    system being managed. If you are running on a single node with normal
    traffic and resources, you just set the GUC to include the index you
    want to be invisible, wait for a few days (maybe no one runs monthly
    reports on this system?), take a quick look at your monitoring/stats
    to make sure things seem copacetic, and then you drop the index and
    reset the GUC.
    
    But of course the people who I am most worried about are the ones who
    are operating on high scale, high transaction, high connection,
    "mission critical" systems... ie. people operating in high risk
    environments, where things can go very bad very fast. Where safety
    considerations are a critical part of every deployment.
    
    In that type of environment, the GUC-only method enables you to
    control changes at very precise levels, so you can do things like:
    - run it ad-hoc at the session level to confirm that the explain plans
    you get in production match your expectations.
    - you can stay ad-hoc at the session level and run explain analyze and
    confirm acceptable performance within your workload, and see what kind
    of buffer impact you are going to have (typically overlooked, but a
    potential landmine for outages, but I'll come back to this)
    - because we are operating at the session level, we can then add this
    on a per query basis at the application level, and in really high
    traffic scenarios, you can use canary releases and/or feature flags to
    ramp up those new queries into the live system.
    - depending on how much risk you are concerned about, you can use this
    session level method across queries individually, or at some point
    roll it up to a user/application level. And again, we can roll it out
    to different users at different times if you want.
    - at some point when you feel confident that you have covered enough
    angles, you set the GUC globally and let that marinate for a few more
    weeks as needed.
    
    And the funny thing is, at this point, once you have the guc put in
    globally, and it's run for some number of weeks or months and everyone
    is confident, you don't actually need the ALTER INDEX part any more;
    you can just drop the index and be done with it. Now of course if you
    aren't running at this kind of scale or don't have this level of risk,
    you can speed run this a bit and go directly to the user level or skip
    right to adding it globally, so the ease of use is on par with using
    ALTER. But in any case where you do have elevated levels of risk, this
    is actually less steps (and less risk) that having to use the
    ALTER/guc method.
    
    Earlier I mentioned the idea of monitoring buffer impact; let's talk
    about that. I often hear people say that you should be doing things
    like confirming your explain plans in development or have some type of
    staging system where you do these kind of "experiments", as if a test
    on a secondary system could really give you absolute confidence when
    deploying to a system that automatically updates its settings (ie
    pg_stats) at semi-random times with randomly sampled values; but in
    any case, most people will at least agree that there is no way to
    match up buffer usage across machines. That means if we are making
    production changes that might have a significant impact on buffers, we
    are doing something inherently dangerous. Well, dropping an index is
    one of those things.
    
    Imagine a scenario where you have a large index on a column and a
    similar partial index on the same column, which are both used in
    production for different queries, and therefore taking up some amount
    of space within the buffer pool. When you make the partial index
    invisible, the index is still maintained, and therefore it likely
    still needs to maintain pages within the buffer pool to stay updated.
    However, with queries now shifting to the full index, the full index
    may very well need to pull in additional pages into the buffer pool
    that it didn't need before, and this action can cause other pages from
    some unknown object to get evicted. If you are lucky, this all works
    itself and nothing bad happens, if you aren't, you may end up with a
    server overloaded by latency in queries that aren't even related to
    the indexes you're working on. (If you have a hard time seeing it with
    partial indexes, the same can happen with consolidating indexes with
    different INCLUDE statements, and certainly will be a scenario when
    people look to drop indexes by way of skip-scan based plans). Now, is
    it possible to handle this with the ALTER/guc method? Well, you can
    mitigate it somewhat, but ironically to do so requires pushing out the
    guc part of the ALTER/guc to all the places you would have pushed out
    the GUC-only method, and that has to have been done BEFORE running
    ALTER INDEX, so what does it really buy you?
    
    I suppose while we're here, let me also make some observations about
    how these methods differ when dealing with replica clusters. You
    mentioned that one of the things you liked about the ALTER/guc method
    is that it replicates the changes across all systems which makes it
    easy to revert, however I believe that thinking is flawed. For
    starters, any change that has to occur across the WAL stream is not
    something that can be relied on to happen quickly; there are too many
    other items that traverse that space that could end up blocking a
    rollback from being applied in a timely fashion. The more complex the
    replica cluster, the worse this is. One very common use case is to run
    different workloads on different nodes, with the ALTER/guc method, you
    are forcing users to make changes on a primary when they want to
    target a workload that only runs on a replica. This means I have to
    account for all potential workloads on all clusters before I can
    safely start making changes, and to the degree that the ALTER/guc
    gives me a safety net, that safety net is... to deploy a guc globally,
    one at a time, on each individual server.
    
    I feel like this email is already long, and tbh I could go on even
    more, but hopefully I've covered enough to help explain some of the
    issues that are involved here. I'm not trying to say that GUC-only is
    a perfect solution, but I do think it handles every use case on par
    with ALTER/guc, and enables some use cases ALTER/guc can't, especially
    for people who have to operate in risk-first environments. And I get
    it that some people are going to want a thing that looks very simple
    or is familiar to how Oracle did it, but I can't help but think this
    is one of those cases like how people used to always ask us to
    implement UPSERT because that's what MySQL had, but instead we gave
    them INSERT ON CONFLICT because it was the better solution to the
    problem they (actually) had.
    
    
    Robert Treat
    https://xzilla.net
    
    
    
    
  98. Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX

    Sami Imseih <samimseih@gmail.com> — 2025-06-07T16:22:10Z

    > In that type of environment, the GUC-only method enables you to
    > control changes at very precise levels, so you can do things like:
    > - run it ad-hoc at the session level to confirm that the explain plans
    > you get in production match your expectations.
    > - you can stay ad-hoc at the session level and run explain analyze and
    > confirm acceptable performance within your workload, and see what kind
    > of buffer impact you are going to have (typically overlooked, but a
    > potential landmine for outages, but I'll come back to this)
    > - because we are operating at the session level, we can then add this
    > on a per query basis at the application level, and in really high
    > traffic scenarios, you can use canary releases and/or feature flags to
    > ramp up those new queries into the live system.
    > - depending on how much risk you are concerned about, you can use this
    > session level method across queries individually, or at some point
    > roll it up to a user/application level. And again, we can roll it out
    > to different users at different times if you want.
    > - at some point when you feel confident that you have covered enough
    > angles, you set the GUC globally and let that marinate for a few more
    > weeks as needed.
    
    Do we need this level of granular control in core, or should this be
    delegated to other tools in the ecosystem, like pg_hint_plan? The de
    facto tool for influencing planning.
    There is probably some work that must happen in that extension to make
    the use-cases above work, but it is something to consider.
    
    With that said, I am not really opposed to a multi-value GUC that takes
    in a list of index names, but I do have several concerns with that
    approach being available in core:
    
    1. The list of indexes getting too long, and the potential performance
    impact of having to translate the index name to a relid to find which
    index to make "invisible". I don't think a list of index relids will
    be good from a usability perspective either.
    
    2. A foot-gun such as adding an index name to my list, dropping the
    index, recreating it with the same name, and now my new index is not
    being used.
    
    3. not sync'd up with the replica, so manual work is required there. That
    could be seen as a positive aspect of this approach as well.
    
    4. The above points speak on the level of maintenance required for this.
    
    > You mentioned that one of the things you liked about the ALTER/guc method
    > is that it replicates the changes across all systems which makes it
    > easy to revert, however I believe that thinking is flawed. For
    > starters, any change that has to occur across the WAL stream is not
    > something that can be relied on to happen quickly; there are too many
    > other items that traverse that space that could end up blocking a
    > rollback from being applied in a timely fashion.
    
    This is not going to be unique to this feature though. Other critical
    DDLs will be blocked, so this is a different problem, IMO.
    
    > but it also crystalized my
    > feeling that an Oracle-style implementation would be a red herring
    > that can keep us from a better solution.
    
    Going back to this point, I still think that the ALTER option is useful
    after the user's confidence is near 100% and they are ready to drop
    the index for good, and which also gets replicated.
    
    The GUC is useful for experimentation or for users that want to do a
    slow rollout of dropping an index. We can discuss whether this should
    be a multi-value setting or a boolean in core, or if it should be
    delegated to an extension.
    
    Essentially, I don't think we need to choose one or the other, but
    perhaps we can improve upon the GUC.
    
    
    --
    Sami
    
    
    
    
  99. Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX

    David Rowley <dgrowleyml@gmail.com> — 2025-06-08T01:17:06Z

    On Sun, 8 Jun 2025 at 01:35, Robert Treat <rob@xzilla.net> wrote:
    >
    > On Fri, Jun 6, 2025 at 8:04 PM David Rowley <dgrowleyml@gmail.com> wrote:
    > > Can you list your proposed series of steps you'd recommend to a DBA
    > > wishing to remove an index, assuming this feature exists in core as
    > > you'd like it to?
    > >
    >
    > Well, the series of steps differs depending on the nature of the
    > system being managed. If you are running on a single node with normal
    > traffic and resources, you just set the GUC to include the index you
    > want to be invisible, wait for a few days (maybe no one runs monthly
    > reports on this system?), take a quick look at your monitoring/stats
    > to make sure things seem copacetic, and then you drop the index and
    > reset the GUC.
    
    Thanks for explaining.
    
    What are your thoughts on cached plans? In this scenario, do you
    assume that waiting a few days means that connections get reset and
    prepared statements will have been replanned? Or do you think cached
    plans don't matter in this scenario?
    
    David
    
    
    
    
  100. Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX

    Robert Treat <rob@xzilla.net> — 2025-06-08T18:53:20Z

    On Sat, Jun 7, 2025 at 9:17 PM David Rowley <dgrowleyml@gmail.com> wrote:
    > On Sun, 8 Jun 2025 at 01:35, Robert Treat <rob@xzilla.net> wrote:
    > > On Fri, Jun 6, 2025 at 8:04 PM David Rowley <dgrowleyml@gmail.com> wrote:
    > > > Can you list your proposed series of steps you'd recommend to a DBA
    > > > wishing to remove an index, assuming this feature exists in core as
    > > > you'd like it to?
    > > >
    > >
    > > Well, the series of steps differs depending on the nature of the
    > > system being managed. If you are running on a single node with normal
    > > traffic and resources, you just set the GUC to include the index you
    > > want to be invisible, wait for a few days (maybe no one runs monthly
    > > reports on this system?), take a quick look at your monitoring/stats
    > > to make sure things seem copacetic, and then you drop the index and
    > > reset the GUC.
    >
    > Thanks for explaining.
    >
    > What are your thoughts on cached plans? In this scenario, do you
    > assume that waiting a few days means that connections get reset and
    > prepared statements will have been replanned? Or do you think cached
    > plans don't matter in this scenario?
    >
    
    Heh; I did say that the GUC model wasn't perfect, so good on you for
    getting right to one of the more wonky parts. In practice, I actually
    don't think it matters as much as one might think; IME there is a sort
    of inverse relationship were the more sensitive you are to production
    changes and/or running at high scale, the more likely you are going to
    want to slow deploy / ramp up these changes, and doing things like
    adding the GUC at the session level will likely require a connection
    recycle anyway. Also keeping invisible indexes in place for days or
    weeks is likely to be a common scenario, and again we don't normally
    expect connections, or cached plans, to stay alive for weeks at a
    time. Of course you can't dismiss this; you'd definitely have to
    document that if they are worried about queries with cached plans the
    best solution would be to recycle any connections that might have
    existed before setting the guc in place. That may not sound ideal, but
    I think in practice it is no worse than the practical effects of
    thinking that ANALYZE will help keep your queries fast; sure it keeps
    your statistics up to date, but if you are running cached plans for
    indefinite periods of time, you wouldn't actually pick those up those
    statistics changes*, which means cached plans are already susceptible
    to degrading over time, and we are expecting people to recycle
    connections regularly even if we don't say it very loud.
    
    * As an aside, I once looked into implementing some kind of
    pg_invalidate_cached_plans() function that would send a signal to all
    backend to dump their plans; kind of like a global DISCARD ALL, but it
    always seemed scarier than just recycling connections, so I gave up on
    it pretty quick; maybe some would find that useful though?
    
    
    Robert Treat
    https://xzilla.net
    
    
    
    
  101. Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX

    David Rowley <dgrowleyml@gmail.com> — 2025-06-09T01:36:56Z

    On Mon, 9 Jun 2025 at 06:53, Robert Treat <rob@xzilla.net> wrote:
    >
    > On Sat, Jun 7, 2025 at 9:17 PM David Rowley <dgrowleyml@gmail.com> wrote:
    > > What are your thoughts on cached plans? In this scenario, do you
    > > assume that waiting a few days means that connections get reset and
    > > prepared statements will have been replanned? Or do you think cached
    > > plans don't matter in this scenario?
    > >
    >
    > Heh; I did say that the GUC model wasn't perfect, so good on you for
    > getting right to one of the more wonky parts. In practice, I actually
    > don't think it matters as much as one might think; IME there is a sort
    > of inverse relationship were the more sensitive you are to production
    > changes and/or running at high scale, the more likely you are going to
    > want to slow deploy / ramp up these changes, and doing things like
    > adding the GUC at the session level will likely require a connection
    > recycle anyway. Also keeping invisible indexes in place for days or
    > weeks is likely to be a common scenario, and again we don't normally
    > expect connections, or cached plans, to stay alive for weeks at a
    > time. Of course you can't dismiss this; you'd definitely have to
    > document that if they are worried about queries with cached plans the
    > best solution would be to recycle any connections that might have
    > existed before setting the guc in place. That may not sound ideal, but
    > I think in practice it is no worse than the practical effects of
    > thinking that ANALYZE will help keep your queries fast; sure it keeps
    > your statistics up to date, but if you are running cached plans for
    > indefinite periods of time, you wouldn't actually pick those up those
    > statistics changes*, which means cached plans are already susceptible
    > to degrading over time, and we are expecting people to recycle
    > connections regularly even if we don't say it very loud.
    
    I agree that it doesn't seem ideal. I feel like if we're adding a
    feature that we have to list a bunch of caveats in the documentation,
    then we're doing something wrong. BTW, the ALTER INDEX will correctly
    invalidate cached plans and does not suffer from the same issue.
    
    My thoughts on this are that extensions are a better place to keep
    solutions that work most of the time. Once you start committing quirky
    things to Postgres, you sentence yourself to answering the same
    question for possibly a few decades in the -bugs or -general mailing
    list. I do my best to avoid that and feel we have enough of that
    already, so I'm -1 on the GUC solution for this. I know there are a
    few other people that are for it, so feel free to listen to them
    instead.
    
    Personally, I'd rather see us getting query hints in core and having
    some method to specify a global hint to hint "not using index X". I'm
    not holding my breath for that one.
    
    David
    
    
    
    
  102. Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX

    Robert Treat <rob@xzilla.net> — 2025-06-09T16:39:54Z

    On Sun, Jun 8, 2025 at 9:37 PM David Rowley <dgrowleyml@gmail.com> wrote:
    > On Mon, 9 Jun 2025 at 06:53, Robert Treat <rob@xzilla.net> wrote:
    > > On Sat, Jun 7, 2025 at 9:17 PM David Rowley <dgrowleyml@gmail.com> wrote:
    > > > What are your thoughts on cached plans? In this scenario, do you
    > > > assume that waiting a few days means that connections get reset and
    > > > prepared statements will have been replanned? Or do you think cached
    > > > plans don't matter in this scenario?
    > > >
    > >
    > > Heh; I did say that the GUC model wasn't perfect, so good on you for
    > > getting right to one of the more wonky parts. In practice, I actually
    > > don't think it matters as much as one might think; IME there is a sort
    > > of inverse relationship were the more sensitive you are to production
    > > changes and/or running at high scale, the more likely you are going to
    > > want to slow deploy / ramp up these changes, and doing things like
    > > adding the GUC at the session level will likely require a connection
    > > recycle anyway. Also keeping invisible indexes in place for days or
    > > weeks is likely to be a common scenario, and again we don't normally
    > > expect connections, or cached plans, to stay alive for weeks at a
    > > time. Of course you can't dismiss this; you'd definitely have to
    > > document that if they are worried about queries with cached plans the
    > > best solution would be to recycle any connections that might have
    > > existed before setting the guc in place. That may not sound ideal, but
    > > I think in practice it is no worse than the practical effects of
    > > thinking that ANALYZE will help keep your queries fast; sure it keeps
    > > your statistics up to date, but if you are running cached plans for
    > > indefinite periods of time, you wouldn't actually pick those up those
    > > statistics changes*, which means cached plans are already susceptible
    > > to degrading over time, and we are expecting people to recycle
    > > connections regularly even if we don't say it very loud.
    >
    > I agree that it doesn't seem ideal. I feel like if we're adding a
    > feature that we have to list a bunch of caveats in the documentation,
    > then we're doing something wrong. BTW, the ALTER INDEX will correctly
    > invalidate cached plans and does not suffer from the same issue.
    >
    
    While the ALTER INDEX provides a simple way to do cache invalidation,
    for practical application you still have most of the same issues and
    need to jump through many of the same guc hoops with
    force_invisible_index, which is a large part of why this is such a red
    herring.
    
    > My thoughts on this are that extensions are a better place to keep
    > solutions that work most of the time. Once you start committing quirky
    > things to Postgres, you sentence yourself to answering the same
    > question for possibly a few decades in the -bugs or -general mailing
    > list. I do my best to avoid that and feel we have enough of that
    > already, so I'm -1 on the GUC solution for this. I know there are a
    > few other people that are for it, so feel free to listen to them
    > instead.
    >
    
    I hear you wrt explaining quirky things to users; you wouldn't believe
    the level of confusion I got when I started explaining
    "plan_cache_mode" to users when v12 rolled out. I'd guess the vast
    majority of users have still never heard of this guc and have no idea
    that Postgres behaves like this, which is another reason why I'd
    rather not optimize for a very small segment of the user base at the
    expense of a much larger set of users.
    
    And to be clear, this isn't a case of a GUC solution vs an ALTER
    solution. There is a reason that the proposed ALTER solution contains
    a GUC as well, and why Oracle had to make use of a session flag in
    their implementation. You are going to have a guc either way, which
    means you are going to have to explain a bunch of these different
    caveats in BOTH solutions. It's just that in one of the solutions, you
    are further entangling the usage with DDL changes (and the additional
    caveats that come with that).
    
    
    Robert Treat
    https://xzilla.net
    
    
    
    
  103. Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX

    David Rowley <dgrowleyml@gmail.com> — 2025-06-11T04:52:40Z

    On Tue, 10 Jun 2025 at 04:40, Robert Treat <rob@xzilla.net> wrote:
    > You are going to have a guc either way, which
    > means you are going to have to explain a bunch of these different
    > caveats in BOTH solutions. It's just that in one of the solutions, you
    > are further entangling the usage with DDL changes (and the additional
    > caveats that come with that).
    
    IMO, having this GUC to force the use of invisible indexes is quite
    strange. In my view, it detracts from the guarantees that you're meant
    to get from disabling indexes. What if some connection has
    use_invisible_index set to true? The DBA might assume all is well
    after having seen nobody complain and then drop the index. The user
    might then complain.
    
    David
    
    
    
    
  104. Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX

    Sami Imseih <samimseih@gmail.com> — 2025-06-11T13:00:59Z

    > IMO, having this GUC to force the use of invisible indexes is quite
    > strange. In my view, it detracts from the guarantees that you're meant
    > to get from disabling indexes. What if some connection has
    > use_invisible_index set to true? The DBA might assume all is well
    > after having seen nobody complain and then drop the index. The user
    > might then complain.
    >
    
    Sure, this may occur. I can also imagine cases where an index is made
    visible only for certain workloads, intentionally. But such efforts should
    be coordinated by application teams and DBAs. Someone would need to modify
    this GUC at the connection level, alter the database, or change the session
    via application code. An ad-hoc connection enabling this GUC is unlikely to
    be an issue.
    
    I don't see how we could provide the INVISIBLE index DDL without also
    providing this boolean GUC. If a user creates an index that is initially
    INVISIBLE, they need a GUC to try it out before deciding to make it
    visible.
    
    It was also pointed out in the thread above that this GUC can serve as a
    backstop for replicas if the DDL to make an index visible is delayed.
    
    --
    
    Sami
    
  105. Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX

    Shayon Mukherjee <shayonj@gmail.com> — 2025-06-21T13:37:38Z

    
    > On Jun 11, 2025, at 9:00 AM, Sami Imseih <samimseih@gmail.com> wrote:
    > 
    > 
    >> IMO, having this GUC to force the use of invisible indexes is quite
    >> strange. In my view, it detracts from the guarantees that you're meant
    >> to get from disabling indexes. What if some connection has
    >> use_invisible_index set to true? The DBA might assume all is well
    >> after having seen nobody complain and then drop the index. The user
    >> might then complain.
    > 
    > Sure, this may occur. I can also imagine cases where an index is made
    > visible only for certain workloads, intentionally. But such efforts should
    > be coordinated by application teams and DBAs. Someone would need to modify
    > this GUC at the connection level, alter the database, or change the session
    > via application code. An ad-hoc connection enabling this GUC is unlikely to
    > be an issue.
    > 
    > I don't see how we could provide the INVISIBLE index DDL without also
    > providing this boolean GUC. If a user creates an index that is initially
    > INVISIBLE, they need a GUC to try it out before deciding to make it
    > visible.
    > 
    > It was also pointed out in the thread above that this GUC can serve as a
    > backstop for replicas if the DDL to make an index visible is delayed.
    > 
    
    Hello,
    
    Thank you everyone for all the discussions and also to Robert Treat for feedback and the operational considerations.
    
    It seems like there are multiple ways to solve this problem, which is encouraging. From the discussion, there appears to be consensus on few things as well, including the DDL approach, which I personally am a proponent for as well.
    
    I believe this is a valuable feature for DBAs and engineers working with large databases. Esp since it provides the confidence to "turn off" an index to observe the impact through their observability tools and make an informed decision about whether to drop it. If they're wrong, they can quickly rollback by making the index visible again, rather than waiting for a full index rebuild that can take 30 minutes to hours.
    
    The primary use case I have in mind is for helping engineers (ones not so seasoned like DBAs) decide whether to drop *existing* indexes. For new indexes, I expect most users would create them in visible mode (the default). Or so has been my experience so far.
    
    The GUC component opens the door for additional workflows, such as creating an index as initially invisible (like Sami points out) and testing its performance before making it visible. I originally wasn't thinking it this way, but this demonstrates the flexibility of the feature and accommodates different development approaches.
    
    As Robert noted, both approaches have trade-offs around operational safety and granular control. However, I think the DDL approach provides the right balance of simplicity and system-wide consistency that most users need, while the GUC still enables experimentation for those who want it.
    
    I'm very much committed to iterating on this patch to address any remaining feedback and help make progress on this. Is there something we can do here in the essence of "start small, think big", perhaps?
    
    Thanks
    Shayon
    
    
  106. Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX

    Merlin Moncure <mmoncure@gmail.com> — 2025-06-21T16:59:34Z

    On Sat, Jun 21, 2025 at 8:38 AM Shayon Mukherjee <shayonj@gmail.com> wrote:
    
    >
    >
    > On Jun 11, 2025, at 9:00 AM, Sami Imseih <samimseih@gmail.com> wrote:
    >
    >
    > IMO, having this GUC to force the use of invisible indexes is quite
    >> strange. In my view, it detracts from the guarantees that you're meant
    >> to get from disabling indexes. What if some connection has
    >> use_invisible_index set to true? The DBA might assume all is well
    >> after having seen nobody complain and then drop the index. The user
    >> might then complain.
    >>
    >
    > Sure, this may occur. I can also imagine cases where an index is made
    > visible only for certain workloads, intentionally. But such efforts should
    > be coordinated by application teams and DBAs. Someone would need to modify
    > this GUC at the connection level, alter the database, or change the session
    > via application code. An ad-hoc connection enabling this GUC is unlikely to
    > be an issue.
    >
    > I don't see how we could provide the INVISIBLE index DDL without also
    > providing this boolean GUC. If a user creates an index that is initially
    > INVISIBLE, they need a GUC to try it out before deciding to make it
    > visible.
    >
    > It was also pointed out in the thread above that this GUC can serve as a
    > backstop for replicas if the DDL to make an index visible is delayed.
    >
    >
    > Hello,
    >
    > Thank you everyone for all the discussions and also to Robert Treat for
    > feedback and the operational considerations.
    >
    > It seems like there are multiple ways to solve this problem, which is
    > encouraging. From the discussion, there appears to be consensus on few
    > things as well, including the DDL approach, which I personally am a
    > proponent for as well.
    >
    > I believe this is a valuable feature for DBAs and engineers working with
    > large databases. Esp since it provides the confidence to "turn off" an
    > index to observe the impact through their observability tools and make an
    > informed decision about whether to drop it. If they're wrong, they can
    > quickly rollback by making the index visible again, rather than waiting for
    > a full index rebuild that can take 30 minutes to hours.
    >
    > The primary use case I have in mind is for helping engineers (ones not so
    > seasoned like DBAs) decide whether to drop *existing* indexes. For new
    > indexes, I expect most users would create them in visible mode (the
    > default). Or so has been my experience so far.
    >
    
    What I would be using this for is when the server is choosing the wrong
    index, often in multi column index scenarios. The server can be obtuse in
    those situations.  So I see this as a query optimization aid rather than a
    'should I drop this?' Given that there are several ways to do that
    already.  I can see scenarios  where I'd want the index backed constraint
    to never be used for some/all queries.
    
    ALTER driving this seems ok.  It seems more of a planner directive to me
    but having potential permanent configuration (vs mostly temporary needs)
    tips the scale IMO.
    
    ENABLE | DISABLE seems off.  I would take it further to, ENABLE | DISABLE
    OPTIMIZATION  for clarify and to leave room for syntax expansion.
    
    Nice stuff. Did not review patch
    
    merlin
    
  107. Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX

    Junwang Zhao <zhjwpku@gmail.com> — 2025-06-22T09:56:15Z

    Hi Shayon,
    
    On Sat, Jun 21, 2025 at 9:38 PM Shayon Mukherjee <shayonj@gmail.com> wrote:
    >
    >
    >
    > On Jun 11, 2025, at 9:00 AM, Sami Imseih <samimseih@gmail.com> wrote:
    >
    >
    >> IMO, having this GUC to force the use of invisible indexes is quite
    >> strange. In my view, it detracts from the guarantees that you're meant
    >> to get from disabling indexes. What if some connection has
    >> use_invisible_index set to true? The DBA might assume all is well
    >> after having seen nobody complain and then drop the index. The user
    >> might then complain.
    >
    >
    > Sure, this may occur. I can also imagine cases where an index is made
    > visible only for certain workloads, intentionally. But such efforts should
    > be coordinated by application teams and DBAs. Someone would need to modify
    > this GUC at the connection level, alter the database, or change the session
    > via application code. An ad-hoc connection enabling this GUC is unlikely to
    > be an issue.
    >
    > I don't see how we could provide the INVISIBLE index DDL without also
    > providing this boolean GUC. If a user creates an index that is initially
    > INVISIBLE, they need a GUC to try it out before deciding to make it
    > visible.
    >
    > It was also pointed out in the thread above that this GUC can serve as a
    > backstop for replicas if the DDL to make an index visible is delayed.
    >
    >
    > Hello,
    >
    > Thank you everyone for all the discussions and also to Robert Treat for feedback and the operational considerations.
    >
    > It seems like there are multiple ways to solve this problem, which is encouraging. From the discussion, there appears to be consensus on few things as well, including the DDL approach, which I personally am a proponent for as well.
    >
    > I believe this is a valuable feature for DBAs and engineers working with large databases. Esp since it provides the confidence to "turn off" an index to observe the impact through their observability tools and make an informed decision about whether to drop it. If they're wrong, they can quickly rollback by making the index visible again, rather than waiting for a full index rebuild that can take 30 minutes to hours.
    >
    > The primary use case I have in mind is for helping engineers (ones not so seasoned like DBAs) decide whether to drop *existing* indexes. For new indexes, I expect most users would create them in visible mode (the default). Or so has been my experience so far.
    >
    > The GUC component opens the door for additional workflows, such as creating an index as initially invisible (like Sami points out) and testing its performance before making it visible. I originally wasn't thinking it this way, but this demonstrates the flexibility of the feature and accommodates different development approaches.
    >
    > As Robert noted, both approaches have trade-offs around operational safety and granular control. However, I think the DDL approach provides the right balance of simplicity and system-wide consistency that most users need, while the GUC still enables experimentation for those who want it.
    >
    > I'm very much committed to iterating on this patch to address any remaining feedback and help make progress on this. Is there something we can do here in the essence of "start small, think big", perhaps?
    >
    > Thanks
    > Shayon
    >
    
    Based on your analysis, I think the patch could be split into two
    parts: one focusing on the DDL approach and the other on the
    additional GUC control.
    
    From reading the discussions, it seems that the GUC control
    depends on the DDL approach (eg. creating an index as initially
    invisible and making it visible later).
    
    Therefore, maybe the DDL approach can be committed first
    and extend the GUC control later as needed?
    
    I read the v18 patch, I think the following changes should not
    be included:
    
    diff --git a/src/interfaces/ecpg/test/regression.diffs
    b/src/interfaces/ecpg/test/regression.diffs
    new file mode 100644
    index 0000000000..e69de29bb2
    diff --git a/src/interfaces/ecpg/test/regression.out
    b/src/interfaces/ecpg/test/regression.out
    new file mode 100644
    index 0000000000..cb633f4d71
    --- /dev/null
    +++ b/src/interfaces/ecpg/test/regression.out
    @@ -0,0 +1,55 @@
    +# initializing database system by copying initdb template
    +# using temp instance on port 65312 with PID 30031
    +ok 1         - compat_informix/dec_test                  563 ms
    +ok 2         - compat_informix/charfuncs                 255 ms
    +ok 3         - compat_informix/rfmtdate                  355 ms
    
    
    -- 
    Regards
    Junwang Zhao
    
    
    
    
  108. Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX

    Robert Treat <rob@xzilla.net> — 2025-06-23T14:14:29Z

    On Sat, Jun 21, 2025 at 10:59 AM Merlin Moncure <mmoncure@gmail.com> wrote:
    > On Sat, Jun 21, 2025 at 8:38 AM Shayon Mukherjee <shayonj@gmail.com> wrote:
    >> The primary use case I have in mind is for helping engineers (ones not so seasoned like DBAs) decide whether to drop *existing* indexes. For new indexes, I expect most users would create them in visible mode (the default). Or so has been my experience so far.
    >
    
    +1
    
    >
    > What I would be using this for is when the server is choosing the wrong index, often in multi column index scenarios. The server can be obtuse in those situations.  So I see this as a query optimization aid rather than a 'should I drop this?' Given that there are several ways to do that already.  I can see scenarios  where I'd want the index backed constraint to never be used for some/all queries.
    >
    > ALTER driving this seems ok.  It seems more of a planner directive to me but having potential permanent configuration (vs mostly temporary needs) tips the scale IMO.
    >
    
    If your use case falls along the lines of modifying planner decisions,
    a DDL based interface is really the wrong interface for that; it
    forces system wide impact and provides no ability to work in a per
    query/connection/role/etc type manner, and is the most susceptible to
    having rollback issues.  These types of issues have always been
    resolved through GUCs, which again, fits the use case here as well. I
    guess I'll caveat that with the note that your use case is already
    addressable using pg_hint_plan, which operates using sql comments, but
    I think we're trying to not mention query hints in this thread :-)
    
    
    Robert Treat
    https://xzilla.net
    
    
    
    
  109. Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX

    Robert Treat <rob@xzilla.net> — 2025-06-23T14:14:36Z

    On Sat, Jun 21, 2025 at 7:37 AM Shayon Mukherjee <shayonj@gmail.com> wrote:
    > On Jun 11, 2025, at 9:00 AM, Sami Imseih <samimseih@gmail.com> wrote:
    >> IMO, having this GUC to force the use of invisible indexes is quite
    >> strange. In my view, it detracts from the guarantees that you're meant
    >> to get from disabling indexes. What if some connection has
    >> use_invisible_index set to true? The DBA might assume all is well
    >> after having seen nobody complain and then drop the index. The user
    >> might then complain.
    >
    >
    > Sure, this may occur. I can also imagine cases where an index is made
    > visible only for certain workloads, intentionally. But such efforts should
    > be coordinated by application teams and DBAs. Someone would need to modify
    > this GUC at the connection level, alter the database, or change the session
    > via application code. An ad-hoc connection enabling this GUC is unlikely to
    > be an issue.
    >
    > I don't see how we could provide the INVISIBLE index DDL without also
    > providing this boolean GUC. If a user creates an index that is initially
    > INVISIBLE, they need a GUC to try it out before deciding to make it
    > visible.
    >
    > It was also pointed out in the thread above that this GUC can serve as a
    > backstop for replicas if the DDL to make an index visible is delayed.
    >
    >
    > Hello,
    >
    > Thank you everyone for all the discussions and also to Robert Treat for feedback and the operational considerations.
    >
    > It seems like there are multiple ways to solve this problem, which is encouraging. From the discussion, there appears to be consensus on few things as well, including the DDL approach, which I personally am a proponent for as well.
    >
    > I believe this is a valuable feature for DBAs and engineers working with large databases. Esp since it provides the confidence to "turn off" an index to observe the impact through their observability tools and make an informed decision about whether to drop it. If they're wrong, they can quickly rollback by making the index visible again, rather than waiting for a full index rebuild that can take 30 minutes to hours.
    >
    > The primary use case I have in mind is for helping engineers (ones not so seasoned like DBAs) decide whether to drop *existing* indexes. For new indexes, I expect most users would create them in visible mode (the default). Or so has been my experience so far.
    >
    > The GUC component opens the door for additional workflows, such as creating an index as initially invisible (like Sami points out) and testing its performance before making it visible. I originally wasn't thinking it this way, but this demonstrates the flexibility of the feature and accommodates different development approaches.
    >
    > As Robert noted, both approaches have trade-offs around operational safety and granular control. However, I think the DDL approach provides the right balance of simplicity and system-wide consistency that most users need, while the GUC still enables experimentation for those who want it.
    >
    > I'm very much committed to iterating on this patch to address any remaining feedback and help make progress on this. Is there something we can do here in the essence of "start small, think big", perhaps?
    >
    
    Glad to hear you are still interested, slightly disheartened by the
    general lack of concern around operational safety in this thread. I
    actually think what you have done covers a lot of the ground for
    multiple implementations, so I'm optimistic we can get something for
    19.
    
    I was thinking about this some more over the weekend and it does seem
    like you can't get away from doing something with DDL; even though it
    is the wrong mental model... like when your AC is running but you
    don't think it is cool enough, so you turn it down farther, as if it
    would blow colder air... but that isn't how AC actually work... it
    seems you can't eliminate the desire for this mental model entirely.
    Which to be clear, I am not against, it's just a bad tool for the hard
    cases, but not in every case. Anyway, if I were picking this up, I
    would separate out the two ideas; as I laid out in my email to David,
    the GUC solution can stand on it's own without the DDL implementation,
    and I would do that first, and then add a simplified DDL
    implementation after the fact. Of course it could be done the other
    way around, but I think you're more likely to land on the correct GUC
    implementation if it isn't mixed up with DDL, and the best way to
    assure that is by not having the DDL for the initial patch. Just my
    .02, but happy to help spec it out further.
    
    
    Robert Treat
    https://xzilla.net
    
    
    
    
  110. Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX

    Shayon Mukherjee <shayonj@gmail.com> — 2025-07-15T12:19:08Z

    
    > On Jun 23, 2025, at 10:14 AM, Robert Treat <rob@xzilla.net> wrote:
    >> 
    
    
    Sorry for the late response, been busy at work :D.
    
    > 
    > Glad to hear you are still interested, slightly disheartened by the
    > general lack of concern around operational safety in this thread. I
    > actually think what you have done covers a lot of the ground for
    > multiple implementations, so I'm optimistic we can get something for
    > 19.
    > 
    
    Just for my own learning and mental model - what would be a good way to understand the change that wasn’t operationally safe? 
    
    > I was thinking about this some more over the weekend and it does seem
    > like you can't get away from doing something with DDL; even though it
    > is the wrong mental model... like when your AC is running but you
    > don't think it is cool enough, so you turn it down farther, as if it
    > would blow colder air... but that isn't how AC actually work... it
    > seems you can't eliminate the desire for this mental model entirely.
    > Which to be clear, I am not against, it's just a bad tool for the hard
    > cases, but not in every case. Anyway, if I were picking this up, I
    > would separate out the two ideas; as I laid out in my email to David,
    > the GUC solution can stand on it's own without the DDL implementation,
    > and I would do that first, and then add a simplified DDL
    > implementation after the fact. Of course it could be done the other
    > way around, but I think you're more likely to land on the correct GUC
    > implementation if it isn't mixed up with DDL, and the best way to
    > assure that is by not having the DDL for the initial patch. Just my
    > .02, but happy to help spec it out further.
    > 
    
    I am happy to split this into two, however I think starting with GUC first may not achieve a lot of cases that David and I were talking about earlier in the thread, perhaps? Where, if you want quick feedback without needing to make application / session / connection level changes (i.e GUC) then you can quickly do it via the ALTER statement. Happy to redo the patch and just keep ALTER for v1 accordingly, if it still makes sense.
    
    Would folks have any preference between the two approaches?
    
    Thank you
    Shayon
  111. Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX

    Robert Treat <rob@xzilla.net> — 2025-07-15T17:58:54Z

    On Tue, Jul 15, 2025 at 8:19 AM Shayon Mukherjee <shayonj@gmail.com> wrote:
    > On Jun 23, 2025, at 10:14 AM, Robert Treat <rob@xzilla.net> wrote:
    > Glad to hear you are still interested, slightly disheartened by the
    > general lack of concern around operational safety in this thread. I
    > actually think what you have done covers a lot of the ground for
    > multiple implementations, so I'm optimistic we can get something for
    > 19.
    >
    > Just for my own learning and mental model - what would be a good way to understand the change that wasn’t operationally safe?
    >
    
    Generally speaking, the two biggest factors for operational safety are
    the ability to slowly ramp up changes in a controlled fashion, and
    conversely the ability to quickly reverse them. On its surface, the
    ALTER feature looks like it passes both of these tests because (in
    simple cases) it appears better than drop/create index process alone;
    indeed, the ability to "turn off" an index before dropping it feels
    like a slower roll out than dropping it, and the ability to "turn it
    back on" seems like a much quicker reversal than having to recreate
    the index. Our problem is that this only gives the appearance of
    safety without having provided any significant improvement in system
    safety, especially in more complex and/or demanding setups. With
    regards to roll out specifically, the ALTER method is no safer than
    drop index because both use DDL which means they are both open to
    blocking or being blocked by conflicting queries, which increase
    operational risk within the system. Similarly, the nature of the DDL
    change also requires that all sessions be impacted everywhere at once;
    there is no way to slowly roll the change to some segment of the
    database or some specific workload within the system. So it fails the
    first test. With regards to the ability to quickly reverse the change,
    it does beat the need to rebuild an index, but that only helps in a
    very small subset of the typical use cases for this feature; ie where
    you are concerned that your server might get "swamped" by poorly
    performing queries while the index rebuilds. But that's a pretty low
    level version of the problem; on very busy systems and/or system with
    delicately balanced buffer caching, even a small pause measured in
    seconds could be enough to bring a system down, and again our use of
    DDL opens us up to delays from conflicting queries, untimely
    wraparound vacuums, concurrent WAL traffic in the case of wanting to
    do this across replica trees (which you can't not do). So we generally
    fail the second test for a large portion of the use cases involved.
    And maybe that would be ok if we didn't have a way to solve this
    problem that doesn't fail these tests, but we do, which is through
    using a GUC.
    
    > I was thinking about this some more over the weekend and it does seem
    > like you can't get away from doing something with DDL; even though it
    > is the wrong mental model... like when your AC is running but you
    > don't think it is cool enough, so you turn it down farther, as if it
    > would blow colder air... but that isn't how AC actually work... it
    > seems you can't eliminate the desire for this mental model entirely.
    > Which to be clear, I am not against, it's just a bad tool for the hard
    > cases, but not in every case. Anyway, if I were picking this up, I
    > would separate out the two ideas; as I laid out in my email to David,
    > the GUC solution can stand on it's own without the DDL implementation,
    > and I would do that first, and then add a simplified DDL
    > implementation after the fact. Of course it could be done the other
    > way around, but I think you're more likely to land on the correct GUC
    > implementation if it isn't mixed up with DDL, and the best way to
    > assure that is by not having the DDL for the initial patch. Just my
    > .02, but happy to help spec it out further.
    >
    >
    > I am happy to split this into two, however I think starting with GUC first may not achieve a lot of cases that David and I were talking about earlier in the thread, perhaps? Where, if you want quick feedback without needing to make application / session / connection level changes (i.e GUC) then you can quickly do it via the ALTER statement. Happy to redo the patch and just keep ALTER for v1 accordingly, if it still makes sense.
    >
    
    I think it is much more the other way around; the GUC handles far more
    of the potential use cases that you might want to use the ALTER for,
    and the ALTER clearly falls short of what the GUC can do. (Side note,
    remember you can modify the GUC at the database level. And if you
    really want to get ambitious, GUCs can be extended to work through
    ALTER TABLE).
    
    > Would folks have any preference between the two approaches?
    >
    
    Contrary to how it sounds, I'm not actually opposed to having both :-)
    But I am very concerned that an implementation which does ALTER first
    sets a sort of anchoring bias that would affect how the GUC feature
    gets implemented, which is how I suspect Oracle ended up with their
    crappy implementation. I don't think this happens in reverse; ie. the
    GUC first implementation handles most of the heavy lifting so the
    ALTER only needs to cover the suite spot of the use cases that it can
    actually help with.
    
    
    Robert Treat
    https://xzilla.net
    
    
    
    
  112. Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX

    David Rowley <dgrowleyml@gmail.com> — 2025-07-18T01:48:47Z

    On Wed, 16 Jul 2025 at 05:59, Robert Treat <rob@xzilla.net> wrote:
    > operational risk within the system. Similarly, the nature of the DDL
    > change also requires that all sessions be impacted everywhere at once;
    > there is no way to slowly roll the change to some segment of the
    > database or some specific workload within the system.
    
    IMO, sounds like your method for safety here is to slowly walk your
    bull into the china shop. Wouldn't it be much better to learn where or
    confirm the index isn't being used before you go turning it off for
    various queries? I'm stumped at why your method for removing the index
    amounts to closing your eyes and doing your best to narrow the blast
    radius of the trial and error method.
    
    > regards to roll out specifically, the ALTER method is no safer than
    > drop index because both use DDL which means they are both open to
    > blocking or being blocked by conflicting queries, which increase
    
    Aside from not having to recreate the index, I agree with this part.
    It's a genuine concern. If some query switches to a Seq Scan and the
    queries to that table start taking a week to execute, then it'll be a
    long wait before you can get an AccessExclusiveLock on the table
    again. I think our mental models for this differ, however. In my
    imagination, I've checked that the index is unused before I disable
    it. It seems like in your model, you're going to disable it and
    measure the yield of the resulting explosion.
    
    The latest patch seems to be using a ShareUpdateExclusiveLock, so it
    looks like those concurrent seq scans won't block making the index
    visible again.
    
    My concern with the GUC approach is that:
    
    1. It'll be yet another crappy way to hint what you want the planner
    to do. (The other way being enable_* GUCs)
    2. There's no plan cache invalidation when changing the GUC.
    3. Standby servers may get forgotten about
    4. It encourages trial and error methodology for removing indexes
    5. All the committers who showed any hints at liking this method have
    disappeared from the thread.
    
    My concern with #1 is that when we one day do get query hints, we'll
    be left with a bunch of redundant ways to influence planner behaviour.
    
    Maybe you could get the behaviour you want by some additions to
    pg_hint_plan. Looking at [1], if query_id could be NULL to apply to
    all queries and there was some way of doing "No IndexScan(*
    index_name)", would that get you what you want?
    
    David
    
    [1] https://github.com/ossc-db/pg_hint_plan/blob/master/docs/hint_table.md
    
    
    
    
  113. Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX

    Sami Imseih <samimseih@gmail.com> — 2025-07-18T02:25:17Z

    > My concern with #1 is that when we one day do get query hints, we'll
    > be left with a bunch of redundant ways to influence planner behaviour.
    
    
    The GUC is more than just a hint. It can serve as a hint, but it also offers
    operational benefits. For example, if I mark an index as invisible and that
    only affects a subset of my workload, I don’t need to make the index visible
    again. Instead, I can tune that specific workload to operate without it.
    
    Once I’m confident the workload performs well, I can safely drop the index.
    
    I’d argue we should not provide the ALTER option without the GUC, for
    more granular control.
    Regards,
    
    Sami
    
  114. Re: Proposal to Enable/Disable Index using ALTER INDEX

    David G. Johnston <david.g.johnston@gmail.com> — 2025-07-18T03:09:57Z

    On Thursday, July 17, 2025, Sami Imseih <samimseih@gmail.com> wrote:
    
    >
    > My concern with #1 is that when we one day do get query hints, we'll
    >> be left with a bunch of redundant ways to influence planner behaviour.
    >
    >
    I’d rather not stop accepting improvements of this sort until we get a
    proper planner hint into core.
    
    I’d argue we should not provide the ALTER option without the GUC, for
    > more granular control.
    >
    
    Agreed.  DBAs should do research.  Optimistic ones can then use alter index
    to efficiently deploy their informed decisions throughout their systems.
    They have GUC as a temporary fallback mechanism should they need it.
    Pessimistic ones can just micro-manage using the GUC.
    
    Unless someone is willing to try and get “The PostgreSQL team’s blessed
    guide to index management” into the documentation as well we should
    probably just accept this will be a bit tool belt approach and there will
    be tools that for one person’s approach are not useful.  That does not mean
    they need to be removed (so long as they were added intentionally).
    
    David J.
    
  115. Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX

    David Rowley <dgrowleyml@gmail.com> — 2025-07-18T05:12:18Z

    On Fri, 18 Jul 2025 at 14:25, Sami Imseih <samimseih@gmail.com> wrote:
    > I’d argue we should not provide the ALTER option without the GUC, for
    > more granular control.
    
    If you mean the use_invisible_index GUC, then for transparency here,
    I'm not in favour of that. I do understand that this might be useful
    when trying to get a lagged hot-standby which is desperately
    performing Seq Scans back up and running again while waiting on the
    replay of the ALTER TABLE VISIBLE, but I just don't feel comfortable
    being the committer/forever-owner of having a GUC that overwrites
    something that's explicitly written in the system catalogue tables
    that is disabled. It's just too magical for my liking. I don't think
    we have anything like that today.
    
    Other committers might feel differently, so if the general consensus
    is ALTER TABLE+GUC, then I'll leave it to them. I'm by no means saying
    this to try and influence the discussion here. If the ALTER TABLE
    alone is not seen as useful and I'm the only one who thinks it would
    be useful by itself, then I'll just back away from this and let
    someone else pick it up.
    
    David
    
    
    
    
  116. Re: Proposal to Enable/Disable Index using ALTER INDEX

    Sami Imseih <samimseih@gmail.com> — 2025-07-18T15:40:46Z

    > Unless someone is willing to try and get “The PostgreSQL team’s blessed guide to index management”
    >  into the documentation
    
    I really doubt we can agree on one set of index management guidelines.
    If anything, this thread has proven there are many ways to bake this
    cake :) and all approaches have merit.
    
    > we should probably just accept this will be a bit tool belt approach and
    > there will be tools that for one person’s approach are not useful.
    
    +1
    
    > but I just don't feel comfortable
    > being the committer/forever-owner of having a GUC that overwrites
    > something that's explicitly written in the system catalogue tables
    > that is disabled.
    
    That's fair
    
    > Other committers might feel differently, so if the general consensus
    > is ALTER TABLE+GUC, then I'll leave it to them. I'm by no means saying
    > this to try and influence the discussion here. If the ALTER TABLE
    > alone is not seen as useful
    
    If we only go with the ALTER, my concern is there is really no way an extension
    ( i.e. pg_hint_plan ) can even provide the behavior of this GUC. If the value is
    'invisible' in the catalog, the index is no longer available to extensions via
    RelOptInfo->indexlist, and it cannot be forced to be considered for planning by
    the extension. So, unless we provide the GUC in-core, it will not be
    possible for it
    to be achieved by extensions. Maybe someone can prove me wrong here.
    
    --
    Sami
    
    
    
    
  117. Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX

    Robert Treat <rob@xzilla.net> — 2025-07-20T23:54:38Z

    On Thu, Jul 17, 2025 at 9:49 PM David Rowley <dgrowleyml@gmail.com> wrote:
    >
    > On Wed, 16 Jul 2025 at 05:59, Robert Treat <rob@xzilla.net> wrote:
    > > operational risk within the system. Similarly, the nature of the DDL
    > > change also requires that all sessions be impacted everywhere at once;
    > > there is no way to slowly roll the change to some segment of the
    > > database or some specific workload within the system.
    >
    > IMO, sounds like your method for safety here is to slowly walk your
    > bull into the china shop. Wouldn't it be much better to learn where or
    > confirm the index isn't being used before you go turning it off for
    > various queries? I'm stumped at why your method for removing the index
    > amounts to closing your eyes and doing your best to narrow the blast
    > radius of the trial and error method.
    >
    > > regards to roll out specifically, the ALTER method is no safer than
    > > drop index because both use DDL which means they are both open to
    > > blocking or being blocked by conflicting queries, which increase
    >
    > Aside from not having to recreate the index, I agree with this part.
    > It's a genuine concern. If some query switches to a Seq Scan and the
    > queries to that table start taking a week to execute, then it'll be a
    > long wait before you can get an AccessExclusiveLock on the table
    > again. I think our mental models for this differ, however. In my
    > imagination, I've checked that the index is unused before I disable
    > it. It seems like in your model, you're going to disable it and
    > measure the yield of the resulting explosion.
    >
    
    The whole premise of this feature is that there is no way to have
    certainty that an index is truly unused. I can assure you that I've
    done just as much due diligence as you have (perhaps more) to
    determine that the index is unused, but if that were enough to ensure
    safety, we wouldn't need invisible indexes in the first place; we
    could just drop the index.
    
    Once we admit that neither of us have operational safety, the question
    becomes just how close can we get people to certainty, and how can we
    most limit the fallout from being wrong. In my world, I'd never have
    to worry about a plan flipping to sequential scan causing a query to
    hold locks for a week because the server will have certainly crashed
    within minutes from the overwhelming level of traffic said sequential
    scan brings, so if the solution requires heavy locks and global
    application (like alter would), it will still be extremely risky in
    heavy production workloads. In short, we're both walking a bull
    through the china shop, but it would seem mine is much more
    temperamental than yours.
    
    
    Robert Treat
    https://xzilla.net
    
    
    
    
  118. Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX

    Sami Imseih <samimseih@gmail.com> — 2025-07-21T17:16:38Z

    > it will still be extremely risky in
    > heavy production workloads. In short, we're both walking a bull
    > through the china shop, but it would seem mine is much more
    > temperamental than yours.
    
    Robert, Could you describe the GUC you would like to see?
    
    Also, I'd like to ask. what would be the argument against offering both options,
    ALTER and a GUC to override the catalog, as currently proposed in the patch?
    
    This conversation has been mainly GUC is better than ALTER, or vice versa.
    
    It is clear, at least to me, there are merits in both approaches, so
    what would be
    the argument against making both options available ( maybe with a GUC that
    could be more useful than a simple boolean )?
    
    --
    
    Sami
    
    
    
    
  119. Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX

    Robert Treat <rob@xzilla.net> — 2025-07-21T20:29:56Z

    On Mon, Jul 21, 2025 at 1:17 PM Sami Imseih <samimseih@gmail.com> wrote:
    >
    > > it will still be extremely risky in
    > > heavy production workloads. In short, we're both walking a bull
    > > through the china shop, but it would seem mine is much more
    > > temperamental than yours.
    >
    > Robert, Could you describe the GUC you would like to see?
    >
    > Also, I'd like to ask. what would be the argument against offering both options,
    > ALTER and a GUC to override the catalog, as currently proposed in the patch?
    >
    > This conversation has been mainly GUC is better than ALTER, or vice versa.
    >
    > It is clear, at least to me, there are merits in both approaches, so
    > what would be
    > the argument against making both options available ( maybe with a GUC that
    > could be more useful than a simple boolean )?
    >
    
    Just to reiterate, I am not against having both a GUC and ALTER
    option, if implemented correctly. Like David, I don't have good
    feelings about the ALTER / use_invisible_index GUC overwriting
    behavior that is explicitly written in the catalog, and I see no
    reason to settle for a technically awkward solution when I think it
    also delivers a poor user interface that will be hard to reason about
    and/or debug in production.
    
    So I think the "right" interface looks something like a GUC that would
    be something like "ignore_index_planning" which takes a csv list of
    index names that the planner would ignore. On its own, this provides
    as much flexibility as we can offer when attempting to change index
    visibility, since it would be set global/local/etc, and could be set
    on some, none, or some combo thereof within replica tree environments.
    You can make that convoluted, but it is operating like other GUCs.
    
    This also seems compatible with the implementation approach discussed
    by PeterE, Tom, and Haas earlier upthread (1)(2) with regard to
    providing a list of names and filtering them out. (There could be
    other ways of implementing it, but this certainly seems to cover a lot
    of the ground we'd want covered).
    
    I know one concern of this method is that this could introduce some
    parsing overhead if people choose to use large lists of indexes, but I
    think that's generally ok as long as it is documented. Our typical use
    case is expected to be one or maybe a few at most, indexes, but if
    people feel strongly they need to run with dozens and dozens of
    indexes listed, there will be a trade off, similar to other GUCs/tools
    (think track_activity_query_size or adding pg_stat_statements, or even
    wildly long search_paths).
    
    This also covers some of the more esoteric use cases, such as wanting
    to "turn off" indexes for mixed workload replica trees, and covers the
    often mentioned use case of allowing an index to be created
    "invisible" by default (just add the proposed index name to the list
    before creation).
    
    And I'll also mention that this seems like the method least likely to
    conflict with an ALTER INDEX implementation if we want to add one down
    the line (I think there is an argument for it), since I imagine that
    you could create such a thing with a boolean catalog flag that mimics
    the gucs behavior, so that the GUC or catalog aren't trying to
    override each other. Of course I'm tempted to say you could maybe
    implement this like an index storage parameter, but that might be a
    bridge too far... still if we make the GUC first, that would certainly
    be an interesting idea to explore.
    
    
    1) https://www.postgresql.org/message-id/15238d97-f667-48df-8319-ab73b37d4511%40eisentraut.org
    2) https://www.postgresql.org/message-id/3465209.1727202064%40sss.pgh.pa.us
    
    
    Robert Treat
    https://xzilla.net
    
    
    
    
  120. Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX

    Sami Imseih <samimseih@gmail.com> — 2025-07-21T21:23:52Z

    > > > it will still be extremely risky in
    > > > heavy production workloads. In short, we're both walking a bull
    > > > through the china shop, but it would seem mine is much more
    > > > temperamental than yours.
    > >
    > > Robert, Could you describe the GUC you would like to see?
    > >
    > > Also, I'd like to ask. what would be the argument against offering both options,
    > > ALTER and a GUC to override the catalog, as currently proposed in the patch?
    > >
    > > This conversation has been mainly GUC is better than ALTER, or vice versa.
    > >
    > > It is clear, at least to me, there are merits in both approaches, so
    > > what would be
    > > the argument against making both options available ( maybe with a GUC that
    > > could be more useful than a simple boolean )?
    > >
    >
    > Just to reiterate, I am not against having both a GUC and ALTER
    > option, if implemented correctly.
    
    Thanks. This got lost, at least to me, in the thread above.
    
    > Like David, I don't have good
    > feelings about the ALTER / use_invisible_index GUC overwriting
    > behavior that is explicitly written in the catalog,
    
    OK, although I don't necessarily think this is something to be frowned
    upon. I mean, if we end up with an ALTER/GUC combo, I can't see
    how we can avoid such behavior.
    
    > and I see no
    > reason to settle for a technically awkward solution when I think it
    > also delivers a poor user interface that will be hard to reason about
    > and/or debug in production.
    
    This is already an established pattern has been used by other
    RDBMS's. Having worked with such interface in the past, a combo of
    ALTER and GUC, I never thought it was awkward and it's quite simple to
    understand/maintain. But that is subjective.
    
    > So I think the "right" interface looks something like a GUC that would
    > be something like "ignore_index_planning" which takes a csv list of
    > index names that the planner would ignore.
    
    A few years back, I explored this idea, and I did not really like the parsing
    overhead for every execution. You will need to supply a list of fully-qualified
    ( dbname.schemaname.indexname) names or carefully manage the GUC
    per database. Also, if you drop an index, you now must go cleanup the list,
    and especially if at some point you recreate the index with the same name.
    There is also that you have to push this GUC to all your standbys manually.
    
    This never sounded good to me as a core feature, or do I think it's a really
    friendly interface, and I think you can get in more trouble trying to deal with
    such a GUC that requires such management.
    
    --
    Sami
    
    
    
    
  121. Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX

    Robert Treat <rob@xzilla.net> — 2025-07-21T22:17:13Z

    On Mon, Jul 21, 2025 at 5:24 PM Sami Imseih <samimseih@gmail.com> wrote:
    >
    > > > > it will still be extremely risky in
    > > > > heavy production workloads. In short, we're both walking a bull
    > > > > through the china shop, but it would seem mine is much more
    > > > > temperamental than yours.
    > > >
    > > > Robert, Could you describe the GUC you would like to see?
    > > >
    > > > Also, I'd like to ask. what would be the argument against offering both options,
    > > > ALTER and a GUC to override the catalog, as currently proposed in the patch?
    > > >
    > > > This conversation has been mainly GUC is better than ALTER, or vice versa.
    > > >
    > > > It is clear, at least to me, there are merits in both approaches, so
    > > > what would be
    > > > the argument against making both options available ( maybe with a GUC that
    > > > could be more useful than a simple boolean )?
    > > >
    > >
    > > Just to reiterate, I am not against having both a GUC and ALTER
    > > option, if implemented correctly.
    >
    > Thanks. This got lost, at least to me, in the thread above.
    >
    > > Like David, I don't have good
    > > feelings about the ALTER / use_invisible_index GUC overwriting
    > > behavior that is explicitly written in the catalog,
    >
    > OK, although I don't necessarily think this is something to be frowned
    > upon. I mean, if we end up with an ALTER/GUC combo, I can't see
    > how we can avoid such behavior.
    >
    
    I laid out two potential options in my previous email; if we implement
    this like a storage option, then you are setting the guc at the index
    level, which would resolve any opposing behavior. Even if you didn't
    do that, if the catalog only indicates that index should be ignored,
    rather than trying to control yes/no of index ignoring, then there is
    no conflict. If the flag says ignore, you ignore. If the GUC says
    ignore, you ignore. If you don't find either, you don't ignore
    (default).
    
    > > and I see no
    > > reason to settle for a technically awkward solution when I think it
    > > also delivers a poor user interface that will be hard to reason about
    > > and/or debug in production.
    >
    > This is already an established pattern has been used by other
    > RDBMS's. Having worked with such interface in the past, a combo of
    > ALTER and GUC, I never thought it was awkward and it's quite simple to
    > understand/maintain. But that is subjective.
    >
    
    It's amazing what people are willing to put up with if they are first
    conditioned to believe it is the right way :-)
    
    What stands out to me in the Oracle implementation is that they don't
    sell it as a way to safely verify that indexes are unused before
    dropping, but that it provides a way to safely create an index without
    it being used. Both use cases are valid, but the former certainly
    seems like the far more desired feature, and yet they seem to shy away
    from showing the extra hoop jumping to make that work, I think
    precisely because it is awkward to work with. I think we should try to
    make the most common use case easier.
    
    > > So I think the "right" interface looks something like a GUC that would
    > > be something like "ignore_index_planning" which takes a csv list of
    > > index names that the planner would ignore.
    >
    > A few years back, I explored this idea, and I did not really like the parsing
    > overhead for every execution. You will need to supply a list of fully-qualified
    > ( dbname.schemaname.indexname) names or carefully manage the GUC
    > per database.
    
    I think I'd agree that you may need to be careful, but that's true of
    most things. I'm less sure of the need to use fully qualified names;
    pg_hint_plan does not have that restriction, and arguably there are
    use cases against wanting to do that (imagine a multi-tenant scenario
    where you want a specific role to ignore an index regardless of which
    database it connects to, as all databases have the same schema).
    
    > Also, if you drop an index, you now must go cleanup the list,
    > and especially if at some point you recreate the index with the same name.
    > There is also that you have to push this GUC to all your standbys manually.
    >
    > This never sounded good to me as a core feature, or do I think it's a really
    > friendly interface, and I think you can get in more trouble trying to deal with
    > such a GUC that requires such management.
    >
    
    It isn't clear to me which option you are speaking about here. If you
    believe you need a GUC (which you have said up-thread), then there is
    going to be some GUC management for any such implementation. If you
    set a role with a boolean "ignore_disabled_indexes" because you are
    dropping an index, you'll certainly want to clean that up once the
    index is dropped. There might be more bookkeeping for the DBA with a
    csv list, but only because it allows the DBA more flexibility in how
    it is implemented. If you stick to managing one index at a time, the
    bookkeeping is basically the same.
    
    
    Robert Treat
    https://xzilla.net
    
    
    
    
  122. Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX

    Sami Imseih <samimseih@gmail.com> — 2025-07-22T18:15:16Z

    > > This is already an established pattern has been used by other
    > > RDBMS's. Having worked with such interface in the past, a combo of
    > > ALTER and GUC, I never thought it was awkward and it's quite simple to
    > > understand/maintain. But that is subjective.
    > >
    >
    > It's amazing what people are willing to put up with if they are first
    > conditioned to believe it is the right way :-)
    
    Well, it works and serves its purpose (or even multiple purposes).
    
    Also, whichever direction we go in will ultimately become the method
    our users adopt. That’s just how these things work.
    
    So, I respectfully disagree with your view :)
    
    > What stands out to me in the Oracle implementation is that they don't
    > sell it as a way to safely verify that indexes are unused before
    > dropping, but that it provides a way to safely create an index without
    > it being used.
    
    Ultimately, the ALTER command guarantees that the index is not being used,
    since it applies a global change.
    
    The GUC serves multiple purposes. For example,I can create an index as invisible
    and use it in a controlled way, which is helpful for experimenting
    with a new index.
    I can also make an index visible only to certain workloads, let's say
    the replicas only.
    Also, If part of my workload suffers because I made the index is
    invisible, I can selectively
    make the index visible again using this GUC whileI figure things out.
    In that case, it acts as a
    safety measure against the global change, without having to roll it
    back everywhere.
    I think it’s quite versatile in its application.
    
    > Both use cases are valid, but the former certainly
    > seems like the far more desired feature, and yet they seem to shy away
    > from showing the extra hoop jumping to make that work
    
    I'm not following your point about how it's awkward.
    
    > > > So I think the "right" interface looks something like a GUC that would
    > > > be something like "ignore_index_planning" which takes a csv list of
    > > > index names that the planner would ignore.
    > >
    > > A few years back, I explored this idea, and I did not really like the parsing
    > > overhead for every execution. You will need to supply a list of fully-qualified
    > > ( dbname.schemaname.indexname) names or carefully manage the GUC
    > > per database.
    >
    > I think I'd agree that you may need to be careful, but that's true of
    > most things. I'm less sure of the need to use fully qualified names;
    > pg_hint_plan does not have that restriction,
    
    pg_hint_plan works at the query level, and the hints are resolved based on
    aliases, if I recall correctly. This is quite different from a GUC, which can
    be applied at multiple levels, including the cluster level.
    
    > There might be more bookkeeping for the DBA with a
    > csv list, but only because it allows the DBA more flexibility in how
    > it is implemented. If you stick to managing one index at a time, the
    > bookkeeping is basically the same.
    
    Sure, that's a fair point, and I don't disagree with the flexibility that such
    a GUC provides. As I said, when I first started thinking about this problem,
    I found the flexibility of a list-based GUC to be desirable, but I couldn't
    rationalize the performance and maintenance trade-offs it incurs.
    
    I'm definitely open to having my mind changed again on this topic. But I
    don’t see this GUC as an opposing feature to the ALTER command, which I
    still believe we should have.
    
    In my view, the real question we are now debating is about how we
    should implement the GUC.
    
    --
    Sami
    
    
    
    
  123. Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX

    David Rowley <dgrowleyml@gmail.com> — 2025-07-22T22:50:13Z

    On Tue, 22 Jul 2025 at 05:16, Sami Imseih <samimseih@gmail.com> wrote:
    > Also, I'd like to ask. what would be the argument against offering both options,
    > ALTER and a GUC to override the catalog, as currently proposed in the patch?
    
    For me, the reason I don't like ALTER TABLE + the use_invisible_index
    / force_invisible_index (the v18 patch seems to be confused about the
    name of that GUC) is because it puts into question what "invisible"
    means. It's going to be a pretty useless feature for use cases where a
    DBA wants to ensure a certain index is *never* used, but does not want
    to drop it. A DBA might want to disable a certain index to investigate
    certain forms of index corruption and it might not be good if people
    can just overwrite that to bypass the DBA's choice.
    
    It might be a slightly more flexible feature if there were 3 possible
    states and one of those states could be clearly defined to mean that
    users can overwrite the disabledness of all indexes by setting a GUC.
    I'm still struggling to like that, however.
    
    Now wondering if it would be better to spend the effort looking at
    pg_hint_plan and seeing how hard it would be to get global hints added
    which are applied to all queries, and then add a way to disable use of
    a named index. (I don't have any experience with that extension other
    than looking at the documentation)
    
    David
    
    
    
    
  124. Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX

    Sami Imseih <samimseih@gmail.com> — 2025-07-22T23:04:50Z

    > Now wondering if it would be better to spend the effort looking at
    > pg_hint_plan and seeing how hard it would be to get global hints added
    > which are applied to all queries, and then add a way to disable use of
    > a named index. (I don't have any experience with that extension other
    > than looking at the documentation)
    
    Regardless of what comes out of this thread, pg_hint_plan supporting
    a NoIndexScan hint that takes in an index name, and not only a relname
    is needed. I plan on taking that up there.
    
    --
    
    Sami
    
    
    
    
  125. Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX

    Robert Treat <rob@xzilla.net> — 2025-07-23T23:34:08Z

    On Tue, Jul 22, 2025 at 6:50 PM David Rowley <dgrowleyml@gmail.com> wrote:
    > On Tue, 22 Jul 2025 at 05:16, Sami Imseih <samimseih@gmail.com> wrote:
    > > Also, I'd like to ask. what would be the argument against offering both options,
    > > ALTER and a GUC to override the catalog, as currently proposed in the patch?
    >
    > For me, the reason I don't like ALTER TABLE + the use_invisible_index
    > / force_invisible_index (the v18 patch seems to be confused about the
    > name of that GUC) is because it puts into question what "invisible"
    > means. It's going to be a pretty useless feature for use cases where a
    > DBA wants to ensure a certain index is *never* used, but does not want
    > to drop it. A DBA might want to disable a certain index to investigate
    > certain forms of index corruption and it might not be good if people
    > can just overwrite that to bypass the DBA's choice.
    >
    
    Thanks for elaborating on this, you said it better than me.
    
    So I'll note that in my proposal, the hypothetical catalog update
    ("alter index set guc" or whatever) is a one way door; if the dba (or
    whomever) sets that, then the index is ignored everywhere, since the
    session level guc can only also suggest the index be ignored from
    planning. That is enough to allow people to both slow roll out OR slow
    roll in new indexes, as needed, which I think covers enough ground
    without the complexity going too far (which your below example clearly
    shows is possible).
    
    > It might be a slightly more flexible feature if there were 3 possible
    > states and one of those states could be clearly defined to mean that
    > users can overwrite the disabledness of all indexes by setting a GUC.
    > I'm still struggling to like that, however.
    >
    > Now wondering if it would be better to spend the effort looking at
    > pg_hint_plan and seeing how hard it would be to get global hints added
    > which are applied to all queries, and then add a way to disable use of
    > a named index. (I don't have any experience with that extension other
    > than looking at the documentation)
    >
    
    I'd be interested in your evaluation of this, but the GUC I've
    outlined would accomplish most of the use cases here automagically.
    
    
    Robert Treat
    https://xzilla.net
    
    
    
    
  126. Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX

    Michael Paquier <michael@paquier.xyz> — 2025-07-24T01:38:29Z

    On Tue, Jul 22, 2025 at 06:04:50PM -0500, Sami Imseih wrote:
    > Regardless of what comes out of this thread, pg_hint_plan supporting
    > a NoIndexScan hint that takes in an index name, and not only a relname
    > is needed. I plan on taking that up there.
    
    Patches are welcome upstream.
    --
    Michael
    
  127. Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX

    Michael Paquier <michael@paquier.xyz> — 2025-07-24T01:43:12Z

    On Tue, Jul 22, 2025 at 01:15:16PM -0500, Sami Imseih wrote:
    > The GUC serves multiple purposes. For example,I can create an index as invisible
    > and use it in a controlled way, which is helpful for experimenting
    > with a new index.
    
    An in-core GUC to control the list of indexes that should be allowed
    or disallowed is I think asking for trouble, adding schema-related
    knowledge directly into the GUC machinery.  This does not scale well,
    even if you force all the entries to be specified down to the database
    and the schema.  And it makes harder to control what a "good" behavior
    should be at query-level.
    
    My 2c.
    --
    Michael
    
  128. Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX

    Shayon Mukherjee <shayonj@gmail.com> — 2025-08-02T18:46:23Z

    
    > On Jul 15, 2025, at 1:58 PM, Robert Treat <rob@xzilla.net> wrote:
    > 
    > On Tue, Jul 15, 2025 at 8:19 AM Shayon Mukherjee <shayonj@gmail.com> wrote:
    >> On Jun 23, 2025, at 10:14 AM, Robert Treat <rob@xzilla.net> wrote:
    >> Glad to hear you are still interested, slightly disheartened by the
    >> general lack of concern around operational safety in this thread. I
    >> actually think what you have done covers a lot of the ground for
    >> multiple implementations, so I'm optimistic we can get something for
    >> 19.
    >> 
    >> Just for my own learning and mental model - what would be a good way to understand the change that wasn’t operationally safe?
    >> 
    > 
    > Generally speaking, the two biggest factors for operational safety are
    > the ability to slowly ramp up changes in a controlled fashion, and
    > conversely the ability to quickly reverse them. On its surface, the
    > ALTER feature looks like it passes both of these tests because (in
    > simple cases) it appears better than drop/create index process alone;
    > indeed, the ability to "turn off" an index before dropping it feels
    > like a slower roll out than dropping it, and the ability to "turn it
    > back on" seems like a much quicker reversal than having to recreate
    > the index. Our problem is that this only gives the appearance of
    > safety without having provided any significant improvement in system
    > safety, especially in more complex and/or demanding setups. With
    > regards to roll out specifically, the ALTER method is no safer than
    > drop index because both use DDL which means they are both open to
    > blocking or being blocked by conflicting queries, which increase
    > operational risk within the system. Similarly, the nature of the DDL
    > change also requires that all sessions be impacted everywhere at once;
    > there is no way to slowly roll the change to some segment of the
    > database or some specific workload within the system. So it fails the
    > first test. With regards to the ability to quickly reverse the change,
    > it does beat the need to rebuild an index, but that only helps in a
    > very small subset of the typical use cases for this feature; ie where
    > you are concerned that your server might get "swamped" by poorly
    > performing queries while the index rebuilds. But that's a pretty low
    > level version of the problem; on very busy systems and/or system with
    > delicately balanced buffer caching, even a small pause measured in
    > seconds could be enough to bring a system down, and again our use of
    > DDL opens us up to delays from conflicting queries, untimely
    > wraparound vacuums, concurrent WAL traffic in the case of wanting to
    > do this across replica trees (which you can't not do). So we generally
    > fail the second test for a large portion of the use cases involved.
    > And maybe that would be ok if we didn't have a way to solve this
    > problem that doesn't fail these tests, but we do, which is through
    > using a GUC.
    > 
    >> I was thinking about this some more over the weekend and it does seem
    >> like you can't get away from doing something with DDL; even though it
    >> is the wrong mental model... like when your AC is running but you
    >> don't think it is cool enough, so you turn it down farther, as if it
    >> would blow colder air... but that isn't how AC actually work... it
    >> seems you can't eliminate the desire for this mental model entirely.
    >> Which to be clear, I am not against, it's just a bad tool for the hard
    >> cases, but not in every case. Anyway, if I were picking this up, I
    >> would separate out the two ideas; as I laid out in my email to David,
    >> the GUC solution can stand on it's own without the DDL implementation,
    >> and I would do that first, and then add a simplified DDL
    >> implementation after the fact. Of course it could be done the other
    >> way around, but I think you're more likely to land on the correct GUC
    >> implementation if it isn't mixed up with DDL, and the best way to
    >> assure that is by not having the DDL for the initial patch. Just my
    >> .02, but happy to help spec it out further.
    >> 
    >> 
    >> I am happy to split this into two, however I think starting with GUC first may not achieve a lot of cases that David and I were talking about earlier in the thread, perhaps? Where, if you want quick feedback without needing to make application / session / connection level changes (i.e GUC) then you can quickly do it via the ALTER statement. Happy to redo the patch and just keep ALTER for v1 accordingly, if it still makes sense.
    >> 
    > 
    > I think it is much more the other way around; the GUC handles far more
    > of the potential use cases that you might want to use the ALTER for,
    > and the ALTER clearly falls short of what the GUC can do. (Side note,
    > remember you can modify the GUC at the database level. And if you
    > really want to get ambitious, GUCs can be extended to work through
    > ALTER TABLE).
    > 
    >> Would folks have any preference between the two approaches?
    >> 
    > 
    > Contrary to how it sounds, I'm not actually opposed to having both :-)
    > But I am very concerned that an implementation which does ALTER first
    > sets a sort of anchoring bias that would affect how the GUC feature
    > gets implemented, which is how I suspect Oracle ended up with their
    > crappy implementation. I don't think this happens in reverse; ie. the
    > GUC first implementation handles most of the heavy lifting so the
    > ALTER only needs to cover the suite spot of the use cases that it can
    > actually help with.
    > 
    > 
    > 
    
    
    I know the thread has gotten quite long and I'm a little late to the party, but thank you for taking the time to walk me through the operational safety considerations you were referencing. I have a much better mental model of what you mean now.
    
    Just like David mentioned earlier in this thread, I was approaching this problem from the perspective of being 99% certain the index isn't used, and thinking that a DDL to enable/disable indexes without needing a rebuild would be useful. However, I understand your point that not everyone may approach using the DDL in the same way, or may not have done their due diligence, which could negatively impact busy systems. Though I think that's partly the responsibility of the DBA/developer as well - for example, we have `pg_stat_user_indexes`, and I'd typically use the stats in that table to understand index usage before trying to disable it. At the same time, I do agree with your point about making common workflows easy and safe for users. There is def something there.
    
    The whole thread has made it very clear that there are pros and cons to all the approaches mentioned so far, and I'll refrain from extending this tangent any further :D. I'll reply on the main thread with some thoughts additional thoughts. I just wanted to express my appreciation for you taking the time to walk me through your perspective in more depth.
    
    P.S. I think we're approaching the 11-month anniversary of this thread (started on September 9, 2024, by yours truly) and I am very humbled, and impressed by the community's rigor here :). I personally would love to see something land in core, and I'm confident we'll get there.
    
    Thanks
    Shayon
    
    
    
  129. Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX

    Shayon Mukherjee <shayonj@gmail.com> — 2025-08-02T19:09:19Z

    Hello,
    
    > On Jul 23, 2025, at 9:43 PM, Michael Paquier <michael@paquier.xyz> wrote:
    > 
    > On Tue, Jul 22, 2025 at 01:15:16PM -0500, Sami Imseih wrote:
    >> The GUC serves multiple purposes. For example,I can create an index as invisible
    >> and use it in a controlled way, which is helpful for experimenting
    >> with a new index.
    > 
    > An in-core GUC to control the list of indexes that should be allowed
    > or disallowed is I think asking for trouble, adding schema-related
    > knowledge directly into the GUC machinery.  This does not scale well,
    > even if you force all the entries to be specified down to the database
    > and the schema.  And it makes harder to control what a "good" behavior
    > should be at query-level.
    > 
    > My 2c.
    
    +1
    
    I wonder if there's a path to simplify things further here while still providing a way to gradually build confidence when disabling and then dropping an index. As a developer/DBA or person in a similar position, I think my journey for dropping an index in this new world would look something like this:
    
    1. Research phase: Use `pg_stat_user_indexes`, query analysis to understand index usage
    2. Experimentation phase: Use `pg_hint_plan` (or GUC?) for session-level testing and slower rollout from applications using feature flags
      - Up until a while ago, this step won't exist because once I had enough confidence from step 1, I'd go to step 3. Which is a huge improvement from jumping to Step 4 below. But the new discussions have made me think that this step is important.
    3. Validation phase: Use `ALTER INDEX INVISIBLE` for final system-wide confidence building  
    4. Cleanup phase: `DROP INDEX` when certain
    
    Per this plan, this would mean that pg_hint_plan would need to support index-level hints, and it’s not a massive / impossible task. But it also means that both systems aren't fighting/overriding each other or making it difficult for users to understand when exactly an index is being used or not. Ultimately, this would also mean that `ALTER INDEX INVISIBLE` is a one-way door, and there is only one way to control index visibility in core, which makes sense to me.
    
    I think any pitfalls and guarantees can be communicated well through documentation both in core and in `pg_hint_plan`. What’s not clear to me here is, how common / intuitive of a workflow will this be and if it fits easily in the “common use case” path?
    
    There are some aspects of the GUC approach that I'd miss, also because as a developer I've used DDLs and GUCs more than pg_hint_plan, but it's probably just a tooling exposure thing perhaps. 
    
    Curious what folks think.
    
    P.S. Still very happy to help with patches whenever that is.
    
    Thanks,
    Shayon
    
    
    
    
    
  130. Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX

    Michael Paquier <michael@paquier.xyz> — 2025-08-04T04:19:46Z

    On Sat, Aug 02, 2025 at 03:09:19PM -0400, Shayon Mukherjee wrote:
    > Per this plan, this would mean that pg_hint_plan would need to
    > support index-level hints, and it’s not a massive / impossible
    > task. But it also means that both systems aren't fighting/overriding
    > each other or making it difficult for users to understand when
    > exactly an index is being used or not. Ultimately, this would also
    > mean that `ALTER INDEX INVISIBLE` is a one-way door, and there is
    > only one way to control index visibility in core, which makes sense
    > to me. 
    
    Sami has proposed a patch for pg_hint_plan that goes in the direction
    of a DisableIndex hint, which is doable even if it requires some work
    inside the extension code like some refactoring:
    https://github.com/ossc-db/pg_hint_plan/issues/226
    
    So I cannot say much about this proposal for core, but for
    pg_hint_plan I'm looking at adding that for the v18 release of the
    module, which is planned for the end of August/beginning of September.
    --
    Michael
    
  131. Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX

    Julien Rouhaud <rjuju123@gmail.com> — 2025-08-04T04:34:14Z

    On Mon, Aug 04, 2025 at 01:19:46PM +0900, Michael Paquier wrote:
    > On Sat, Aug 02, 2025 at 03:09:19PM -0400, Shayon Mukherjee wrote:
    > > Per this plan, this would mean that pg_hint_plan would need to
    > > support index-level hints, and it’s not a massive / impossible
    > > task. But it also means that both systems aren't fighting/overriding
    > > each other or making it difficult for users to understand when
    > > exactly an index is being used or not. Ultimately, this would also
    > > mean that `ALTER INDEX INVISIBLE` is a one-way door, and there is
    > > only one way to control index visibility in core, which makes sense
    > > to me.
    >
    > Sami has proposed a patch for pg_hint_plan that goes in the direction
    > of a DisableIndex hint, which is doable even if it requires some work
    > inside the extension code like some refactoring:
    > https://github.com/ossc-db/pg_hint_plan/issues/226
    >
    > So I cannot say much about this proposal for core, but for
    > pg_hint_plan I'm looking at adding that for the v18 release of the
    > module, which is planned for the end of August/beginning of September.
    
    FWIW if anyone actually needs it, that feature had existed for decades in
    plantuner: http://www.sigaev.ru/git/gitweb.cgi?p=plantuner.git;a=summary
    
    
    
    
  132. Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX

    Michael Paquier <michael@paquier.xyz> — 2025-08-04T04:39:10Z

    On Mon, Aug 04, 2025 at 12:34:14PM +0800, Julien Rouhaud wrote:
    > FWIW if anyone actually needs it, that feature had existed for decades in
    > plantuner: http://www.sigaev.ru/git/gitweb.cgi?p=plantuner.git;a=summary
    
    Your link does not work here, this one does:
    https://github.com/postgrespro/plantuner
    
    Thanks for mentioning that, so it's a set of GUCs that rely on
    get_relation_info_hook_type for the index filtering.
    --
    Michael
    
  133. Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX

    Julien Rouhaud <rjuju123@gmail.com> — 2025-08-04T04:52:20Z

    On Mon, 4 Aug 2025, 12:39 Michael Paquier, <michael@paquier.xyz> wrote:
    
    > On Mon, Aug 04, 2025 at 12:34:14PM +0800, Julien Rouhaud wrote:
    > > FWIW if anyone actually needs it, that feature had existed for decades in
    > > plantuner: http://www.sigaev.ru/git/gitweb.cgi?p=plantuner.git;a=summary
    
    
    that's weird, I tried from 2 different ISP in Taiwan and both work just
    fine, I guess Japan is blocking .ru tld.
    
    >
    > Your link does not work here, this one does:
    > https://github.com/postgrespro/plantuner
    
    
    yes although this one doesn't seem maintained anymore, the original repo
    does have way more recent commits.
    
    >
    > Thanks for mentioning that, so it's a set of GUCs that rely on
    > get_relation_info_hook_type for the index filtering.
    >
    
    exactly.
    
    >
    
  134. Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX

    Michael Paquier <michael@paquier.xyz> — 2025-08-11T10:22:13Z

    On Mon, Aug 04, 2025 at 01:19:46PM +0900, Michael Paquier wrote:
    > Sami has proposed a patch for pg_hint_plan that goes in the direction
    > of a DisableIndex hint, which is doable even if it requires some work
    > inside the extension code like some refactoring:
    > https://github.com/ossc-db/pg_hint_plan/issues/226
    > 
    > So I cannot say much about this proposal for core, but for
    > pg_hint_plan I'm looking at adding that for the v18 release of the
    > module, which is planned for the end of August/beginning of September.
    
    FWIW, I have been able to merge the patch to add support for this
    DisableIndex hint in pg_hint_plan.  So at least this will give people
    something to toy with even without this in-core feature.  That will be
    included in next week's release, available in the version of the
    module compatible with v18.
    --
    Michael
    
  135. Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX

    Shayon Mukherjee <shayonj@gmail.com> — 2025-09-08T12:15:05Z

    Hello Hackers, 
    
    Looks like there may not be enough interest to port any functionality into core. So, just noting that I have withdrawn the patch from CommitFest[1]. Thank you for the good discussions and leanings. 
    
    [1] https://commitfest.postgresql.org/patch/5274/
    
    Shayon
    
    > On Aug 11, 2025, at 6:22 AM, Michael Paquier <michael@paquier.xyz> wrote:
    > 
    > On Mon, Aug 04, 2025 at 01:19:46PM +0900, Michael Paquier wrote:
    >> Sami has proposed a patch for pg_hint_plan that goes in the direction
    >> of a DisableIndex hint, which is doable even if it requires some work
    >> inside the extension code like some refactoring:
    >> https://github.com/ossc-db/pg_hint_plan/issues/226
    >> 
    >> So I cannot say much about this proposal for core, but for
    >> pg_hint_plan I'm looking at adding that for the v18 release of the
    >> module, which is planned for the end of August/beginning of September.
    > 
    > FWIW, I have been able to merge the patch to add support for this
    > DisableIndex hint in pg_hint_plan.  So at least this will give people
    > something to toy with even without this in-core feature.  That will be
    > included in next week's release, available in the version of the
    > module compatible with v18.
    > --
    > Michael
    
    
  136. Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX

    Robert Treat <rob@xzilla.net> — 2025-09-08T13:37:39Z

    On Mon, Sep 8, 2025 at 1:15 PM Shayon Mukherjee <shayonj@gmail.com> wrote:
    
    > Hello Hackers,
    >
    > Looks like there may not be enough interest to port any functionality into
    > core. So, just noting that I have withdrawn the patch from CommitFest[1].
    > Thank you for the good discussions and leanings.
    >
    > Thanks for your work on this. For those who may not be aware, Sami did
    implement a version of this in pg_hint_plan{1}, so that is helpful. I think
    it may be worth revisiting this in core, but perhaps once we seen this new
    implementation in action for a bit. (?)
    
    {1}
    https://github.com/ossc-db/pg_hint_plan/commit/d2cfd2f2c1fd18f55123b12c2250a384ccfaefaf
    
    
    Robert Treat
    https://xzilla.net
    
  137. Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX

    Michael Paquier <michael@paquier.xyz> — 2025-09-09T00:10:46Z

    On Mon, Sep 08, 2025 at 09:37:39AM -0400, Robert Treat wrote:
    > Thanks for your work on this. For those who may not be aware, Sami did
    > implement a version of this in pg_hint_plan{1}, so that is helpful. I think
    > it may be worth revisiting this in core, but perhaps once we seen this new
    > implementation in action for a bit. (?)
    > 
    > {1}
    > https://github.com/ossc-db/pg_hint_plan/commit/d2cfd2f2c1fd18f55123b12c2250a384ccfaefaf
    
    A side note on this one: it may make sense to implement regexp
    handling in this new hint, where all the indexes matching the regexp
    become ignored by the planner.  We have not done that because we
    wanted to see the impact of the initial feature first.  I don't think
    that this would increase the complexity footprint by a lot.
    --
    Michael