Subject: Re: Proposal to Enable/Disable Index using ALTER INDEX

From: Shayon Mukherjee <[email-masked]>
Date: Wed, 9 Oct 2024 08:06:50 +0100

> On Oct 7, 2024, at 4:52 PM, Robert Haas <[email-masked]> wrote:
> 
> On Mon, Sep 23, 2024 at 11:14 AM Peter Eisentraut <[email-masked]> 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/[email-masked]

Thanks
Shayon

---

From: David Rowley <[email-masked]>
Date: Wed, 9 Oct 2024 21:19:34 +1300

> [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

---

From: Robert Haas <[email-masked]>
Date: Wed, 9 Oct 2024 08:41:10 -0400

> apply to more than just 1 query. Useful for things such as "don't use

+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

---

From: Shayon Mukherjee <[email-masked]>
Date: Sat, 12 Oct 2024 10:53:42 +0100

Hi David, 

Answered below

> On Oct 9, 2024, at 9:19 AM, David Rowley <[email-masked]> wrote:
> 

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.
  
> lots of users considering if they still need a certain index or not

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

---

From: Shayon Mukherjee <[email-masked]>
Date: Sat, 12 Oct 2024 10:56:24 +0100

> On Oct 9, 2024, at 1:41 PM, Robert Haas <[email-masked]> wrote:
> 
> On Wed, Oct 9, 2024 at 4:19 AM David Rowley <[email-masked] <mailto:[email-masked]>> 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

Thanks
Shayon

---

From: Robert Haas <[email-masked]>
Date: Tue, 15 Oct 2024 10:40:38 -0400

> [1] https://www.postgresql.org/message-id/ABD42A12-4DCF-4EE4-B903-4C657903CECE%40gmail.com

Yes, I think we can do both things.

---

From: David Rowley <[email-masked]>
Date: Wed, 16 Oct 2024 12:25:38 +1300

> 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

---

From: Shayon Mukherjee <[email-masked]>
Date: Wed, 16 Oct 2024 12:19:47 -0400

> On Oct 15, 2024, at 7:25 PM, David Rowley <[email-masked]> wrote:
> 

+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/[email-masked] 
[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

￼

---

From: Shayon Mukherjee <[email-masked]>
Date: Wed, 16 Oct 2024 14:15:51 -0400

> On Oct 16, 2024, at 12:19 PM, Shayon Mukherjee <[email-masked]> wrote:
> 

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/[email-masked]
[2] https://www.postgresql.org/message-id/EF2313B8-A017-4869-9B7F-A24EDD8795DE%40gmail.com
[3]  https://github.com/shayonj/postgres/pull/1

Thanks
Shayon

￼

---

From: Shayon Mukherjee <[email-masked]>
Date: Wed, 16 Oct 2024 18:01:13 -0400

> On Oct 16, 2024, at 2:15 PM, Shayon Mukherjee <[email-masked]> wrote:
> 
> 

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

---

From: Shayon Mukherjee <[email-masked]>
Date: Thu, 17 Oct 2024 09:59:34 -0400

On Oct 16, 2024, at 6:01 PM, Shayon Mukherjee <[email-masked]> 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/[email-masked]
[2]
https://www.postgresql.org/message-id/EF2313B8-A017-4869-9B7F-A24EDD8795DE%40gmail.com

Thank you for bearing with me on this :D
Shayon

---

From: Rafia Sabih <[email-masked]>
Date: Tue, 5 Nov 2024 12:32:34 +0100

Interesting idea.
This patch needs a rebase.

On Thu, 17 Oct 2024 at 15:59, Shayon Mukherjee <[email-masked]> wrote:

> - I understand the xmin would get incremented when
> using CatalogTupleUpdate to update indisenabled.

-- 
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

---

From: Robert Haas <[email-masked]>
Date: Tue, 5 Nov 2024 10:55:12 -0500

On Thu, Oct 17, 2024 at 9:59 AM Shayon Mukherjee <[email-masked]> wrote:
> My take away from whether or not an in-place update is needed on pg_index [1]
>
> - I understand the xmin would get incremented when using CatalogTupleUpdate to update indisenabled.
> - 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.
>

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.

---

From: Shayon Mukherjee <[email-masked]>
Date: Mon, 25 Nov 2024 17:34:38 -0500

> On Nov 5, 2024, at 10:55 AM, Robert Haas <[email-masked]> wrote:
> 

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

---

From: David Rowley <[email-masked]>
Date: Tue, 26 Nov 2024 12:18:48 +1300

> 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 lookin…

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

---

From: Shayon Mukherjee <[email-masked]>
Date: Fri, 6 Dec 2024 11:24:41 -0500

> timing dependent rather than dependent on what another session has

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

---

From: Shayon Mukherjee <[email-masked]>
Date: Sat, 28 Dec 2024 11:09:42 -0500

> 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

---

From: Michail Nikolaev <[email-masked]>
Date: Mon, 30 Dec 2024 17:08:18 +0100

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.

---

From: Shayon Mukherjee <[email-masked]>
Date: Mon, 30 Dec 2024 12:33:02 -0500

> A few comments on patch:

Thank you for the feedback.

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.

> 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.

> (!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

---

From: Sami Imseih <[email-masked]>
Date: Mon, 30 Dec 2024 13:56:01 -0600

> 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

---

From: Michail Nikolaev <[email-masked]>
Date: Mon, 30 Dec 2024 21:48:39 +0100

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.

---

From: Shayon Mukherjee <[email-masked]>
Date: Tue, 31 Dec 2024 11:52:16 -0500

> and have some comments as I continue to look.
Thank you so much for such detailed and useful feedback.

> ....

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.

> 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.

> ...

> 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