Horton

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

Michael Paquier <michael@paquier.xyz>

View thread
From: Michael Paquier <michael@paquier.xyz>
To: "Bossart, Nathan" <bossartn@amazon.com>
Cc: Dilip Kumar <dilipbalaut@gmail.com>, Vik Fearing <vik@postgresfriends.org>, "postgresql@zr40.nl" <postgresql@zr40.nl>, Alexander Korotkov <aekorotkov@gmail.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>, Robert Haas <robertmhaas@gmail.com>
Date: 2021-10-18T07:46:40Z
Lists: pgsql-bugs, pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Block ALTER INDEX/TABLE index_name ALTER COLUMN colname SET (options)

Attachments

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