From: Michael Paquier <michael@paquier.xyz>
To: Bossart, Nathan <bossartn@amazon.com>
Cc: Dilip Kumar <dilipbalaut@gmail.com>
Cc: Vik Fearing <vik@postgresfriends.org>
Cc: postgresql@zr40.nl <postgresql@zr40.nl>
Cc: Alexander Korotkov <aekorotkov@gmail.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Cc: Robert Haas <robertmhaas@gmail.com>
Subject: Re: BUG #17220: ALTER INDEX ALTER COLUMN SET (..) with an optionless opclass makes index and table unusable
Date: Thu, 14 Oct 2021 11:07:21 +0900
Message-ID: <YWeQ2UXvXUH/Gt4T@paquier.xyz> (raw)
In-Reply-To: <678CF71A-934B-4FD0-BF63-E41D795E41C9@amazon.com>
References: <17220-15d684c6c2171a83@postgresql.org>
<9b91d241-4d05-e5a4-67da-5b8acdb9a2f1@postgresfriends.org>
<E29EF33B-CA1A-4146-BFCF-C8EC08FB91A8@amazon.com>
<YWZViGJFJkrgBKwk@paquier.xyz>
<CAFiTN-t9JSHmCtOtB_28BmAXEantUC2=1OKV_irAPAoxnzPadA@mail.gmail.com>
<678CF71A-934B-4FD0-BF63-E41D795E41C9@amazon.com>
On Wed, Oct 13, 2021 at 05:20:56PM +0000, Bossart, Nathan wrote:
> AFAICT the fact that these commands can succeed at all seems to be
> unintentional, and I wonder if modifying these options requires extra
> steps such as rebuilding the index.
I was looking at all this business with more attention, and this code
block is standing out in analyze.c:
/*
* Now we can compute the statistics for the expression columns.
*/
if (numindexrows > 0)
{
MemoryContextSwitchTo(col_context);
for (i = 0; i < attr_cnt; i++)
{
VacAttrStats *stats = thisdata->vacattrstats[i];
AttributeOpts *aopt =
get_attribute_options(stats->attr->attrelid,
stats->attr->attnum);
stats->exprvals = exprvals + i;
stats->exprnulls = exprnulls + i;
stats->rowstride = attr_cnt;
stats->compute_stats(stats,
ind_fetch_func,
numindexrows,
totalindexrows);
/*
* If the n_distinct option is specified, it overrides the
* above computation. For indices, we always use just
* n_distinct, not n_distinct_inherited.
*/
if (aopt != NULL && aopt->n_distinct != 0.0)
stats->stadistinct = aopt->n_distinct;
MemoryContextResetAndDeleteChildren(col_context);
}
}
When computing statistics on an index expression, this code means that
we would grab the value of n_distinct from the *index* if set and
force the stats to use it, and not use what the parent table has. For
example, say:
create table aa (a int);
insert into aa values (generate_series(1,1000));
create index aai on aa((a+a)) where a > 500;
alter index aai alter column expr set (n_distinct = 2);
analyze aa; -- n_distinct forced to 2.0 for the index stats
This code comes from 76a47c0 back in 2010. In PG <= 12, this would
work, but that does not as of 13~. Enforcing n_distinct for index
attributes was discussed back when this code was introduced:
https://www.postgresql.org/message-id/603c8f071001101127w3253899vb3f3e15073638774@mail.gmail.com
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.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: michael@paquier.xyz, bossartn@amazon.com, dilipbalaut@gmail.com, vik@postgresfriends.org, postgresql@zr40.nl, aekorotkov@gmail.com, pgsql-hackers@postgresql.org, robertmhaas@gmail.com
Subject: Re: BUG #17220: ALTER INDEX ALTER COLUMN SET (..) with an optionless opclass makes index and table unusable
In-Reply-To: <YWeQ2UXvXUH/Gt4T@paquier.xyz>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox