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

On Mon, Dec 30, 2024 at 11:08 AM Michail Nikolaev <
michail(dot)nikolaev(at)gmail(dot)com> wrote:

> Hello.
>
> A few comments on patch:
>

Thank you for the feedback.

>
> > + temporarily reducing the overhead of index maintenance
> > + during bulk data loading operations
>
>>
>
But tuples are still inserted, where the difference come from?
>
>
I think in retrospect this wasn't needed. I likely conflated a few
different use cases of this feature in the docs and I can see how it may
get confusing. I was originally thinking of the scenario where the use case
of this feature could allow users to identify indexes no longer needed and
once dropped, it would simplify the above tasks. I have simplified
the documentation now and removed that reference.

> > or verifying an index is not being used
> > + before dropping it
>
> Hm, it does not provide the guarantee - index may also be used as an
> arbiter for INSERT ON CONFLICT, for example. For that case, "update
> pg_index set indisvalid = false" should be used before the DROP, probably.
> Also index may also be used for constraint, part of partitioned table, etc.
>

Good catch, thank you! The feature is primarily geared towards the query
planner, so we are intentionally not changing the enforcements of say
constraints, including uniqueness, and the ones you mentioned above. I have
updated the documentation to call out the distinction. Let me know if you
think we can tweak the wording further.

>
> Also, I think it is better to move check to indisvalid as if
> (!index->indisvalid || !index->indisenabled).
>
>
Thank you! I was a bit split in terms of code maintenance and quality, so
originally I went with a dedicated block and a return/continue statement
for index ->indisenabled . I have now updated the patch to perform if
(!index->indisvalid || !index->indisenabled) and also updated the code
comment accordingly.

Rebased with the latest master as well.

Thank you
Shayon