public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v3] Add a new COLLATION option to REINDEX. 2+ messages / 2 participants [nested] [flat]
* [PATCH v3] Add a new COLLATION option to REINDEX. @ 2020-12-03 07:54 Julien Rouhaud <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Julien Rouhaud @ 2020-12-03 07:54 UTC (permalink / raw) --- doc/src/sgml/ref/reindex.sgml | 13 +++++ src/backend/catalog/index.c | 59 +++++++++++++++++++++- src/backend/commands/indexcmds.c | 13 +++-- src/backend/utils/cache/relcache.c | 43 ++++++++++++++++ src/include/catalog/index.h | 4 ++ src/include/utils/relcache.h | 1 + src/test/regress/expected/create_index.out | 10 ++++ src/test/regress/sql/create_index.sql | 10 ++++ 8 files changed, 149 insertions(+), 4 deletions(-) diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml index 07795b5737..ead2904b67 100644 --- a/doc/src/sgml/ref/reindex.sgml +++ b/doc/src/sgml/ref/reindex.sgml @@ -25,6 +25,7 @@ REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { IN <phrase>where <replaceable class="parameter">option</replaceable> can be one of:</phrase> + COLLATION [ <replaceable class="parameter">text</replaceable> ] CONCURRENTLY [ <replaceable class="parameter">boolean</replaceable> ] TABLESPACE <replaceable class="parameter">new_tablespace</replaceable> VERBOSE [ <replaceable class="parameter">boolean</replaceable> ] @@ -169,6 +170,18 @@ REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { IN </listitem> </varlistentry> + <varlistentry> + <term><literal>COLLATION</literal></term> + <listitem> + <para> + This option can be used to filter the list of indexes to rebuild. The + only allowed value is <literal>'not_current'</literal>, which will only + process indexes that depend on a collation version different than the + current one. + </para> + </listitem> + </varlistentry> + <varlistentry> <term><literal>CONCURRENTLY</literal></term> <listitem> diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 1cb9172a5f..b74ee79d38 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -100,6 +100,12 @@ typedef struct Oid pendingReindexedIndexes[FLEXIBLE_ARRAY_MEMBER]; } SerializedReindexState; +typedef struct +{ + Oid relid; /* targetr index oid */ + bool deprecated; /* depends on at least on deprected collation? */ +} IndexHasDeprecatedColl; + /* non-export function prototypes */ static bool relationHasPrimaryKey(Relation rel); static TupleDesc ConstructTupleDescriptor(Relation heapRelation, @@ -1350,6 +1356,57 @@ index_check_collation_versions(Oid relid) list_free(context.warned_colls); } +/* + * Detect if an index depends on at least one deprecated collation. + * This is a callback for visitDependenciesOf(). + */ +static bool +do_check_index_has_deprecated_collation(const ObjectAddress *otherObject, + const char *version, + char **new_version, + void *data) +{ + IndexHasDeprecatedColl *context = data; + char *current_version; + + /* We only care about dependencies on collations. */ + if (otherObject->classId != CollationRelationId) + return false; + + /* Fast exit if we already found a deprecated collation version. */ + if (context->deprecated) + return false; + + /* Ask the provider for the current version. Give up if unsupported. */ + current_version = get_collation_version_for_oid(otherObject->objectId); + if (!current_version) + return false; + + if (!version || strcmp(version, current_version) != 0) + context->deprecated = true; + + return false; +} + +bool +index_has_deprecated_collation(Oid relid) +{ + ObjectAddress object; + IndexHasDeprecatedColl context; + + object.classId = RelationRelationId; + object.objectId = relid; + object.objectSubId = 0; + + context.relid = relid; + context.deprecated = false; + + visitDependenciesOf(&object, &do_check_index_has_deprecated_collation, + &context); + + return context.deprecated; +} + /* * Update the version for collations. A callback for visitDependenciesOf(). */ @@ -3930,7 +3987,7 @@ reindex_relation(Oid relid, int flags, ReindexParams *params) * relcache to get this with a sequential scan if ignoring system * indexes.) */ - indexIds = RelationGetIndexList(rel); + indexIds = RelationGetIndexListFiltered(rel, params->options); if (flags & REINDEX_REL_SUPPRESS_INDEX_USE) { diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 127ba7835d..9bf69ff9d7 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -2475,6 +2475,7 @@ ExecReindex(ParseState *pstate, ReindexStmt *stmt, bool isTopLevel) bool concurrently = false; bool verbose = false; char *tablespacename = NULL; + bool coll_filter = false; /* Parse option list */ foreach(lc, stmt->params) @@ -2487,6 +2488,9 @@ ExecReindex(ParseState *pstate, ReindexStmt *stmt, bool isTopLevel) concurrently = defGetBoolean(opt); else if (strcmp(opt->defname, "tablespace") == 0) tablespacename = defGetString(opt); + else if ((strcmp(opt->defname, "collation") == 0) + && (strcmp(defGetString(opt), "not_current") == 0)) + coll_filter = true; else ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), @@ -2501,7 +2505,8 @@ ExecReindex(ParseState *pstate, ReindexStmt *stmt, bool isTopLevel) params.options = (verbose ? REINDEXOPT_VERBOSE : 0) | - (concurrently ? REINDEXOPT_CONCURRENTLY : 0); + (concurrently ? REINDEXOPT_CONCURRENTLY : 0) | + (coll_filter ? REINDEXOPT_COLL_NOT_CURRENT : 0); /* * Assign the tablespace OID to move indexes to, with InvalidOid to do @@ -3298,7 +3303,8 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params) RelationGetRelationName(heapRelation)))); /* Add all the valid indexes of relation to list */ - foreach(lc, RelationGetIndexList(heapRelation)) + foreach(lc, RelationGetIndexListFiltered(heapRelation, + params->options)) { Oid cellOid = lfirst_oid(lc); Relation indexRelation = index_open(cellOid, @@ -3350,7 +3356,8 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params) MemoryContextSwitchTo(oldcontext); - foreach(lc2, RelationGetIndexList(toastRelation)) + foreach(lc2, RelationGetIndexListFiltered(toastRelation, + params->options)) { Oid cellOid = lfirst_oid(lc2); Relation indexRelation = index_open(cellOid, diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 7ef510cd01..efef6c5ae5 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -4620,6 +4620,49 @@ RelationGetIndexList(Relation relation) return result; } +/* + * RelationGetIndexListFiltered -- get a filtered list of indexes on this + * relation. + * + * Calls RelationGetIndexList and filters out the result as required by the + * caller. The filters are cumulative, although for now the only supported + * filter is for indexes that don't depend on deprecated collation version. + */ +List * +RelationGetIndexListFiltered(Relation relation, bits32 options) +{ + List *result, + *full_list; + ListCell *lc; + + full_list = RelationGetIndexList(relation); + + /* Fast exit if no filtering was asked, or if the list if empty. */ + if (!reindexHasFilter(options) || full_list == NIL) + return full_list; + + result = NIL; + foreach(lc, full_list) + { + Oid indexOid = lfirst_oid(lc); + + /* + * The caller wants to discard indexes that don't depend on a + * deprecated collation version. + */ + if ((options & REINDEXOPT_COLL_NOT_CURRENT) != 0) + { + if (!index_has_deprecated_collation(indexOid)) + continue; + } + + /* Index passed all filters, keep it */ + result = lappend_oid(result, indexOid); + } + + return result; +} + /* * RelationGetStatExtList * get a list of OIDs of statistics objects on this relation diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h index e22d506436..3f08e5f454 100644 --- a/src/include/catalog/index.h +++ b/src/include/catalog/index.h @@ -42,6 +42,9 @@ typedef struct ReindexParams #define REINDEXOPT_REPORT_PROGRESS 0x02 /* report pgstat progress */ #define REINDEXOPT_MISSING_OK 0x04 /* skip missing relations */ #define REINDEXOPT_CONCURRENTLY 0x08 /* concurrent mode */ +#define REINDEXOPT_COLL_NOT_CURRENT 0x10/* outdated collation only */ + +#define reindexHasFilter(x) ((x & REINDEXOPT_COLL_NOT_CURRENT) != 0) /* state info for validate_index bulkdelete callback */ typedef struct ValidateIndexState @@ -137,6 +140,7 @@ extern void FormIndexDatum(IndexInfo *indexInfo, bool *isnull); extern void index_check_collation_versions(Oid relid); +extern bool index_has_deprecated_collation(Oid relid); extern void index_update_collation_versions(Oid relid, Oid coll); extern void index_build(Relation heapRelation, diff --git a/src/include/utils/relcache.h b/src/include/utils/relcache.h index 2fcdf79323..a7a2272abd 100644 --- a/src/include/utils/relcache.h +++ b/src/include/utils/relcache.h @@ -45,6 +45,7 @@ extern void RelationClose(Relation relation); */ extern List *RelationGetFKeyList(Relation relation); extern List *RelationGetIndexList(Relation relation); +extern List *RelationGetIndexListFiltered(Relation relation, bits32 options); extern List *RelationGetStatExtList(Relation relation); extern Oid RelationGetPrimaryKeyIndex(Relation relation); extern Oid RelationGetReplicaIndex(Relation relation); diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index ce734f7ef3..3339fcb831 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -2018,6 +2018,16 @@ INFO: index "reindex_verbose_pkey" was reindexed \set VERBOSITY default DROP TABLE reindex_verbose; -- +-- REINDEX (COLLATION 'not_current') +-- +CREATE TABLE reindex_coll(id integer primary key); +\set VERBOSITY terse \\ -- suppress machine-dependent details +-- no suitable index should be found +REINDEX (COLLATION 'not_current') TABLE reindex_coll; +NOTICE: table "reindex_coll" has no indexes to reindex +\set VERBOSITY default +DROP TABLE reindex_coll; +-- -- REINDEX CONCURRENTLY -- CREATE TABLE concur_reindex_tab (c1 int); diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index fd4f30876e..01d193b9de 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -790,6 +790,16 @@ REINDEX (VERBOSE) TABLE reindex_verbose; \set VERBOSITY default DROP TABLE reindex_verbose; +-- +-- REINDEX (COLLATION 'not_current') +-- +CREATE TABLE reindex_coll(id integer primary key); +\set VERBOSITY terse \\ -- suppress machine-dependent details +-- no suitable index should be found +REINDEX (COLLATION 'not_current') TABLE reindex_coll; +\set VERBOSITY default +DROP TABLE reindex_coll; + -- -- REINDEX CONCURRENTLY -- -- 2.20.1 --mYCpIKhGyMATD0i+-- ^ permalink raw reply [nested|flat] 2+ messages in thread
* some validate_relation_kind() tidying @ 2026-02-18 15:15 Peter Eisentraut <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Peter Eisentraut @ 2026-02-18 15:15 UTC (permalink / raw) To: pgsql-hackers We have three different functions called validate_relation_kind(), namely in src/backend/access/index/indexam.c src/backend/access/sequence/sequence.c src/backend/access/table/table.c These all check which relkinds are permitted by index_open(), sequence_open(), and table_open(), respectively, which are each wrappers around relation_open() (which accepts any relkind). I always found the one in table.c a little too mysterious, because it just checks which relkinds it does *not* want, and so if you want to know whether a particular relkind is suitable for table_open(), you need to do additional research and check what all the possible relkinds are and so on, and there is no real explanation why those choices were made. I think it would be clearer and more robust and also more consistent with the other ones if we flipped that around and listed the ones that are acceptable and why. Secondly, the sequence.c one was probably copied from the table.c one, but I think we can make the error message a bit more direct by just saying "... is not a sequence" instead of "cannot open relation". These are the two attached patches. This is just something I found while working on something else nearby. From 12e4eb279cf64ac23131c40ae0420f6b7e2db33a Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <[email protected]> Date: Wed, 18 Feb 2026 11:48:37 +0100 Subject: [PATCH 1/2] Change error message for sequence validate_relation_kind() We can just say "... is not a sequence" instead of the more complicated variant from before, which was probably copied from src/backend/access/table/table.c. Fix a typo in a comment in passing. --- src/backend/access/sequence/sequence.c | 7 +++---- src/test/regress/expected/sequence.out | 3 +-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/backend/access/sequence/sequence.c b/src/backend/access/sequence/sequence.c index 106af1477e9..6897f8432d6 100644 --- a/src/backend/access/sequence/sequence.c +++ b/src/backend/access/sequence/sequence.c @@ -63,7 +63,7 @@ sequence_close(Relation relation, LOCKMODE lockmode) /* ---------------- * validate_relation_kind - check the relation's kind * - * Make sure relkind is from a sequence. + * Make sure relkind is a sequence. * ---------------- */ static inline void @@ -72,7 +72,6 @@ validate_relation_kind(Relation r) if (r->rd_rel->relkind != RELKIND_SEQUENCE) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("cannot open relation \"%s\"", - RelationGetRelationName(r)), - errdetail_relkind_not_supported(r->rd_rel->relkind))); + errmsg("\"%s\" is not a sequence", + RelationGetRelationName(r)))); } diff --git a/src/test/regress/expected/sequence.out b/src/test/regress/expected/sequence.out index c4454e5b435..a0883b11007 100644 --- a/src/test/regress/expected/sequence.out +++ b/src/test/regress/expected/sequence.out @@ -313,8 +313,7 @@ ALTER SEQUENCE IF EXISTS sequence_test2 RESTART WITH 24 INCREMENT BY 4 MAXVALUE 36 MINVALUE 5 CYCLE; NOTICE: relation "sequence_test2" does not exist, skipping ALTER SEQUENCE serialTest1 CYCLE; -- error, not a sequence -ERROR: cannot open relation "serialtest1" -DETAIL: This operation is not supported for tables. +ERROR: "serialtest1" is not a sequence CREATE SEQUENCE sequence_test2 START WITH 32; CREATE SEQUENCE sequence_test4 INCREMENT BY -1; SELECT nextval('sequence_test2'); -- 2.53.0 From 2cd2bf31d5adb6fcdbb4528018de700094075936 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <[email protected]> Date: Wed, 18 Feb 2026 14:32:45 +0100 Subject: [PATCH 2/2] Flip logic in table validate_relation_kind It instead of checking which relkinds it shouldn't be, explicitly list the ones we accept. This is used to check which relkinds are accepted in table_open() and related functions. Before this change, figuring that out was always a few steps too complicated. This also makes changes for new relkinds more explicit instead of accidental. Finally, this makes this more aligned with the functions of the same name in src/backend/access/index/indexam.c and src/backend/access/sequence/sequence.c. --- src/backend/access/table/table.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/backend/access/table/table.c b/src/backend/access/table/table.c index fe330ae862a..ef33d75d6fa 100644 --- a/src/backend/access/table/table.c +++ b/src/backend/access/table/table.c @@ -131,15 +131,20 @@ table_close(Relation relation, LOCKMODE lockmode) /* ---------------- * validate_relation_kind - check the relation's kind * - * Make sure relkind is not index or composite type + * Make sure relkind is table-like. In particular, this excludes indexes + * and composite types, which cannot be read from in a query. * ---------------- */ static inline void validate_relation_kind(Relation r) { - if (r->rd_rel->relkind == RELKIND_INDEX || - r->rd_rel->relkind == RELKIND_PARTITIONED_INDEX || - r->rd_rel->relkind == RELKIND_COMPOSITE_TYPE) + if (r->rd_rel->relkind != RELKIND_RELATION || + r->rd_rel->relkind != RELKIND_SEQUENCE || + r->rd_rel->relkind != RELKIND_TOASTVALUE || + r->rd_rel->relkind != RELKIND_VIEW || + r->rd_rel->relkind != RELKIND_MATVIEW || + r->rd_rel->relkind != RELKIND_FOREIGN_TABLE || + r->rd_rel->relkind != RELKIND_PARTITIONED_TABLE) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), errmsg("cannot open relation \"%s\"", -- 2.53.0 Attachments: [text/plain] 0001-Change-error-message-for-sequence-validate_relation_.patch (2.2K, ../../[email protected]/2-0001-Change-error-message-for-sequence-validate_relation_.patch) download | inline diff: From 12e4eb279cf64ac23131c40ae0420f6b7e2db33a Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <[email protected]> Date: Wed, 18 Feb 2026 11:48:37 +0100 Subject: [PATCH 1/2] Change error message for sequence validate_relation_kind() We can just say "... is not a sequence" instead of the more complicated variant from before, which was probably copied from src/backend/access/table/table.c. Fix a typo in a comment in passing. --- src/backend/access/sequence/sequence.c | 7 +++---- src/test/regress/expected/sequence.out | 3 +-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/backend/access/sequence/sequence.c b/src/backend/access/sequence/sequence.c index 106af1477e9..6897f8432d6 100644 --- a/src/backend/access/sequence/sequence.c +++ b/src/backend/access/sequence/sequence.c @@ -63,7 +63,7 @@ sequence_close(Relation relation, LOCKMODE lockmode) /* ---------------- * validate_relation_kind - check the relation's kind * - * Make sure relkind is from a sequence. + * Make sure relkind is a sequence. * ---------------- */ static inline void @@ -72,7 +72,6 @@ validate_relation_kind(Relation r) if (r->rd_rel->relkind != RELKIND_SEQUENCE) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("cannot open relation \"%s\"", - RelationGetRelationName(r)), - errdetail_relkind_not_supported(r->rd_rel->relkind))); + errmsg("\"%s\" is not a sequence", + RelationGetRelationName(r)))); } diff --git a/src/test/regress/expected/sequence.out b/src/test/regress/expected/sequence.out index c4454e5b435..a0883b11007 100644 --- a/src/test/regress/expected/sequence.out +++ b/src/test/regress/expected/sequence.out @@ -313,8 +313,7 @@ ALTER SEQUENCE IF EXISTS sequence_test2 RESTART WITH 24 INCREMENT BY 4 MAXVALUE 36 MINVALUE 5 CYCLE; NOTICE: relation "sequence_test2" does not exist, skipping ALTER SEQUENCE serialTest1 CYCLE; -- error, not a sequence -ERROR: cannot open relation "serialtest1" -DETAIL: This operation is not supported for tables. +ERROR: "serialtest1" is not a sequence CREATE SEQUENCE sequence_test2 START WITH 32; CREATE SEQUENCE sequence_test4 INCREMENT BY -1; SELECT nextval('sequence_test2'); -- 2.53.0 [text/plain] 0002-Flip-logic-in-table-validate_relation_kind.patch (2.0K, ../../[email protected]/3-0002-Flip-logic-in-table-validate_relation_kind.patch) download | inline diff: From 2cd2bf31d5adb6fcdbb4528018de700094075936 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <[email protected]> Date: Wed, 18 Feb 2026 14:32:45 +0100 Subject: [PATCH 2/2] Flip logic in table validate_relation_kind It instead of checking which relkinds it shouldn't be, explicitly list the ones we accept. This is used to check which relkinds are accepted in table_open() and related functions. Before this change, figuring that out was always a few steps too complicated. This also makes changes for new relkinds more explicit instead of accidental. Finally, this makes this more aligned with the functions of the same name in src/backend/access/index/indexam.c and src/backend/access/sequence/sequence.c. --- src/backend/access/table/table.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/backend/access/table/table.c b/src/backend/access/table/table.c index fe330ae862a..ef33d75d6fa 100644 --- a/src/backend/access/table/table.c +++ b/src/backend/access/table/table.c @@ -131,15 +131,20 @@ table_close(Relation relation, LOCKMODE lockmode) /* ---------------- * validate_relation_kind - check the relation's kind * - * Make sure relkind is not index or composite type + * Make sure relkind is table-like. In particular, this excludes indexes + * and composite types, which cannot be read from in a query. * ---------------- */ static inline void validate_relation_kind(Relation r) { - if (r->rd_rel->relkind == RELKIND_INDEX || - r->rd_rel->relkind == RELKIND_PARTITIONED_INDEX || - r->rd_rel->relkind == RELKIND_COMPOSITE_TYPE) + if (r->rd_rel->relkind != RELKIND_RELATION || + r->rd_rel->relkind != RELKIND_SEQUENCE || + r->rd_rel->relkind != RELKIND_TOASTVALUE || + r->rd_rel->relkind != RELKIND_VIEW || + r->rd_rel->relkind != RELKIND_MATVIEW || + r->rd_rel->relkind != RELKIND_FOREIGN_TABLE || + r->rd_rel->relkind != RELKIND_PARTITIONED_TABLE) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), errmsg("cannot open relation \"%s\"", -- 2.53.0 ^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2026-02-18 15:15 UTC | newest] Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-12-03 07:54 [PATCH v3] Add a new COLLATION option to REINDEX. Julien Rouhaud <[email protected]> 2026-02-18 15:15 some validate_relation_kind() tidying Peter Eisentraut <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox