Add support for ALTER INDEX .. ALTER [COLUMN] col_num {SET,RESET}
Hi all,
Since 911e702 (13~), it is possible to define opclass parameters for
index attributes as of CREATE INDEX, but we lack an equivalent grammar
for ALTER INDEX. I was looking at that, and it seems natural to me to
do the same thing as what we do for SET STATISTICS, where we use a
column number to decide which column should be used to set or reset a
parameter.
It happens that most of the infrastructure is already in place to
allow that to work, I just had to push a bit the parser and
tablecmds.c to handle that, thanks to the fact that opclass parameters
are stored in pg_attribute in the same fashion as table parameters,
where we use a simple text array for each param/value pair. The only
tweak is to go through the correct validation option, aka
index_opclass_options() (this was discussed on the thread that led to
fdd8857). So this adds much more flexibility to the opclass handling
for indexes.
The attached does the work, with tests and documentation added to all
the places I could think about while reviewing the existing opclass
code for indexes. There is no need to worry about pg_dump, as opclass
parameters are loaded with CREATE INDEX.
I am adding that to the upcoming CF. Thoughts?
Thanks,
--
Michael
Attachments:
v1-0001-Add-support-for-ALTER-INDEX-ALTER-COLUMN-SET-RESE.patchtext/x-diff; charset=us-asciiDownload
From fb5710f092da6ebc0c1ebd6b9a2a67d3cc5717d1 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Fri, 29 Oct 2021 13:54:07 +0900
Subject: [PATCH v1] Add support for ALTER INDEX ALTER COLUMN {SET,RESET}
---
src/backend/commands/tablecmds.c | 56 +++++++++++++++++------
src/backend/parser/gram.y | 32 +++++++++++++
src/test/regress/expected/btree_index.out | 9 ++--
src/test/regress/expected/tsearch.out | 22 +++++++++
src/test/regress/sql/btree_index.sql | 3 +-
src/test/regress/sql/tsearch.sql | 8 ++++
doc/src/sgml/indices.sgml | 2 +
doc/src/sgml/ref/alter_index.sgml | 25 ++++++++++
8 files changed, 138 insertions(+), 19 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 857cc5ce6e..0111d6d0eb 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -447,7 +447,8 @@ static ObjectAddress ATExecDropExpression(Relation rel, const char *colName, boo
static ObjectAddress ATExecSetStatistics(Relation rel, const char *colName, int16 colNum,
Node *newValue, LOCKMODE lockmode);
static ObjectAddress ATExecSetOptions(Relation rel, const char *colName,
- Node *options, bool isReset, LOCKMODE lockmode);
+ int16 colNum, Node *options,
+ bool isReset, LOCKMODE lockmode);
static ObjectAddress ATExecSetStorage(Relation rel, const char *colName,
Node *newValue, LOCKMODE lockmode);
static void ATPrepDropColumn(List **wqueue, Relation rel, bool recurse, bool recursing,
@@ -4528,7 +4529,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
break;
case AT_SetOptions: /* ALTER COLUMN SET ( options ) */
case AT_ResetOptions: /* ALTER COLUMN RESET ( options ) */
- ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_MATVIEW | ATT_FOREIGN_TABLE);
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_MATVIEW | ATT_INDEX | ATT_PARTITIONED_INDEX | ATT_FOREIGN_TABLE);
/* This command never recurses */
pass = AT_PASS_MISC;
break;
@@ -4909,10 +4910,10 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab,
address = ATExecSetStatistics(rel, cmd->name, cmd->num, cmd->def, lockmode);
break;
case AT_SetOptions: /* ALTER COLUMN SET ( options ) */
- address = ATExecSetOptions(rel, cmd->name, cmd->def, false, lockmode);
+ address = ATExecSetOptions(rel, cmd->name, cmd->num, cmd->def, false, lockmode);
break;
case AT_ResetOptions: /* ALTER COLUMN RESET ( options ) */
- address = ATExecSetOptions(rel, cmd->name, cmd->def, true, lockmode);
+ address = ATExecSetOptions(rel, cmd->name, cmd->num, cmd->def, true, lockmode);
break;
case AT_SetStorage: /* ALTER COLUMN SET STORAGE */
address = ATExecSetStorage(rel, cmd->name, cmd->def, lockmode);
@@ -8059,8 +8060,8 @@ ATExecSetStatistics(Relation rel, const char *colName, int16 colNum, Node *newVa
* Return value is the address of the modified column
*/
static ObjectAddress
-ATExecSetOptions(Relation rel, const char *colName, Node *options,
- bool isReset, LOCKMODE lockmode)
+ATExecSetOptions(Relation rel, const char *colName, int16 colNum,
+ Node *options, bool isReset, LOCKMODE lockmode)
{
Relation attrelation;
HeapTuple tuple,
@@ -8075,15 +8076,40 @@ ATExecSetOptions(Relation rel, const char *colName, Node *options,
bool repl_null[Natts_pg_attribute];
bool repl_repl[Natts_pg_attribute];
+ /*
+ * We allow referencing columns by numbers only for indexes, since table
+ * column numbers could contain gaps if columns are later dropped.
+ */
+ if (rel->rd_rel->relkind != RELKIND_INDEX &&
+ rel->rd_rel->relkind != RELKIND_PARTITIONED_INDEX &&
+ !colName)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot refer to non-index column by number")));
+
attrelation = table_open(AttributeRelationId, RowExclusiveLock);
- tuple = SearchSysCacheAttName(RelationGetRelid(rel), colName);
+ if (colName)
+ {
+ tuple = SearchSysCacheAttName(RelationGetRelid(rel), colName);
+
+ if (!HeapTupleIsValid(tuple))
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_COLUMN),
+ errmsg("column \"%s\" of relation \"%s\" does not exist",
+ colName, RelationGetRelationName(rel))));
+ }
+ else
+ {
+ tuple = SearchSysCacheAttNum(RelationGetRelid(rel), colNum);
+
+ if (!HeapTupleIsValid(tuple))
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_COLUMN),
+ errmsg("column number %d of relation \"%s\" does not exist",
+ colNum, RelationGetRelationName(rel))));
+ }
- if (!HeapTupleIsValid(tuple))
- ereport(ERROR,
- (errcode(ERRCODE_UNDEFINED_COLUMN),
- errmsg("column \"%s\" of relation \"%s\" does not exist",
- colName, RelationGetRelationName(rel))));
attrtuple = (Form_pg_attribute) GETSTRUCT(tuple);
attnum = attrtuple->attnum;
@@ -8100,7 +8126,11 @@ ATExecSetOptions(Relation rel, const char *colName, Node *options,
castNode(List, options), NULL, NULL,
false, isReset);
/* Validate new options */
- (void) attribute_reloptions(newOptions, true);
+ if (rel->rd_rel->relkind == RELKIND_INDEX ||
+ rel->rd_rel->relkind == RELKIND_PARTITIONED_INDEX)
+ (void) index_opclass_options(rel, attnum, newOptions, true);
+ else
+ (void) attribute_reloptions(newOptions, true);
/* Build new tuple. */
memset(repl_null, false, sizeof(repl_null));
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index d0eb80e69c..0122862aeb 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -2290,6 +2290,22 @@ alter_table_cmd:
n->def = (Node *) $5;
$$ = (Node *)n;
}
+ /* ALTER TABLE <name> ALTER [COLUMN] <colnum> SET ( column_parameter = value [, ... ] */
+ | ALTER opt_column Iconst SET reloptions
+ {
+ AlterTableCmd *n = makeNode(AlterTableCmd);
+
+ if ($3 <= 0 || $3 > PG_INT16_MAX)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("column number must be in range from 1 to %d", PG_INT16_MAX),
+ parser_errposition(@3)));
+
+ n->subtype = AT_SetOptions;
+ n->num = (int16) $3;
+ n->def = (Node *) $5;
+ $$ = (Node *)n;
+ }
/* ALTER TABLE <name> ALTER [COLUMN] <colname> RESET ( column_parameter [, ... ] ) */
| ALTER opt_column ColId RESET reloptions
{
@@ -2299,6 +2315,22 @@ alter_table_cmd:
n->def = (Node *) $5;
$$ = (Node *)n;
}
+ /* ALTER TABLE <name> ALTER [COLUMN] <colnum> RESET ( column_parameter = value [, ... ] */
+ | ALTER opt_column Iconst RESET reloptions
+ {
+ AlterTableCmd *n = makeNode(AlterTableCmd);
+
+ if ($3 <= 0 || $3 > PG_INT16_MAX)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("column number must be in range from 1 to %d", PG_INT16_MAX),
+ parser_errposition(@3)));
+
+ n->subtype = AT_ResetOptions;
+ n->num = (int16) $3;
+ n->def = (Node *) $5;
+ $$ = (Node *)n;
+ }
/* ALTER TABLE <name> ALTER [COLUMN] <colname> SET STORAGE <storagemode> */
| ALTER opt_column ColId SET STORAGE ColId
{
diff --git a/src/test/regress/expected/btree_index.out b/src/test/regress/expected/btree_index.out
index c43e023716..716583c830 100644
--- a/src/test/regress/expected/btree_index.out
+++ b/src/test/regress/expected/btree_index.out
@@ -330,16 +330,15 @@ INSERT INTO delete_test_table SELECT i, 1, 2, 3 FROM generate_series(1,1000) i;
create index on btree_tall_tbl (id int4_ops(foo=1));
ERROR: operator class int4_ops has no options
-- Test case of ALTER INDEX with abuse of column names for indexes.
--- This grammar is not officially supported, but the parser allows it.
+-- This grammar is allowed by the parser, but table parameters are not
+-- supported by indexes.
CREATE INDEX btree_tall_idx2 ON btree_tall_tbl (id);
ALTER INDEX btree_tall_idx2 ALTER COLUMN id SET (n_distinct=100);
-ERROR: ALTER action ALTER COLUMN ... SET cannot be performed on relation "btree_tall_idx2"
-DETAIL: This operation is not supported for indexes.
+ERROR: operator class int4_ops has no options
DROP INDEX btree_tall_idx2;
-- Partitioned index
CREATE TABLE btree_part (id int4) PARTITION BY RANGE (id);
CREATE INDEX btree_part_idx ON btree_part(id);
ALTER INDEX btree_part_idx ALTER COLUMN id SET (n_distinct=100);
-ERROR: ALTER action ALTER COLUMN ... SET cannot be performed on relation "btree_part_idx"
-DETAIL: This operation is not supported for partitioned indexes.
+ERROR: operator class int4_ops has no options
DROP TABLE btree_part;
diff --git a/src/test/regress/expected/tsearch.out b/src/test/regress/expected/tsearch.out
index 45b92a6338..e09c7627de 100644
--- a/src/test/regress/expected/tsearch.out
+++ b/src/test/regress/expected/tsearch.out
@@ -536,6 +536,28 @@ Indexes:
"wowidx" gist (a)
"wowidx2" gist (a tsvector_ops (siglen='1'))
+ALTER INDEX wowidx2 ALTER COLUMN 1 RESET (siglen);
+\d test_tsvector
+ Table "public.test_tsvector"
+ Column | Type | Collation | Nullable | Default
+--------+----------+-----------+----------+---------
+ t | text | | |
+ a | tsvector | | |
+Indexes:
+ "wowidx" gist (a)
+ "wowidx2" gist (a)
+
+ALTER INDEX wowidx2 ALTER COLUMN 1 SET (siglen=2);
+\d test_tsvector
+ Table "public.test_tsvector"
+ Column | Type | Collation | Nullable | Default
+--------+----------+-----------+----------+---------
+ t | text | | |
+ a | tsvector | | |
+Indexes:
+ "wowidx" gist (a)
+ "wowidx2" gist (a tsvector_ops (siglen='2'))
+
DROP INDEX wowidx;
EXPLAIN (costs off) SELECT count(*) FROM test_tsvector WHERE a @@ 'wr|qh';
QUERY PLAN
diff --git a/src/test/regress/sql/btree_index.sql b/src/test/regress/sql/btree_index.sql
index c34502249f..8fd49dfc0d 100644
--- a/src/test/regress/sql/btree_index.sql
+++ b/src/test/regress/sql/btree_index.sql
@@ -174,7 +174,8 @@ INSERT INTO delete_test_table SELECT i, 1, 2, 3 FROM generate_series(1,1000) i;
create index on btree_tall_tbl (id int4_ops(foo=1));
-- Test case of ALTER INDEX with abuse of column names for indexes.
--- This grammar is not officially supported, but the parser allows it.
+-- This grammar is allowed by the parser, but table parameters are not
+-- supported by indexes.
CREATE INDEX btree_tall_idx2 ON btree_tall_tbl (id);
ALTER INDEX btree_tall_idx2 ALTER COLUMN id SET (n_distinct=100);
DROP INDEX btree_tall_idx2;
diff --git a/src/test/regress/sql/tsearch.sql b/src/test/regress/sql/tsearch.sql
index d929210998..7fdc2bb06c 100644
--- a/src/test/regress/sql/tsearch.sql
+++ b/src/test/regress/sql/tsearch.sql
@@ -140,6 +140,14 @@ CREATE INDEX wowidx2 ON test_tsvector USING gist (a tsvector_ops(siglen=1));
\d test_tsvector
+ALTER INDEX wowidx2 ALTER COLUMN 1 RESET (siglen);
+
+\d test_tsvector
+
+ALTER INDEX wowidx2 ALTER COLUMN 1 SET (siglen=2);
+
+\d test_tsvector
+
DROP INDEX wowidx;
EXPLAIN (costs off) SELECT count(*) FROM test_tsvector WHERE a @@ 'wr|qh';
diff --git a/doc/src/sgml/indices.sgml b/doc/src/sgml/indices.sgml
index 56fbd45178..9a8ec977ac 100644
--- a/doc/src/sgml/indices.sgml
+++ b/doc/src/sgml/indices.sgml
@@ -1316,6 +1316,8 @@ SELECT target FROM tests WHERE subject = 'some-subject' AND success;
class</firstterm> for each column of an index.
<synopsis>
CREATE INDEX <replaceable>name</replaceable> ON <replaceable>table</replaceable> (<replaceable>column</replaceable> <replaceable>opclass</replaceable> [ ( <replaceable>opclass_options</replaceable> ) ] <optional><replaceable>sort options</replaceable></optional> <optional>, ...</optional>);
+ALTER INDEX <replaceable>name</replaceable> ALTER [ COLUMN ] <replaceable>column_number</replaceable> SET ( <replaceable>opclass_parameter</replaceable> = <replaceable>value</replaceable> <optional>, ...</optional> )
+ALTER INDEX <replaceable>name</replaceable> ALTER [ COLUMN ] <replaceable>column_number</replaceable> RESET ( <replaceable>opclass_parameter</replaceable> <optional>, ...</optional> )
</synopsis>
The operator class identifies the operators to be used by the index
for that column. For example, a B-tree index on the type <type>int4</type>
diff --git a/doc/src/sgml/ref/alter_index.sgml b/doc/src/sgml/ref/alter_index.sgml
index e26efec064..ddab7bee8f 100644
--- a/doc/src/sgml/ref/alter_index.sgml
+++ b/doc/src/sgml/ref/alter_index.sgml
@@ -27,6 +27,8 @@ ALTER INDEX <replaceable class="parameter">name</replaceable> ATTACH PARTITION <
ALTER INDEX <replaceable class="parameter">name</replaceable> [ NO ] DEPENDS ON EXTENSION <replaceable class="parameter">extension_name</replaceable>
ALTER INDEX [ IF EXISTS ] <replaceable class="parameter">name</replaceable> SET ( <replaceable class="parameter">storage_parameter</replaceable> [= <replaceable class="parameter">value</replaceable>] [, ... ] )
ALTER INDEX [ IF EXISTS ] <replaceable class="parameter">name</replaceable> RESET ( <replaceable class="parameter">storage_parameter</replaceable> [, ... ] )
+ALTER INDEX [ IF EXISTS ] <replaceable class="parameter">name</replaceable> ALTER [ COLUMN ] <replaceable class="parameter">column_number</replaceable> SET ( <replaceable class="parameter">opclass_parameter</replaceable> = <replaceable class="parameter">value</replaceable> [, ... ] )
+ALTER INDEX [ IF EXISTS ] <replaceable class="parameter">name</replaceable> ALTER [ COLUMN ] <replaceable class="parameter">column_number</replaceable> RESET ( <replaceable class="parameter">opclass_parameter</replaceable> [, ... ] )
ALTER INDEX [ IF EXISTS ] <replaceable class="parameter">name</replaceable> ALTER [ COLUMN ] <replaceable class="parameter">column_number</replaceable>
SET STATISTICS <replaceable class="parameter">integer</replaceable>
ALTER INDEX ALL IN TABLESPACE <replaceable class="parameter">name</replaceable> [ OWNED BY <replaceable class="parameter">role_name</replaceable> [, ... ] ]
@@ -139,6 +141,18 @@ ALTER INDEX ALL IN TABLESPACE <replaceable class="parameter">name</replaceable>
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>ALTER [ COLUMN ] <replaceable class="parameter">column_number</replaceable> SET ( <replaceable class="parameter">opclass_parameter</replaceable> = <replaceable class="parameter">value</replaceable> [, ... ] )</literal></term>
+ <term><literal>ALTER [ COLUMN ] <replaceable class="parameter">column_number</replaceable> RESET ( <replaceable class="parameter">opclass_parameter</replaceable> [, ... ] )</literal></term>
+ <listitem>
+ <para>
+ This form sets or resets per-attribute operator class parameters.
+ Since expressions lack a unique name, we refer to them using the
+ ordinal number of the index column.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><literal>ALTER [ COLUMN ] <replaceable class="parameter">column_number</replaceable> SET STATISTICS <replaceable class="parameter">integer</replaceable></literal></term>
<listitem>
@@ -207,6 +221,17 @@ ALTER INDEX ALL IN TABLESPACE <replaceable class="parameter">name</replaceable>
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><replaceable class="parameter">opclass_parameter</replaceable></term>
+ <listitem>
+ <para>
+ The name of an operator class parameter. See
+ <xref linkend="indexes-opclass"/> and <xref linkend="xindex"/>
+ for details.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><replaceable class="parameter">tablespace_name</replaceable></term>
<listitem>
--
2.33.0
On Fri, Oct 29, 2021 at 02:09:34PM +0900, Michael Paquier wrote:
The attached does the work, with tests and documentation added to all
the places I could think about while reviewing the existing opclass
code for indexes. There is no need to worry about pg_dump, as opclass
parameters are loaded with CREATE INDEX.
By the way, while looking at this area of the code (particularly
tsvector and gist), I was under the impression that it is safe to
assume that we don't particularly need to rebuild the index when
changing its attoptions this way, but I have also the feeling to be
rather naive here.
Alexander, you have much more experience in this area than I do, what
are your thoughts on the matter?
--
Michael
On 10/29/21, 2:14 AM, "Michael Paquier" <michael@paquier.xyz> wrote:
By the way, while looking at this area of the code (particularly
tsvector and gist), I was under the impression that it is safe to
assume that we don't particularly need to rebuild the index when
changing its attoptions this way, but I have also the feeling to be
rather naive here.
This is my concern as well. I remember seeing some weird errors in
the thread that led to fdd8857, and I was able to reproduce it with
this new patch. If you apply the following patch for the hstore test,
it will begin failing.
--
diff --git a/contrib/hstore/expected/hstore.out b/contrib/hstore/expected/hstore.out
index 64a3272b9c..a4c78a40c8 100644
--- a/contrib/hstore/expected/hstore.out
+++ b/contrib/hstore/expected/hstore.out
@@ -1352,6 +1352,7 @@ create index hidx on testhstore using gist(h gist_hstore_ops(siglen=2025));
ERROR: value 2025 out of bounds for option "siglen"
DETAIL: Valid values are between "1" and "2024".
create index hidx on testhstore using gist(h gist_hstore_ops(siglen=2024));
+alter index hidx alter column 1 set (siglen=100);
set enable_seqscan=off;
select count(*) from testhstore where h @> 'wait=>NULL';
count
diff --git a/contrib/hstore/sql/hstore.sql b/contrib/hstore/sql/hstore.sql
index a59db66b0a..5e2a1ec99f 100644
--- a/contrib/hstore/sql/hstore.sql
+++ b/contrib/hstore/sql/hstore.sql
@@ -308,6 +308,7 @@ drop index hidx;
create index hidx on testhstore using gist(h gist_hstore_ops(siglen=0));
create index hidx on testhstore using gist(h gist_hstore_ops(siglen=2025));
create index hidx on testhstore using gist(h gist_hstore_ops(siglen=2024));
+alter index hidx alter column 1 set (siglen=100);
set enable_seqscan=off;select count(*) from testhstore where h @> 'wait=>NULL';
--
However, if you add a REINDEX right after the ALTER INDEX command, it
will succeed.
Nathan
Hi, Michael!
On Fri, Oct 29, 2021 at 12:10 PM Michael Paquier <michael@paquier.xyz> wrote:
On Fri, Oct 29, 2021 at 02:09:34PM +0900, Michael Paquier wrote:
The attached does the work, with tests and documentation added to all
the places I could think about while reviewing the existing opclass
code for indexes. There is no need to worry about pg_dump, as opclass
parameters are loaded with CREATE INDEX.By the way, while looking at this area of the code (particularly
tsvector and gist), I was under the impression that it is safe to
assume that we don't particularly need to rebuild the index when
changing its attoptions this way, but I have also the feeling to be
rather naive here.Alexander, you have much more experience in this area than I do, what
are your thoughts on the matter?
Good question, thanks.
I keep the following use cases in my mind for this feature:
1) One could set up paths to index within the json document (using
jsonpath or another language);
2) The way to index particular paths in json document: for instance,
we can index string "as is" or just hash value;
3) For signatures, we may specify the number of bits to set in
addition to the signature length.
All these cases require index rebuild to change the opclass option. I
can imagine there could be options, which don't require index rebuild
to change. For instance, a split algorithm for R-tree-like GiST index
could be changed on the fly. But not much point to change it on the
existing index, though.
We may introduce the additional property of opclass option
"mutability", which specifies whether it's possible to change for the
existing index. So, I don't think this is viable for now. I suggest
postponing it. Leave the options immutable, until we get some solid
use-cases of mutable options.
------
Regards,
Alexander Korotkov
On Fri, Oct 29, 2021 at 11:15:08PM +0300, Alexander Korotkov wrote:
All these cases require index rebuild to change the opclass option. I
can imagine there could be options, which don't require index rebuild
to change. For instance, a split algorithm for R-tree-like GiST index
could be changed on the fly. But not much point to change it on the
existing index, though.
This point alone is enough to say that what the patch does is not
enough, then, thanks. An index rebuild would require an exclusive
lock, but AT_SetOptions uses a share update exclusive lock. We could
keep the same lock level by trigerring a concurrent reindex while
forbidding running this command flavor on indexes within a transaction
block, though. I guess that this still makes the life or users a bit
easier to avoid mistakes with complicated index expressions or
predicates, but that also feels like adding code for little gain.
Another thing that could be done is to mark the index as invalid once
its set of opclass parameters is updated. That would be simpler,
while allowing users to fire a concurrent or non-concurrent rebuild at
will after an ALTER INDEX.
--
Michael
On Fri, Oct 29, 2021 at 06:23:34PM +0000, Bossart, Nathan wrote:
This is my concern as well. I remember seeing some weird errors in
the thread that led to fdd8857, and I was able to reproduce it with
this new patch. If you apply the following patch for the hstore test,
it will begin failing.
Indeed, thanks. That's the kind of things I was trying with other
opclasses, actually. Missed this one.
--
Michael
On Sat, Oct 30, 2021 at 09:45:50AM +0900, Michael Paquier wrote:
Another thing that could be done is to mark the index as invalid once
its set of opclass parameters is updated. That would be simpler,
while allowing users to fire a concurrent or non-concurrent rebuild at
will after an ALTER INDEX.
For the note here, I have looked at this possibility. And things
become very tricky when it comes to indexes marked as either
indisclustered or indisreplident. REINDEX CONCURRENTLY has the
particularity to switch both parameters to false when swapping to the
new index because we don't need the new index anymore. Invalid
indexes can create with CREATE INDEX CONCURRENTLY (constraint failure
at creation for example), but they won't have any of those flags set.
So we assume now that indisinvalid is linked to both of them.
This makes the problem mentioned upthread trickier than it looks, as
we'd need to decide what to do after an ALTER INDEX that just switches
an index to be invalid if any of these are set for the existing
index.
--
Michael