From: Shayon Mukherjee <shayonj@gmail.com>
Message-Id: <ABD42A12-4DCF-4EE4-B903-4C657903CECE@gmail.com>
Content-Type: multipart/alternative;
	boundary="Apple-Mail=_6C4AC98D-5A1E-4FAA-AEEB-F90FC34C9DFC"
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.300.101.1.3\))
Subject: Re: Proposal to Enable/Disable Index using ALTER INDEX
Date: Sat, 12 Oct 2024 10:53:42 +0100
In-Reply-To: <CAApHDvr1a4crBB2CwT6RoscGqwbp6s9_OakBTQg375eiPHg-RA@mail.gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>,
 Peter Eisentraut <peter@eisentraut.org>,
 pgsql-hackers@lists.postgresql.org
To: David Rowley <dgrowleyml@gmail.com>
References: <CANqtF-oXKe0M=0QOih6H+sZRjE2BWAbkW_1+9nMEAMLxUJg5jA@mail.gmail.com>
 <15238d97-f667-48df-8319-ab73b37d4511@eisentraut.org>
 <CA+TgmoZw+o3UpKVf-=-o-x4fC2A54buK6ogLwk-TZ4C6h2W_ww@mail.gmail.com>
 <31040EAB-AB69-466C-A41B-B34EB6324150@gmail.com>
 <CAApHDvr1a4crBB2CwT6RoscGqwbp6s9_OakBTQg375eiPHg-RA@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/ABD42A12-4DCF-4EE4-B903-4C657903CECE%40gmail.com>
Precedence: bulk


--Apple-Mail=_6C4AC98D-5A1E-4FAA-AEEB-F90FC34C9DFC
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=utf-8

Hi David,=20

Answered below

> On Oct 9, 2024, at 9:19 AM, David Rowley <dgrowleyml@gmail.com> wrote:
>=20
> On Wed, 9 Oct 2024 at 20:07, Shayon Mukherjee <shayonj@gmail.com> =
wrote:
>> [thinking=E2=80=A6] Unless - we try to do support both a GUC and the =
ALTER INDEX ENABLE/DISABLE grammar + isdisabled attribute on pg_index?
>=20
> I just wanted to explain my point of view on this.  This is my opinion
> and is by no means authoritative.
>=20
> I was interested in this patch when you proposed it as an ALTER INDEX
> option. I know other committers seem interested, but I personally
> don't have any interest in the GUC option.  I think the reason I
> dislike it is that it's yet another not even half-baked take on
> planner hints (the other one being enable* GUCs). I often thought that
> if we ever did planner hints that it would be great to have multiple
> ways to specify the hints. Ordinarily, I'd expect some special comment
> type as the primary method to specify hints, but equally, it would be
> nice to be able to specify them in other ways. e.g. a GUC to have them
> apply to more than just 1 query. Useful for things such as "don't use
> index X".

Thank you so much this context, as someone new to psql-hackers, having =
this insight is super useful. Also getting a sense of how folks feel =
about controlling different behaviors like planner hints through GUC and =
SQL grammar. =20

For instance: I wasn=E2=80=99t 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.
 =20
>=20
> 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.
>=20
> I'll also stand by what I said earlier on this thread. If PeterG gets
> index skip scans done for PG18, then it's likely there's going to be
> lots of users considering if they still need a certain index or not
> after upgrading to PG18.


Likewise, I personally feel that the ability to disable indexes quickly =
and reverse the disabling (also quickly) is super useful, especially =
from an operational POV (point of view). So, I am very keen on getting =
this landed and happy to iterate on as many patches as it takes. :D

At this point, I am indifferent to each of the approaches (GUC or SQL =
grammar) based on the pros/cons I shared earlier in the thread & =
discussions in the thread. However, I would like us to make progress on =
getting _something_ out since the topic of disabling indexes has come up =
many times on pgsql-hackers in the past years and there is no easy way =
to toggle this behavior yet.=20

=E2=80=9Cyet 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].=20

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


[1] =
https://www.postgresql.org/message-id/CANqtF-oBaBtRfw9O7GAoHN3nNEZQYsW3oaG=
fD%2BwJfG8R29nZYw%40mail.gmail.com

Thanks
Shayon


--Apple-Mail=_6C4AC98D-5A1E-4FAA-AEEB-F90FC34C9DFC
Content-Transfer-Encoding: quoted-printable
Content-Type: text/html;
	charset=utf-8

<html><head><meta http-equiv=3D"content-type" content=3D"text/html; =
charset=3Dutf-8"></head><body style=3D"overflow-wrap: break-word; =
-webkit-nbsp-mode: space; line-break: after-white-space;">Hi =
David,&nbsp;<div><br></div><div>Answered below<br><div><br><blockquote =
type=3D"cite"><div>On Oct 9, 2024, at 9:19 AM, David Rowley =
&lt;dgrowleyml@gmail.com&gt; wrote:</div><br =
class=3D"Apple-interchange-newline"><div><div>On Wed, 9 Oct 2024 at =
20:07, Shayon Mukherjee &lt;shayonj@gmail.com&gt; wrote:<br><blockquote =
type=3D"cite">[thinking=E2=80=A6] Unless - we try to do support both a =
GUC and the ALTER INDEX ENABLE/DISABLE grammar + isdisabled attribute on =
pg_index?<br></blockquote><br>I just wanted to explain my point of view =
on this. &nbsp;This is my opinion<br>and is by no means =
authoritative.<br><br>I was interested in this patch when you proposed =
it as an ALTER INDEX<br>option. I know other committers seem interested, =
but I personally<br>don't have any interest in the GUC option. &nbsp;I =
think the reason I<br>dislike it is that it's yet another not even =
half-baked take on<br>planner hints (the other one being enable* GUCs). =
I often thought that<br>if we ever did planner hints that it would be =
great to have multiple<br>ways to specify the hints. Ordinarily, I'd =
expect some special comment<br>type as the primary method to specify =
hints, but equally, it would be<br>nice to be able to specify them in =
other ways. e.g. a GUC to have them<br>apply to more than just 1 query. =
Useful for things such as "don't use<br>index =
X".<br></div></div></blockquote><div><br></div>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. =
&nbsp;</div><div><br></div><div>For instance: I wasn=E2=80=99t quite =
able to figure out the how to properly distinguish + reason =
&nbsp;between the enable* GUCs and ALTER index for this case, and =
patches are per my limited understand of the historical context as =
well.</div><div>&nbsp;&nbsp;<br><blockquote =
type=3D"cite"><div><div><br>Now, I'm not suggesting you go off and code =
up planner hints. That's a<br>huge project. I'm just concerned that =
we've already got a fair bit of<br>cruft that will be left remaining if =
we ever get core planner hints<br>and a disabled_indexes GUC will just =
add to that. I don't feel like<br>the ALTER INDEX method would be =
leftover cruft from us gaining core<br>planner hints. Others might feel =
differently on that one. I feel the<br>ALTER INDEX option is less =
controversial.<br><br>I'll also stand by what I said earlier on this =
thread. If PeterG gets<br>index skip scans done for PG18, then it's =
likely there's going to be<br>lots of users considering if they still =
need a certain index or not<br>after upgrading to =
PG18.<br></div></div></blockquote><div><br></div><div><br></div><div>Likew=
ise, 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</div><div><br></div><div>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 &amp; 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.&nbsp;</div><div><br></div><div>=E2=80=9Cyet 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].&nbsp;</div><div><br></div><div>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.&nbsp;</div></div><div><br></div><div><br></div><div>[1]&nbsp;<a =
href=3D"https://www.postgresql.org/message-id/CANqtF-oBaBtRfw9O7GAoHN3nNEZ=
QYsW3oaGfD%2BwJfG8R29nZYw%40mail.gmail.com">https://www.postgresql.org/mes=
sage-id/CANqtF-oBaBtRfw9O7GAoHN3nNEZQYsW3oaGfD%2BwJfG8R29nZYw%40mail.gmail=
.com</a></div><div><br></div><div>Thanks</div><div>Shayon</div><br></div><=
/body></html>=

--Apple-Mail=_6C4AC98D-5A1E-4FAA-AEEB-F90FC34C9DFC--