Thread: Proposal to Enable/Disable Index using ALTER INDEX
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
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
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 beresource-intensive. When evaluating the performance impact of one or more indexes, dropping them might not be ideal sinceas 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 somethinglike: > > 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 allowsusers 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’sre-enabled later. The alternative would be dropping and rebuilding the index upon re-enabling, which I believe wouldintroduce 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
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:
->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
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
somethinglike:
> >
> > 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
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
+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
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
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
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
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
Hi David, Thank you so much for the review and pointers. I totally missed expression indexes. I am going to do another proper passalong 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
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.)
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 beingper-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 foreach index being considered with its OID via get_relname_relid through a helper function in the various places we needto 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 beresource-intensive. When evaluating the performance impact of one or more indexes, dropping them might not be ideal sinceas 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 somethinglike: >> 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. Thisallows 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 havean 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 globalstate, 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 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?
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,
ShayonOn 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.)
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
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();
}
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
If one of the use cases is soft-dropping indexes, would a GUC approach still support that? ALTER TABLE?