Re: BUG #17220: ALTER INDEX ALTER COLUMN SET (..) with an optionless opclass makes index and table unusable

On Thu, Oct 14, 2021 at 11:07:21AM +0900, Michael Paquier wrote:
> This means that we've lost the ability to enforce n_distinct for
> expression indexes for two years. But, do we really care about this
> case? My answer to that would be "no" as long as we don't have a
> documented grammar rather, and we don't dump them either. But I think
> that we'd better do something with the code in analyze.c rather than
> letting it just dead, and my take is that we should remove the call to
> get_attribute_options() for this code path.
>
> Any opinions? @Robert: you were involved in 76a47c0, so I am adding
> you in CC.

Hearing nothing, and after pondering on this point, I think that
removing the get_attribute_options() is the right way to go for now
if there is a point in the future to get n_distinct done for all index
AMs.

I have reviewed the last patch posted upthread, and while testing
partitioned indexes I have noticed that we don't need to do a custom
check as part of ATExecSetOptions(), because we have already that in
ATSimplePermissions() with details on the relkind failing. This makes
the patch simpler, with a better error message generated. I have
added a case for partitioned indexes while on it.

Worth noting that I have spotted an extra weird call of
get_attribute_options() in extended statistics. This is unrelated to
this thread and I am not done analyzing it yet, but a quick check
shows that we call it with an InvalidOid for expression stats, which
is surprising.. I'll start a thread once/if I find anything
interesting on this one.

Attached is the patch I am finishing with, that should go down to
v13 (this is going to conflict on REL_13_STABLE, for sure).

Thoughts?
--
Michael