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

2025-04-28 Thread Shayon Mukherjee
On Thu, Apr 24, 2025 at 12:45 AM jian he 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);

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

2025-04-24 Thread jian he
hi, two more minor issues. src/bin/pg_dump/pg_dump.c if (fout->remoteVersion >= 18) need change to if (fout->remoteVersion >= 19) +-- 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

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

2025-04-23 Thread jian he
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); InvokeObjectPo

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

2025-04-10 Thread Shayon Mukherjee
On Mon, Apr 7, 2025 at 5:39 PM Sami Imseih 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 enco

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

2025-04-07 Thread Sami Imseih
> 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) v17-0001-Introduce-the-ability-to-set-index-visibility-us.patch Description

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

2025-04-07 Thread Shayon Mukherjee
On Mon, Apr 7, 2025 at 4:01 PM Sami Imseih 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-

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

2025-04-07 Thread Sami Imseih
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.

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

2025-04-05 Thread Shayon Mukherjee
On Wed, Apr 2, 2025 at 10:53 PM Sami Imseih 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 poi

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

2025-04-05 Thread Gurjeet Singh
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 VISIBLE. Before making an index >> > invisible, it's recommended >> > + to check >> > pg_stat_user_indexes.idx_scan >>

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

2025-04-02 Thread Sami Imseih
> 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 > addresse

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

2025-04-02 Thread Sami Imseih
> > + indexes. If performance degrades after making an index invisible, it can > > be easily > > + be made visible using VISIBLE. Before making an index > > invisible, it's recommended > > + to check > > pg_stat_user_indexes.idx_scan > > + to identify potentially unused indexes. > > I feel ALTER

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

2025-04-01 Thread Gurjeet Singh
On Sun, Sep 22, 2024 at 3:45 PM David Rowley 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 a

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

2025-04-01 Thread Shayon Mukherjee
On Tue, Apr 1, 2025 at 2:41 PM Sami Imseih 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.

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

2025-04-01 Thread Sami Imseih
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 b

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

2025-03-16 Thread Shayon Mukherjee
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 mome

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

2025-03-07 Thread Shayon Mukherjee
On Sun, Feb 23, 2025 at 3:41 PM Shayon Mukherjee 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-a

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

2025-02-23 Thread Shayon Mukherjee
On Sat, Feb 8, 2025 at 12:41 AM jian he 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 id

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

2025-02-07 Thread jian he
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 o

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

2025-02-04 Thread Shayon Mukherjee
On Sun, Feb 2, 2025 at 3:11 PM jian he 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 ... invisibl

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

2025-02-02 Thread jian he
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’: .

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

2025-02-02 Thread jian he
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"? VISIBLE Make the specified index visible. The index will be used fo

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

2025-01-31 Thread Shayon Mukherjee
On Fri, Jan 24, 2025 at 8:56 PM Benoit Lobréau 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 t

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

2025-01-31 Thread Shayon Mukherjee
On Fri, Jan 24, 2025 at 4:03 PM Benoit Lobréau wrote: > I did notice something in the command prototype: > > +ALTER INDEX [ IF EXISTS ] class="parameter">name VISIBLE > +ALTER INDEX [ IF EXISTS ] class="parameter">name INVISIBLE > > it would probably be better as: > > +ALTER INDEX [ IF EXISTS

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

2025-01-31 Thread Shayon Mukherjee
On Fri, Jan 31, 2025 at 10:18 AM Sami Imseih 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

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

2025-01-30 Thread Sami Imseih
> 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

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

2025-01-30 Thread Benoit Lobréau
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

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

2025-01-24 Thread Benoit Lobréau
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_pk

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

2025-01-24 Thread Benoit Lobréau
On Fri, Jan 24, 2025 at 11:32 AM Benoit Lobréau 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

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

2025-01-24 Thread Benoit Lobréau
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 itse

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

2025-01-12 Thread Shayon Mukherjee
On Sat, Jan 11, 2025 at 5:50 PM Sami Imseih 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.

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

2025-01-11 Thread Sami Imseih
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 th

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

2025-01-11 Thread Shayon Mukherjee
On Fri, Jan 3, 2025 at 4:09 PM Sami Imseih 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

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

2025-01-03 Thread Sami Imseih
+ This is the + default state for newly created indexes. This is not needed in the ALTER INDEX docs, IMO.ss + + 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 +

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

2025-01-01 Thread Michail Nikolaev
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.

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

2024-12-31 Thread Shayon Mukherjee
On Mon, Dec 30, 2024 at 3:48 PM Michail Nikolaev 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 (

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

2024-12-31 Thread Shayon Mukherjee
On Mon, Dec 30, 2024 at 2:56 PM Sami Imseih 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 yo

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

2024-12-30 Thread Sami Imseih
> 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

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

2024-12-30 Thread Michail Nikolaev
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.

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

2024-12-30 Thread Sami Imseih
> 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

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

2024-12-30 Thread Shayon Mukherjee
On Mon, Dec 30, 2024 at 11:08 AM Michail Nikolaev < michail.nikol...@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 ins

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

2024-12-30 Thread Michail Nikolaev
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

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

2024-12-28 Thread Shayon Mukherjee
On Fri, Dec 6, 2024 at 11:24 AM Shayon Mukherjee 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

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

2024-12-06 Thread Shayon Mukherjee
On Mon, Nov 25, 2024 at 6:19 PM David Rowley 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 i

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

2024-11-25 Thread David Rowley
On Tue, 26 Nov 2024 at 11:34, Shayon Mukherjee 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

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

2024-11-25 Thread Shayon Mukherjee
> On Nov 5, 2024, at 10:55 AM, Robert Haas wrote: > > On Thu, Oct 17, 2024 at 9:59 AM Shayon Mukherjee 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

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

2024-11-05 Thread Robert Haas
On Thu, Oct 17, 2024 at 9:59 AM Shayon Mukherjee 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

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

2024-11-05 Thread Rafia Sabih
Interesting idea. This patch needs a rebase. On Thu, 17 Oct 2024 at 15:59, Shayon Mukherjee wrote: > > > On Oct 16, 2024, at 6:01 PM, Shayon Mukherjee wrote: > > I'll take some time to think this through and familiarize myself with the > new systable_inplace_update_begin. In the meantime, aside

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

2024-10-17 Thread Shayon Mukherjee
On Oct 16, 2024, at 6:01 PM, Shayon Mukherjee 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 folk

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

2024-10-16 Thread Shayon Mukherjee
> On Oct 16, 2024, at 2:15 PM, Shayon Mukherjee wrote: > > >> On Oct 16, 2024, at 12:19 PM, Shayon Mukherjee wrote: >> >> - ALTER INDEX ... ENABLE/DISABLE performs an in-place update of the pg_index >> catalog to protect against indcheckxmin [2] (older unrelated thread). > > Performing th

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

2024-10-16 Thread Shayon Mukherjee
On Oct 16, 2024, at 12:19 PM, Shayon Mukherjee 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_i

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

2024-10-16 Thread Shayon Mukherjee
On Oct 15, 2024, at 7:25 PM, David Rowley wrote:On Wed, 16 Oct 2024 at 03:40, Robert Haas wrote:On Sat, Oct 12, 2024 at 5:56 AM Shayon Mukherjee 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 towa

Re: Proposal to Enable/Disable Index using ALTER INDEX

2024-10-16 Thread Robert Haas
On Wed, Oct 16, 2024 at 8:33 AM Peter Eisentraut 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 ha

Re: Proposal to Enable/Disable Index using ALTER INDEX

2024-10-16 Thread Peter Eisentraut
On 24.09.24 20:21, Tom Lane wrote: Peter Eisentraut 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 effe

Re: Proposal to Enable/Disable Index using ALTER INDEX

2024-10-15 Thread David Rowley
On Wed, 16 Oct 2024 at 03:40, Robert Haas wrote: > > On Sat, Oct 12, 2024 at 5:56 AM Shayon Mukherjee 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 ecos

Re: Proposal to Enable/Disable Index using ALTER INDEX

2024-10-15 Thread David Rowley
On Sat, 12 Oct 2024 at 22:41, Vinícius Abrahão 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 tho

Re: Proposal to Enable/Disable Index using ALTER INDEX

2024-10-15 Thread Robert Haas
On Sat, Oct 12, 2024 at 5:56 AM Shayon Mukherjee 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 ac

Re: Proposal to Enable/Disable Index using ALTER INDEX

2024-10-12 Thread Shayon Mukherjee
> On Oct 9, 2024, at 1:41 PM, Robert Haas wrote: > > On Wed, Oct 9, 2024 at 4:19 AM David Rowley > wrote: >> On Wed, 9 Oct 2024 at 20:07, Shayon Mukherjee wrote: >>> [thinking…] Unless - we try to do support both a GUC and the ALTER INDEX >>> ENABLE/DISABLE gramma

Re: Proposal to Enable/Disable Index using ALTER INDEX

2024-10-12 Thread Shayon Mukherjee
Hi David, Answered below > On Oct 9, 2024, at 9:19 AM, David Rowley wrote: > > On Wed, 9 Oct 2024 at 20:07, Shayon Mukherjee 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

Re: Proposal to Enable/Disable Index using ALTER INDEX

2024-10-12 Thread Vinícius Abrahão
On Wed, Oct 9, 2024 at 1:41 PM Robert Haas wrote: > On Wed, Oct 9, 2024 at 4:19 AM David Rowley wrote: > > On Wed, 9 Oct 2024 at 20:07, Shayon Mukherjee wrote: > > > [thinking…] Unless - we try to do support both a GUC and the ALTER > INDEX ENABLE/DISABLE grammar + isdisabled attribute on pg_in

Re: Proposal to Enable/Disable Index using ALTER INDEX

2024-10-09 Thread Robert Haas
On Wed, Oct 9, 2024 at 4:19 AM David Rowley wrote: > On Wed, 9 Oct 2024 at 20:07, Shayon Mukherjee 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 t

Re: Proposal to Enable/Disable Index using ALTER INDEX

2024-10-09 Thread David Rowley
On Wed, 9 Oct 2024 at 20:07, Shayon Mukherjee 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

Re: Proposal to Enable/Disable Index using ALTER INDEX

2024-10-09 Thread Shayon Mukherjee
> On Oct 7, 2024, at 4:52 PM, Robert Haas wrote: > > On Mon, Sep 23, 2024 at 11:14 AM Peter Eisentraut > 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 w

Re: Proposal to Enable/Disable Index using ALTER INDEX

2024-10-07 Thread Robert Haas
On Mon, Sep 23, 2024 at 11:14 AM Peter Eisentraut 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. > >

Re: Proposal to Enable/Disable Index using ALTER INDEX

2024-10-01 Thread Shayon Mukherjee
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 wrote: > > Hello, > > I am back with a PATCH :). Thanks to everyone in the threads for all the > helpful discussions. > > This

Re: Proposal to Enable/Disable Index using ALTER INDEX

2024-09-26 Thread Shayon Mukherjee
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

Re: Proposal to Enable/Disable Index using ALTER INDEX

2024-09-24 Thread Shayon Mukherjee
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 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

Re: Proposal to Enable/Disable Index using ALTER INDEX

2024-09-24 Thread Tom Lane
Peter Eisentraut 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 >> en

Re: Proposal to Enable/Disable Index using ALTER INDEX

2024-09-24 Thread Peter Eisentraut
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

Re: Proposal to Enable/Disable Index using ALTER INDEX

2024-09-24 Thread Peter Eisentraut
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 l

Re: Proposal to Enable/Disable Index using ALTER INDEX

2024-09-24 Thread Shayon Mukherjee
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 case

Re: Proposal to Enable/Disable Index using ALTER INDEX

2024-09-23 Thread Maciek Sakrejda
If one of the use cases is soft-dropping indexes, would a GUC approach still support that? ALTER TABLE?

Re: Proposal to Enable/Disable Index using ALTER INDEX

2024-09-23 Thread Shayon Mukherjee
On Mon, Sep 23, 2024 at 8:31 PM David Rowley wrote: > On Tue, 24 Sept 2024 at 03:14, Peter Eisentraut > 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

Re: Proposal to Enable/Disable Index using ALTER INDEX

2024-09-23 Thread David Rowley
On Tue, 24 Sept 2024 at 03:14, Peter Eisentraut 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

Re: Proposal to Enable/Disable Index using ALTER INDEX

2024-09-23 Thread Shayon Mukherjee
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 `p

Re: Proposal to Enable/Disable Index using ALTER INDEX

2024-09-23 Thread Shayon Mukherjee
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 yo

Re: Proposal to Enable/Disable Index using ALTER INDEX

2024-09-23 Thread Peter Eisentraut
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

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

2024-09-23 Thread Shayon Mukherjee
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

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

2024-09-22 Thread David Rowley
On Mon, 23 Sept 2024 at 05:43, Shayon Mukherjee 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

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

2024-09-22 Thread Shayon Mukherjee
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 wrote: > Hello, > > Thank you for all the feedback and insights. Work wa

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

2024-09-22 Thread Shayon Mukherjee
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]

Re: Proposal to Enable/Disable Index using ALTER INDEX

2024-09-10 Thread David Rowley
On Wed, 11 Sept 2024 at 03:12, Nathan Bossart 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

Re: Proposal to Enable/Disable Index using ALTER INDEX

2024-09-10 Thread Nathan Bossart
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 featur

Re: Proposal to Enable/Disable Index using ALTER INDEX

2024-09-10 Thread 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 wrote: > On Tue, 10 Sept 2024 at 09:39, Shayon Mukherjee wrot

Re: Proposal to Enable/Disable Index using ALTER INDEX

2024-09-10 Thread Shayon Mukherjee
+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 wrote: > On Tue, 10 Sept 2024 at 22:46, Michael Banck wrote: > > How about the indisl

Re: Proposal to Enable/Disable Index using ALTER INDEX

2024-09-10 Thread David Rowley
On Tue, 10 Sept 2024 at 22:46, Michael Banck 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 o

Re: Proposal to Enable/Disable Index using ALTER INDEX

2024-09-10 Thread Michael Banck
Hi, On Tue, Sep 10, 2024 at 10:16:34AM +1200, David Rowley wrote: > On Tue, 10 Sept 2024 at 09:39, Shayon Mukherjee wrote: > > Adding and removing indexes is a common operation in PostgreSQL. On > > larger databases, however, these operations can be > > resource-intensive. When evaluating the per

Re: Proposal to Enable/Disable Index using ALTER INDEX

2024-09-10 Thread wenhui qiu
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 deletin

Re: Proposal to Enable/Disable Index using ALTER INDEX

2024-09-09 Thread David Rowley
On Tue, 10 Sept 2024 at 09:39, Shayon Mukherjee 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 ide

Proposal to Enable/Disable Index using ALTER INDEX

2024-09-09 Thread Shayon Mukherjee
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,