From b9d80e5eff355e930a173c6cf8b03e3a0d623c57 Mon Sep 17 00:00:00 2001 From: Julien Rouhaud Date: Wed, 11 Dec 2019 13:54:32 +0100 Subject: [PATCH 6/6] Add a new ALTER INDEX name ALTER COLLATION name REFRESH VERSION This command allows privileged users to specifify that the currently installed collation library version, for a specific collation, is binary compatible with the one that was installed when the specified index was built for the last time. Author: Julien Rouhaud Reviewed-by: Discussion: https://postgr.es/m/CAEepm%3D0uEQCpfq_%2BLYFBdArCe4Ot98t1aR4eYiYTe%3DyavQygiQ%40mail.gmail.com --- doc/src/sgml/ref/alter_index.sgml | 13 +++ src/backend/catalog/index.c | 2 +- src/backend/commands/tablecmds.c | 90 +++++++++---------- src/backend/nodes/copyfuncs.c | 1 + src/backend/parser/gram.y | 8 ++ src/bin/psql/tab-complete.c | 26 +++++- src/include/catalog/index.h | 3 + src/include/nodes/parsenodes.h | 4 +- .../regress/expected/collate.icu.utf8.out | 17 ++++ .../regress/expected/collate.icu.utf8_2.out | 17 ++++ src/test/regress/sql/collate.icu.utf8.sql | 9 ++ 11 files changed, 139 insertions(+), 51 deletions(-) diff --git a/doc/src/sgml/ref/alter_index.sgml b/doc/src/sgml/ref/alter_index.sgml index 6d34dbb74e..8661b031e9 100644 --- a/doc/src/sgml/ref/alter_index.sgml +++ b/doc/src/sgml/ref/alter_index.sgml @@ -25,6 +25,7 @@ ALTER INDEX [ IF EXISTS ] name RENA ALTER INDEX [ IF EXISTS ] name SET TABLESPACE tablespace_name ALTER INDEX name ATTACH PARTITION index_name ALTER INDEX name DEPENDS ON EXTENSION extension_name +ALTER INDEX name ALTER COLLATION collation_name REFRESH VERSION ALTER INDEX [ IF EXISTS ] name SET ( storage_parameter = value [, ... ] ) ALTER INDEX [ IF EXISTS ] name RESET ( storage_parameter [, ... ] ) ALTER INDEX [ IF EXISTS ] name ALTER [ COLUMN ] column_number @@ -109,6 +110,18 @@ ALTER INDEX ALL IN TABLESPACE name + + ALTER COLLATION + + + This form update the index existing dependency on a specific collation, + to specificy the the currently installed collation version is compatible + with the version used the last time the index was built. Be aware that + an incorrect use of this form can hide a corruption on the index. + + + + SET ( storage_parameter = value [, ... ] ) diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index b595c90ad0..a92977fef7 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -3097,7 +3097,7 @@ index_build(Relation heapRelation, SetUserIdAndSecContext(save_userid, save_sec_context); } -static char * +char * index_force_collation_version(const ObjectAddress *otherObject, const char *version, void *userdata) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index facda420f2..feaea2024e 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -93,6 +93,7 @@ #include "utils/lsyscache.h" #include "utils/memutils.h" #include "utils/partcache.h" +#include "utils/pg_locale.h" #include "utils/relcache.h" #include "utils/ruleutils.h" #include "utils/snapmgr.h" @@ -553,8 +554,7 @@ static void refuseDupeIndexAttach(Relation parentIdx, Relation partIdx, Relation partitionTbl); static List *GetParentedForeignKeyRefs(Relation partition); static void ATDetachCheckNoForeignKeyRefs(Relation partition); -//static void ATExecDependsOnCollationVersion(Relation rel, List *coll, -// char *version); +static void ATExecAlterCollationRefreshVersion(Relation rel, List *coll); /* ---------------------------------------------------------------- @@ -3873,10 +3873,9 @@ AlterTableGetLockLevel(List *cmds) cmd_lockmode = AccessShareLock; break; - /* Only used in binary upgrade mode */ - //case AT_DependsOnCollationVersion: - // cmd_lockmode = AccessExclusiveLock; - // break; + case AT_AlterCollationRefreshVersion: + cmd_lockmode = AccessExclusiveLock; + break; default: /* oops */ elog(ERROR, "unrecognized alter table type: %d", @@ -4045,16 +4044,12 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, /* This command never recurses */ pass = AT_PASS_MISC; break; - //case AT_DependsOnCollationVersion: /* DEPENDS ON COLLATION ... - // * [UNKNOWN VERSION | VERSION ...] */ - // if (!IsBinaryUpgrade) - // ereport(ERROR, - // (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM), - // (errmsg("command can only be called when server is in binary upgrade mode")))); - // ATSimplePermissions(rel, ATT_INDEX); - // /* This command never recurses */ - // pass = AT_PASS_MISC; - // break; + case AT_AlterCollationRefreshVersion: /* ALTER COLLATION ... + * REFRESH VERSION */ + ATSimplePermissions(rel, ATT_INDEX); + /* This command never recurses */ + pass = AT_PASS_MISC; + break; case AT_SetStorage: /* ALTER COLUMN SET STORAGE */ ATSimplePermissions(rel, ATT_TABLE | ATT_MATVIEW | ATT_FOREIGN_TABLE); ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode, context); @@ -4621,11 +4616,11 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel, Assert(rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE); ATExecDetachPartition(rel, ((PartitionCmd *) cmd->def)->name); break; - //case AT_DependsOnCollationVersion: - // /* ATPrepCmd ensured it must be an index */ - // Assert(rel->rd_rel->relkind == RELKIND_INDEX); - // ATExecDependsOnCollationVersion(rel, cmd->object, cmd->version); - // break; + case AT_AlterCollationRefreshVersion: + /* ATPrepCmd ensured it must be an index */ + Assert(rel->rd_rel->relkind == RELKIND_INDEX); + ATExecAlterCollationRefreshVersion(rel, cmd->object); + break; default: /* oops */ elog(ERROR, "unrecognized alter table type: %d", (int) cmd->subtype); @@ -17292,32 +17287,31 @@ ATDetachCheckNoForeignKeyRefs(Relation partition) } } -/* Execute an ALTER INDEX ... ALTER COLLATION DEPENDS ON ... +/* Execute an ALTER INDEX ... ALTER COLLATION ... REFRESH VERSION * - * A version has to be provided. If the caller wants to notify that the - * collation version to depend on is unknown, an empty string is passed. + * This override an existing dependency on a specific collation for a specific + * index to depend on the current collation version. */ -//static void -//ATExecDependsOnCollationVersion(Relation rel, List *coll, char *version) -//{ -// ObjectAddress object; -// NewCollationVersionDependency forced_dependency; -// -// Assert(version); -// -// if (coll == NIL) -// forced_dependency.oid = InvalidOid; -// else -// forced_dependency.oid = get_collation_oid(coll, false); -// -// forced_dependency.version = version; -// -// object.classId = RelationRelationId; -// object.objectId = rel->rd_id; -// object.objectSubId = 0; -// visitDependentObjects(&object, &index_force_collation_version, -// &forced_dependency); -// -// /* Invalidate the index relcache */ -// CacheInvalidateRelcache(rel); -//} +static void +ATExecAlterCollationRefreshVersion(Relation rel, List *coll) +{ + ObjectAddress object; + NewCollationVersionDependency forced_dependency; + + Assert(coll != NIL); + forced_dependency.oid = get_collation_oid(coll, false); + + /* Retrieve the current version for the CURRENT VERSION case. */ + Assert(OidIsValid(forced_dependency.oid)); + forced_dependency.version = + get_collation_version_for_oid(forced_dependency.oid); + + object.classId = RelationRelationId; + object.objectId = rel->rd_id; + object.objectSubId = 0; + visitDependentObjects(&object, &index_force_collation_version, + &forced_dependency); + + /* Invalidate the index relcache */ + CacheInvalidateRelcache(rel); +} diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index bf4f793ba2..57c87ded58 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -3174,6 +3174,7 @@ _copyAlterTableCmd(const AlterTableCmd *from) COPY_SCALAR_FIELD(subtype); COPY_STRING_FIELD(name); + COPY_NODE_FIELD(object); COPY_SCALAR_FIELD(num); COPY_NODE_FIELD(newowner); COPY_NODE_FIELD(def); diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index b6d6a0e239..886d065a8e 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -2569,6 +2569,14 @@ alter_table_cmd: n->subtype = AT_NoForceRowSecurity; $$ = (Node *)n; } + /* ALTER INDEX ALTER COLLATION ... REFRESH VERSION */ + | ALTER COLLATION any_name REFRESH VERSION_P + { + AlterTableCmd *n = makeNode(AlterTableCmd); + n->subtype = AT_AlterCollationRefreshVersion; + n->object = $3; + $$ = (Node *)n; + } | alter_generic_options { AlterTableCmd *n = makeNode(AlterTableCmd); diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index dc03fbde13..4cc1646763 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -45,6 +45,7 @@ #include "catalog/pg_am_d.h" #include "catalog/pg_class_d.h" +#include "catalog/pg_collation_d.h" #include "common.h" #include "libpq-fe.h" #include "pqexpbuffer.h" @@ -807,6 +808,20 @@ static const SchemaQuery Query_for_list_of_statistics = { " (SELECT tgrelid FROM pg_catalog.pg_trigger "\ " WHERE pg_catalog.quote_ident(tgname)='%s')" +/* the silly-looking length condition is just to eat up the current word */ +#define Query_for_list_of_colls_for_one_index \ +" SELECT DISTINCT pg_catalog.quote_ident(coll.collname) " \ +" FROM pg_catalog.pg_depend d, pg_catalog.pg_collation coll, " \ +" pg_catalog.pg_class c" \ +" WHERE (%d = pg_catalog.length('%s'))" \ +" AND d.refclassid = " CppAsString2(CollationRelationId) \ +" AND d.refobjid = coll.oid " \ +" AND d.classid = " CppAsString2(RelationRelationId) \ +" AND d.objid = c.oid " \ +" AND c.relkind = " CppAsString2(RELKIND_INDEX) \ +" AND pg_catalog.pg_table_is_visible(c.oid) " \ +" AND c.relname = '%s'" + #define Query_for_list_of_ts_configurations \ "SELECT pg_catalog.quote_ident(cfgname) FROM pg_catalog.pg_ts_config "\ " WHERE substring(pg_catalog.quote_ident(cfgname),1,%d)='%s'" @@ -1697,7 +1712,7 @@ psql_completion(const char *text, int start, int end) /* ALTER INDEX */ else if (Matches("ALTER", "INDEX", MatchAny)) COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME TO", "SET", - "RESET", "ATTACH PARTITION"); + "RESET", "ATTACH PARTITION", "ALTER COLLATION"); else if (Matches("ALTER", "INDEX", MatchAny, "ATTACH")) COMPLETE_WITH("PARTITION"); else if (Matches("ALTER", "INDEX", MatchAny, "ATTACH", "PARTITION")) @@ -1743,6 +1758,15 @@ psql_completion(const char *text, int start, int end) "buffering =", /* GiST */ "pages_per_range =", "autosummarize =" /* BRIN */ ); + /* ALTER INDEX ALTER COLLATION */ + else if (Matches("ALTER", "INDEX", MatchAny, "ALTER", "COLLATION")) + { + completion_info_charp = prev4_wd; + COMPLETE_WITH_QUERY(Query_for_list_of_colls_for_one_index); + } + /* ALTER INDEX ALTER COLLATION */ + else if (Matches("ALTER", "INDEX", MatchAny, "ALTER", "COLLATION", MatchAny)) + COMPLETE_WITH("REFRESH VERSION"); /* ALTER LANGUAGE */ else if (Matches("ALTER", "LANGUAGE", MatchAny)) diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h index 69d163f3cc..065488374e 100644 --- a/src/include/catalog/index.h +++ b/src/include/catalog/index.h @@ -123,6 +123,9 @@ extern void FormIndexDatum(IndexInfo *indexInfo, extern void index_check_collation_versions(Oid relid); +extern char *index_force_collation_version(const ObjectAddress *otherObject, + const char *version, + void *userdata); extern void index_force_collation_versions(Oid indexid, Oid coll, char *version); diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index c96d027362..9f637597a7 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -1844,7 +1844,8 @@ typedef enum AlterTableType AT_DetachPartition, /* DETACH PARTITION */ AT_AddIdentity, /* ADD IDENTITY */ AT_SetIdentity, /* SET identity column options */ - AT_DropIdentity /* DROP IDENTITY */ + AT_DropIdentity, /* DROP IDENTITY */ + AT_AlterCollationRefreshVersion /* ALTER COLLATION ... REFRESH VERSION */ } AlterTableType; typedef struct ReplicaIdentityStmt @@ -1860,6 +1861,7 @@ typedef struct AlterTableCmd /* one subcommand of an ALTER TABLE */ AlterTableType subtype; /* Type of table alteration to apply */ char *name; /* column, constraint, or trigger to act on, * or tablespace */ + List *object; /* collation to act on if it's a collation */ int16 num; /* attribute number for columns referenced by * number */ RoleSpec *newowner; diff --git a/src/test/regress/expected/collate.icu.utf8.out b/src/test/regress/expected/collate.icu.utf8.out index f049189897..2902160688 100644 --- a/src/test/regress/expected/collate.icu.utf8.out +++ b/src/test/regress/expected/collate.icu.utf8.out @@ -2015,6 +2015,23 @@ SELECT objid::regclass FROM pg_depend WHERE refobjversion = 'not a version'; ------- (0 rows) +-- Test ALTER INDEX name ALTER COLLATION name REFRESH VERSION +UPDATE pg_depend SET refobjversion = 'not a version' +WHERE refclassid = 'pg_collation'::regclass +AND objid::regclass::text = 'icuidx17_part' +AND refobjversion IS NOT NULL; +SELECT objid::regclass FROM pg_depend WHERE refobjversion = 'not a version'; + objid +--------------- + icuidx17_part +(1 row) + +ALTER INDEX icuidx17_part ALTER COLLATION "en-x-icu" REFRESH VERSION; +SELECT objid::regclass FROM pg_depend WHERE refobjversion = 'not a version'; + objid +------- +(0 rows) + -- cleanup RESET search_path; SET client_min_messages TO warning; diff --git a/src/test/regress/expected/collate.icu.utf8_2.out b/src/test/regress/expected/collate.icu.utf8_2.out index 07e76c4f48..33a56af104 100644 --- a/src/test/regress/expected/collate.icu.utf8_2.out +++ b/src/test/regress/expected/collate.icu.utf8_2.out @@ -2015,6 +2015,23 @@ SELECT objid::regclass FROM pg_depend WHERE refobjversion = 'not a version'; ------- (0 rows) +-- Test ALTER INDEX name ALTER COLLATION name REFRESH VERSION +UPDATE pg_depend SET refobjversion = 'not a version' +WHERE refclassid = 'pg_collation'::regclass +AND objid::regclass::text = 'icuidx17_part' +AND refobjversion IS NOT NULL; +SELECT objid::regclass FROM pg_depend WHERE refobjversion = 'not a version'; + objid +--------------- + icuidx17_part +(1 row) + +ALTER INDEX icuidx17_part ALTER COLLATION "en-x-icu" REFRESH VERSION; +SELECT objid::regclass FROM pg_depend WHERE refobjversion = 'not a version'; + objid +------- +(0 rows) + -- cleanup RESET search_path; SET client_min_messages TO warning; diff --git a/src/test/regress/sql/collate.icu.utf8.sql b/src/test/regress/sql/collate.icu.utf8.sql index 920a73512f..10329e3498 100644 --- a/src/test/regress/sql/collate.icu.utf8.sql +++ b/src/test/regress/sql/collate.icu.utf8.sql @@ -793,6 +793,15 @@ REINDEX TABLE collate_part_1; SELECT objid::regclass FROM pg_depend WHERE refobjversion = 'not a version'; +-- Test ALTER INDEX name ALTER COLLATION name REFRESH VERSION +UPDATE pg_depend SET refobjversion = 'not a version' +WHERE refclassid = 'pg_collation'::regclass +AND objid::regclass::text = 'icuidx17_part' +AND refobjversion IS NOT NULL; +SELECT objid::regclass FROM pg_depend WHERE refobjversion = 'not a version'; +ALTER INDEX icuidx17_part ALTER COLLATION "en-x-icu" REFRESH VERSION; +SELECT objid::regclass FROM pg_depend WHERE refobjversion = 'not a version'; + -- cleanup RESET search_path; SET client_min_messages TO warning; -- 2.20.1