From b33dc0533f2285b6e86766b2dc7c1b836b026337 Mon Sep 17 00:00:00 2001 From: Julien Rouhaud Date: Thu, 7 Nov 2019 07:33:20 +0100 Subject: [PATCH 5/6] Preserve index dependencies on collation during pg_upgrade A new binary_upgrade_set_index_coll_version() SQL function is added to override the recorded dependency version for all or a single collation, for a specific index. Also teach pg_dump to call this function for all indexes, including indexes created for constraints, when run with --binary-upgrade flag. When pg_upgrade is run against an older version, collation versions are not known and pg_dump will by default emit an alter index command to mark all collation versions as unkown. However, it's possible that pg_upgrade is run without upgrading the underlying collation libraries, so a new option --collation-binary-compatible is added to avoid this behavior. This will result in running pg_dump with a new --unknown-collations-binary-compatible option, that can only be used in binary upgrade mode, to prevent pg_dump from emitting call to this new function if the dependent collation version is unknown. Note that if the collation version is known, this flag won't change the behavior and the previous collation version will be preserved. Author: Julien Rouhaud Reviewed-by: Discussion: https://postgr.es/m/CAEepm%3D0uEQCpfq_%2BLYFBdArCe4Ot98t1aR4eYiYTe%3DyavQygiQ%40mail.gmail.com --- doc/src/sgml/ref/pgupgrade.sgml | 18 ++ src/backend/catalog/index.c | 62 ++++++ src/backend/commands/tablecmds.c | 52 +++++ src/backend/utils/adt/pg_upgrade_support.c | 25 +++ src/bin/pg_dump/Makefile | 2 + src/bin/pg_dump/pg_backup.h | 1 + src/bin/pg_dump/pg_dump.c | 192 +++++++++++++++- src/bin/pg_dump/pg_dump.h | 3 + src/bin/pg_dump/t/002_pg_dump.pl | 248 ++++++++++++++++----- src/bin/pg_upgrade/dump.c | 4 +- src/bin/pg_upgrade/option.c | 7 + src/bin/pg_upgrade/pg_upgrade.h | 2 + src/include/catalog/dependency.h | 7 + src/include/catalog/index.h | 3 + src/include/catalog/pg_proc.dat | 4 + src/test/perl/PostgresNode.pm | 6 +- 16 files changed, 564 insertions(+), 72 deletions(-) diff --git a/doc/src/sgml/ref/pgupgrade.sgml b/doc/src/sgml/ref/pgupgrade.sgml index 6629d736b8..b62a1d4eef 100644 --- a/doc/src/sgml/ref/pgupgrade.sgml +++ b/doc/src/sgml/ref/pgupgrade.sgml @@ -212,6 +212,24 @@ + + + + + When upgrading from a PostgreSQL major version 12 or older, or from a + PostgreSQL major version that didn't have support for collation + versioning to a major version that now supports it, all indexes will be + marked as depending on an unknown collation version, as such versions + weren't tracked. As a result, numerous warning messages will be + emitted as it can be a sign of a corrupted index. If you're not + upgrading the collation libraries, and if you're absolutly certain that + all existing indexes are compatible with the current collation + libraries, you can use this flag to change this behavior and mark all + indexes as depending on current collation libraries version. + + + + diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index cd5c93d4ef..b595c90ad0 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -3097,6 +3097,68 @@ index_build(Relation heapRelation, SetUserIdAndSecContext(save_userid, save_sec_context); } +static char * +index_force_collation_version(const ObjectAddress *otherObject, + const char *version, + void *userdata) +{ + NewCollationVersionDependency *forced_dependency; + + forced_dependency = (NewCollationVersionDependency *) userdata; + + /* We only care about dependencies on collations. */ + if (otherObject->classId != CollationRelationId) + return NULL; + + /* + * We only care about dependencies on a specific collation if a valid Oid + * was given.= + */ + if (OidIsValid(forced_dependency->oid) && + otherObject->objectId != forced_dependency->oid) + return NULL; + + return forced_dependency->version; +} + +/* index_force_collation_versions + * + * Override collation version dependencies for the given index. If no + * collation identifier is specified, all dependent collation should be + * reset to an unknown version dependency, and no version should be provided + * either. + */ +void +index_force_collation_versions(Oid indexid, Oid coll, char *version) +{ + Relation index; + ObjectAddress object; + NewCollationVersionDependency forced_dependency; + + Assert(version); + + index = relation_open(indexid, AccessExclusiveLock); + + if (index->rd_rel->relkind != RELKIND_INDEX) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + (errmsg("\"%s\" is not an index", get_rel_name(indexid))))); + + forced_dependency.oid = InvalidOid; + forced_dependency.version = version; + + object.classId = RelationRelationId; + object.objectId = indexid; + object.objectSubId = 0; + visitDependentObjects(&object, &index_force_collation_version, + &forced_dependency); + + /* Invalidate the index relcache */ + CacheInvalidateRelcache(index); + + relation_close(index, NoLock); +} + /* * IndexCheckExclusion - verify that a new exclusion constraint is satisfied * diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index b7c8d663fc..facda420f2 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -553,6 +553,8 @@ 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); /* ---------------------------------------------------------------- @@ -3871,6 +3873,11 @@ AlterTableGetLockLevel(List *cmds) cmd_lockmode = AccessShareLock; break; + /* Only used in binary upgrade mode */ + //case AT_DependsOnCollationVersion: + // cmd_lockmode = AccessExclusiveLock; + // break; + default: /* oops */ elog(ERROR, "unrecognized alter table type: %d", (int) cmd->subtype); @@ -4038,6 +4045,16 @@ 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_SetStorage: /* ALTER COLUMN SET STORAGE */ ATSimplePermissions(rel, ATT_TABLE | ATT_MATVIEW | ATT_FOREIGN_TABLE); ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode, context); @@ -4604,6 +4621,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; default: /* oops */ elog(ERROR, "unrecognized alter table type: %d", (int) cmd->subtype); @@ -17269,3 +17291,33 @@ ATDetachCheckNoForeignKeyRefs(Relation partition) table_close(rel, NoLock); } } + +/* Execute an ALTER INDEX ... ALTER COLLATION DEPENDS ON ... + * + * 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. + */ +//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); +//} diff --git a/src/backend/utils/adt/pg_upgrade_support.c b/src/backend/utils/adt/pg_upgrade_support.c index 0d9e55cdcf..ea7c25ee35 100644 --- a/src/backend/utils/adt/pg_upgrade_support.c +++ b/src/backend/utils/adt/pg_upgrade_support.c @@ -13,6 +13,7 @@ #include "catalog/binary_upgrade.h" #include "catalog/heap.h" +#include "catalog/index.h" #include "catalog/namespace.h" #include "catalog/pg_type.h" #include "commands/extension.h" @@ -208,3 +209,27 @@ binary_upgrade_set_missing_value(PG_FUNCTION_ARGS) PG_RETURN_VOID(); } + +Datum +binary_upgrade_set_index_coll_version(PG_FUNCTION_ARGS) +{ + Oid relid; + Oid coll; + char *version; + + CHECK_IS_BINARY_UPGRADE; + + relid = PG_GETARG_OID(0); + + /* Detect if a collation is specified */ + if (PG_ARGISNULL(1)) + coll = InvalidOid; + else + coll = PG_GETARG_OID(1); + + version = TextDatumGetCString(PG_GETARG_TEXT_PP(2)); + + index_force_collation_versions(relid, coll, version); + + PG_RETURN_VOID(); +} diff --git a/src/bin/pg_dump/Makefile b/src/bin/pg_dump/Makefile index 2532d9183a..c58ebaa681 100644 --- a/src/bin/pg_dump/Makefile +++ b/src/bin/pg_dump/Makefile @@ -16,6 +16,8 @@ subdir = src/bin/pg_dump top_builddir = ../../.. include $(top_builddir)/src/Makefile.global +export with_icu + override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS) LDFLAGS_INTERNAL += -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport) diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h index 8c0cedcd98..dc82b076ef 100644 --- a/src/bin/pg_dump/pg_backup.h +++ b/src/bin/pg_dump/pg_backup.h @@ -171,6 +171,7 @@ typedef struct _dumpOptions int sequence_data; /* dump sequence data even in schema-only mode */ int do_nothing; + int unknown_coll_compat; } DumpOptions; /* diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 9aa6496814..a09a0cf53d 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -46,6 +46,7 @@ #include "catalog/pg_attribute_d.h" #include "catalog/pg_cast_d.h" #include "catalog/pg_class_d.h" +#include "catalog/pg_collation_d.h" #include "catalog/pg_default_acl_d.h" #include "catalog/pg_largeobject_d.h" #include "catalog/pg_largeobject_metadata_d.h" @@ -288,6 +289,8 @@ static void binary_upgrade_extension_member(PQExpBuffer upgrade_buffer, static const char *getAttrName(int attrnum, TableInfo *tblInfo); static const char *fmtCopyColumnList(const TableInfo *ti, PQExpBuffer buffer); static bool nonemptyReloptions(const char *reloptions); +static void appendIndexCollationVersion(PQExpBuffer buffer, IndxInfo *indxinfo, + int enc, int unknown_coll_compat); static void appendReloptionsArrayAH(PQExpBuffer buffer, const char *reloptions, const char *prefix, Archive *fout); static char *get_synchronized_snapshot(Archive *fout); @@ -388,6 +391,7 @@ main(int argc, char **argv) {"no-sync", no_argument, NULL, 7}, {"on-conflict-do-nothing", no_argument, &dopt.do_nothing, 1}, {"rows-per-insert", required_argument, NULL, 10}, + {"unknown-collations-binary-compatible", no_argument, &dopt.unknown_coll_compat, 1}, {NULL, 0, NULL, 0} }; @@ -708,6 +712,10 @@ main(int argc, char **argv) if (archiveFormat != archDirectory && numWorkers > 1) fatal("parallel backup only supported by the directory format"); + /* Unknown collation versions can only be ignored in binary upgrade mode */ + if (dopt.unknown_coll_compat && !dopt.binary_upgrade) + fatal("option --unknown-collations-binary-compatible only works in binary upgrade mode"); + /* Open the output file */ fout = CreateArchive(filename, archiveFormat, compressLevel, dosync, archiveMode, setupDumpWorker); @@ -6841,7 +6849,9 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables) i_tablespace, i_indreloptions, i_indstatcols, - i_indstatvals; + i_indstatvals, + i_inddependoids, + i_inddependversions; int ntups; for (i = 0; i < numTables; i++) @@ -6877,7 +6887,62 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables) * is not. */ resetPQExpBuffer(query); - if (fout->remoteVersion >= 110000) + if (fout->remoteVersion >= 130000) + { + appendPQExpBuffer(query, + "SELECT t.tableoid, t.oid, " + "t.relname AS indexname, " + "inh.inhparent AS parentidx, " + "pg_catalog.pg_get_indexdef(i.indexrelid) AS indexdef, " + "i.indnkeyatts AS indnkeyatts, " + "i.indnatts AS indnatts, " + "i.indkey, i.indisclustered, " + "i.indisreplident, " + "c.contype, c.conname, " + "c.condeferrable, c.condeferred, " + "c.tableoid AS contableoid, " + "c.oid AS conoid, " + "pg_catalog.pg_get_constraintdef(c.oid, false) AS condef, " + "(SELECT spcname FROM pg_catalog.pg_tablespace s WHERE s.oid = t.reltablespace) AS tablespace, " + "t.reloptions AS indreloptions, " + "(SELECT pg_catalog.array_agg(attnum ORDER BY attnum) " + " FROM pg_catalog.pg_attribute " + " WHERE attrelid = i.indexrelid AND " + " attstattarget >= 0) AS indstatcols," + "(SELECT pg_catalog.array_agg(attstattarget ORDER BY attnum) " + " FROM pg_catalog.pg_attribute " + " WHERE attrelid = i.indexrelid AND " + " attstattarget >= 0) AS indstatvals, " + "(SELECT pg_catalog.array_agg(refobjid ORDER BY refobjid) " + " FROM pg_catalog.pg_depend " + " WHERE classid = " CppAsString2(RelationRelationId) " AND " + " objid = i.indexrelid AND " + " objsubid = 0 AND " + " refclassid = " CppAsString2(CollationRelationId) " AND " + " refobjversion IS NOT NULL) AS inddependoids, " + "(SELECT pg_catalog.array_agg(quote_literal(refobjversion) ORDER BY refobjid) " + " FROM pg_catalog.pg_depend " + " WHERE classid = " CppAsString2(RelationRelationId) " AND " + " objid = i.indexrelid AND " + " objsubid = 0 AND " + " refclassid = " CppAsString2(CollationRelationId) " AND " + " refobjversion IS NOT NULL) AS inddependversions " + "FROM pg_catalog.pg_index i " + "JOIN pg_catalog.pg_class t ON (t.oid = i.indexrelid) " + "JOIN pg_catalog.pg_class t2 ON (t2.oid = i.indrelid) " + "LEFT JOIN pg_catalog.pg_constraint c " + "ON (i.indrelid = c.conrelid AND " + "i.indexrelid = c.conindid AND " + "c.contype IN ('p','u','x')) " + "LEFT JOIN pg_catalog.pg_inherits inh " + "ON (inh.inhrelid = indexrelid) " + "WHERE i.indrelid = '%u'::pg_catalog.oid " + "AND (i.indisvalid OR t2.relkind = 'p') " + "AND i.indisready " + "ORDER BY indexname", + tbinfo->dobj.catId.oid); + } + else if (fout->remoteVersion >= 110000) { appendPQExpBuffer(query, "SELECT t.tableoid, t.oid, " @@ -6902,7 +6967,9 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables) "(SELECT pg_catalog.array_agg(attstattarget ORDER BY attnum) " " FROM pg_catalog.pg_attribute " " WHERE attrelid = i.indexrelid AND " - " attstattarget >= 0) AS indstatvals " + " attstattarget >= 0) AS indstatvals, " + "' ' AS inddependoids, " + "' ' AS inddependversions " "FROM pg_catalog.pg_index i " "JOIN pg_catalog.pg_class t ON (t.oid = i.indexrelid) " "JOIN pg_catalog.pg_class t2 ON (t2.oid = i.indrelid) " @@ -6941,7 +7008,9 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables) "(SELECT spcname FROM pg_catalog.pg_tablespace s WHERE s.oid = t.reltablespace) AS tablespace, " "t.reloptions AS indreloptions, " "'' AS indstatcols, " - "'' AS indstatvals " + "'' AS indstatvals, " + "' ' AS inddependoids, " + "' ' AS inddependversions " "FROM pg_catalog.pg_index i " "JOIN pg_catalog.pg_class t ON (t.oid = i.indexrelid) " "LEFT JOIN pg_catalog.pg_constraint c " @@ -6976,7 +7045,9 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables) "(SELECT spcname FROM pg_catalog.pg_tablespace s WHERE s.oid = t.reltablespace) AS tablespace, " "t.reloptions AS indreloptions, " "'' AS indstatcols, " - "'' AS indstatvals " + "'' AS indstatvals, " + "' ' AS inddependoids, " + "' ' AS inddependversions " "FROM pg_catalog.pg_index i " "JOIN pg_catalog.pg_class t ON (t.oid = i.indexrelid) " "LEFT JOIN pg_catalog.pg_constraint c " @@ -7007,7 +7078,9 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables) "(SELECT spcname FROM pg_catalog.pg_tablespace s WHERE s.oid = t.reltablespace) AS tablespace, " "t.reloptions AS indreloptions, " "'' AS indstatcols, " - "'' AS indstatvals " + "'' AS indstatvals, " + "' ' AS inddependoids, " + "' ' AS inddependversions " "FROM pg_catalog.pg_index i " "JOIN pg_catalog.pg_class t ON (t.oid = i.indexrelid) " "LEFT JOIN pg_catalog.pg_depend d " @@ -7041,7 +7114,9 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables) "(SELECT spcname FROM pg_catalog.pg_tablespace s WHERE s.oid = t.reltablespace) AS tablespace, " "null AS indreloptions, " "'' AS indstatcols, " - "'' AS indstatvals " + "'' AS indstatvals, " + "' ' AS inddependoids, " + "' ' AS inddependversions " "FROM pg_catalog.pg_index i " "JOIN pg_catalog.pg_class t ON (t.oid = i.indexrelid) " "LEFT JOIN pg_catalog.pg_depend d " @@ -7081,6 +7156,8 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables) i_indreloptions = PQfnumber(res, "indreloptions"); i_indstatcols = PQfnumber(res, "indstatcols"); i_indstatvals = PQfnumber(res, "indstatvals"); + i_inddependoids = PQfnumber(res, "inddependoids"); + i_inddependversions = PQfnumber(res, "inddependversions"); tbinfo->indexes = indxinfo = (IndxInfo *) pg_malloc(ntups * sizeof(IndxInfo)); @@ -7106,6 +7183,8 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables) indxinfo[j].indreloptions = pg_strdup(PQgetvalue(res, j, i_indreloptions)); indxinfo[j].indstatcols = pg_strdup(PQgetvalue(res, j, i_indstatcols)); indxinfo[j].indstatvals = pg_strdup(PQgetvalue(res, j, i_indstatvals)); + indxinfo[j].inddependoids = pg_strdup(PQgetvalue(res, j, i_inddependoids)); + indxinfo[j].inddependversions = pg_strdup(PQgetvalue(res, j, i_inddependversions)); indxinfo[j].indkeys = (Oid *) pg_malloc(indxinfo[j].indnattrs * sizeof(Oid)); parseOidArray(PQgetvalue(res, j, i_indkey), indxinfo[j].indkeys, indxinfo[j].indnattrs); @@ -16374,10 +16453,11 @@ dumpIndex(Archive *fout, IndxInfo *indxinfo) /* * If there's an associated constraint, don't dump the index per se, but - * do dump any comment for it. (This is safe because dependency ordering - * will have ensured the constraint is emitted first.) Note that the - * emitted comment has to be shown as depending on the constraint, not the - * index, in such cases. + * do dump any comment, or in binary upgrade mode dependency on a collation + * version for it. (This is safe because dependency ordering will have + * ensured the constraint is emitted first.) Note that the emitted + * comment has to be shown as depending on the constraint, not the index, + * in such cases. */ if (!is_constraint) { @@ -16437,6 +16517,10 @@ dumpIndex(Archive *fout, IndxInfo *indxinfo) } } + if (dopt->binary_upgrade) + appendIndexCollationVersion(q, indxinfo, fout->encoding, + dopt->unknown_coll_compat); + /* If the index defines identity, we need to record that. */ if (indxinfo->indisreplident) { @@ -16466,6 +16550,21 @@ dumpIndex(Archive *fout, IndxInfo *indxinfo) if (indstatvalsarray) free(indstatvalsarray); } + else if (dopt->binary_upgrade) + { + appendIndexCollationVersion(q, indxinfo, fout->encoding, + dopt->unknown_coll_compat); + + if (indxinfo->dobj.dump & DUMP_COMPONENT_DEFINITION) + ArchiveEntry(fout, indxinfo->dobj.catId, indxinfo->dobj.dumpId, + ARCHIVE_OPTS(.tag = indxinfo->dobj.name, + .namespace = tbinfo->dobj.namespace->dobj.name, + .tablespace = indxinfo->tablespace, + .owner = tbinfo->rolname, + .description = "INDEX", + .section = SECTION_POST_DATA, + .createStmt = q->data)); + } /* Dump Index Comments */ if (indxinfo->dobj.dump & DUMP_COMPONENT_COMMENT) @@ -18423,6 +18522,77 @@ nonemptyReloptions(const char *reloptions) return (reloptions != NULL && strlen(reloptions) > 2); } +/* + * Format inddependoids and inddependversions arrays and append it to the given + * buffer in the form of binary_upgrade_set_index_coll_version() calls. + */ +static void +appendIndexCollationVersion(PQExpBuffer buffer, IndxInfo *indxinfo, int enc, + int unknown_coll_compat) +{ + char *inddependoids = indxinfo->inddependoids; + char *inddependversions = indxinfo->inddependversions; + char **inddependoidsarray = NULL; + char **inddependversionsarray = NULL; + int ninddependoids; + int ninddependversions; + int i; + + /* + * for older versions that don't record the collation depndency, issue a + * statement to mark the collation version as unknown + */ + if (strcmp(inddependoids, " ") == 0) + { + /* + * do not issue UNKNOWN VERSION is caller specified that those are + * compatible + */ + if (unknown_coll_compat) + return; + + Assert(strcmp(inddependversions, " ") == 0); + + appendPQExpBufferStr(buffer, "\n-- For binary upgrade, restore dependent collation version.\n"); + appendPQExpBuffer(buffer, "SELECT " + "pg_catalog.binary_upgrade_set_index_coll_version(%d, NULL, '');", + indxinfo->dobj.catId.oid); + return; + } + + parsePGArray(inddependoids, &inddependoidsarray, &ninddependoids); + parsePGArray(inddependversions, &inddependversionsarray, &ninddependversions); + + Assert(ninddependoids == ninddependversions); + + for (i = 0; i < ninddependoids; i++) + { + /* + * If there was an unknown version dependency recorded for this + * collation and the caller asked to mark those as depending ono + * current version, don't emit a binary_upgrade_set_index_coll_version + * function call. + */ + if ((strcmp(inddependversionsarray[i], "''")) == 0 + && unknown_coll_compat) + { + continue; + } + + appendPQExpBufferStr(buffer, "\n-- For binary upgrade, restore dependent collation version.\n"); + appendPQExpBuffer(buffer, "SELECT " + "pg_catalog.binary_upgrade_set_index_coll_version(%d, %s, %s);", + indxinfo->dobj.catId.oid, + inddependoidsarray[i], + inddependversionsarray[i]); + } + + if (inddependoidsarray) + free(inddependoidsarray); + if (inddependversionsarray) + free(inddependversionsarray); +} + /* * Format a reloptions array and append it to the given buffer. * diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h index 21004e5078..6d1df24080 100644 --- a/src/bin/pg_dump/pg_dump.h +++ b/src/bin/pg_dump/pg_dump.h @@ -364,6 +364,9 @@ typedef struct _indxInfo int indnattrs; /* total number of index attributes */ Oid *indkeys; /* In spite of the name 'indkeys' this field * contains both key and nonkey attributes */ + char *inddependoids; /* oids of collation this index depends on */ + char *inddependversions; /* version of collation this index depends + * on */ bool indisclustered; bool indisreplident; Oid parentidx; /* if partitioned, parent index OID */ diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl index 4a9764c2d2..216a704bb7 100644 --- a/src/bin/pg_dump/t/002_pg_dump.pl +++ b/src/bin/pg_dump/t/002_pg_dump.pl @@ -53,6 +53,24 @@ my %pgdump_runs = ( "$tempdir/binary_upgrade.dump", ], }, + binary_coll_compatible => { + dump_cmd => [ + 'pg_dump', + '--no-sync', + '--format=custom', + "--file=$tempdir/binary_coll_compatible.dump", + '-w', + '--schema-only', + '--binary-upgrade', + '--unknown-collations-binary-compatible', + '-d', 'postgres', # alternative way to specify database + ], + restore_cmd => [ + 'pg_restore', '-Fc', '--verbose', + "--file=$tempdir/binary_coll_compatible.sql", + "$tempdir/binary_coll_compatible.dump", + ], + }, clean => { dump_cmd => [ 'pg_dump', @@ -387,6 +405,7 @@ my %dump_test_schema_runs = ( # are flags used to exclude specific items (ACLs, blobs, etc). my %full_runs = ( binary_upgrade => 1, + binary_coll_compatible => 1, clean => 1, clean_if_exists => 1, createdb => 1, @@ -914,9 +933,10 @@ my %tests = ( test_schema_plus_blobs => 1, }, unlike => { - binary_upgrade => 1, - no_blobs => 1, - schema_only => 1, + binary_upgrade => 1, + binary_coll_compatible => 1, + no_blobs => 1, + schema_only => 1, }, }, @@ -1178,6 +1198,7 @@ my %tests = ( }, unlike => { binary_upgrade => 1, + binary_coll_compatible => 1, exclude_dump_test_schema => 1, exclude_test_table => 1, exclude_test_table_data => 1, @@ -1203,6 +1224,7 @@ my %tests = ( }, unlike => { binary_upgrade => 1, + binary_coll_compatible => 1, exclude_dump_test_schema => 1, schema_only => 1, }, @@ -1238,6 +1260,7 @@ my %tests = ( }, unlike => { binary_upgrade => 1, + binary_coll_compatible => 1, exclude_dump_test_schema => 1, schema_only => 1, }, @@ -1260,6 +1283,7 @@ my %tests = ( }, unlike => { binary_upgrade => 1, + binary_coll_compatible => 1, exclude_dump_test_schema => 1, schema_only => 1, }, @@ -1281,6 +1305,7 @@ my %tests = ( }, unlike => { binary_upgrade => 1, + binary_coll_compatible => 1, exclude_dump_test_schema => 1, schema_only => 1, }, @@ -1302,6 +1327,7 @@ my %tests = ( }, unlike => { binary_upgrade => 1, + binary_coll_compatible => 1, exclude_dump_test_schema => 1, schema_only => 1, }, @@ -1665,6 +1691,7 @@ my %tests = ( { %full_runs, %dump_test_schema_runs, section_pre_data => 1, }, unlike => { binary_upgrade => 1, + binary_coll_compatible => 1, exclude_dump_test_schema => 1, }, }, @@ -1679,7 +1706,7 @@ my %tests = ( \n.*^ \QALTER TYPE dump_test.planets ADD VALUE 'mars';\E \n/xms, - like => { binary_upgrade => 1, }, + like => { binary_upgrade => 1, binary_coll_compatible => 1, }, }, 'CREATE TYPE dump_test.textrange AS RANGE' => { @@ -2347,6 +2374,7 @@ my %tests = ( { %full_runs, %dump_test_schema_runs, section_pre_data => 1, }, unlike => { binary_upgrade => 1, + binary_coll_compatible => 1, exclude_dump_test_schema => 1, }, }, @@ -2540,6 +2568,7 @@ my %tests = ( }, unlike => { binary_upgrade => 1, + binary_coll_compatible => 1, exclude_dump_test_schema => 1, }, }, @@ -2607,6 +2636,7 @@ my %tests = ( /xm, like => { binary_upgrade => 1, + binary_coll_compatible => 1, clean => 1, clean_if_exists => 1, createdb => 1, @@ -2678,6 +2708,7 @@ my %tests = ( /xm, like => { binary_upgrade => 1, + binary_coll_compatible => 1, clean => 1, clean_if_exists => 1, createdb => 1, @@ -3145,6 +3176,7 @@ my %tests = ( { %full_runs, %dump_test_schema_runs, section_post_data => 1, }, unlike => { binary_upgrade => 1, + binary_coll_compatible => 1, exclude_dump_test_schema => 1, schema_only => 1, }, @@ -3160,6 +3192,7 @@ my %tests = ( { %full_runs, %dump_test_schema_runs, section_post_data => 1, }, unlike => { binary_upgrade => 1, + binary_coll_compatible => 1, exclude_dump_test_schema => 1, schema_only => 1, }, @@ -3292,16 +3325,53 @@ my %tests = ( %full_runs, %dump_test_schema_runs, section_pre_data => 1, }, unlike => { exclude_dump_test_schema => 1 }, + }, + + "binary_upgrade_set_index_coll_version(oid, oid, 'not_a_version')" => { + create_order => 101, + create_sql => ' + CREATE TABLE dump_test.regress_table_coll(id integer, val text); + CREATE INDEX regress_coll_idx1 ON dump_test.regress_table_coll(val COLLATE "fr-x-icu"); + UPDATE pg_depend SET refobjversion = \'not_a_version\' WHERE refobjversion IS NOT NULL AND objid::regclass::text = \'dump_test.regress_coll_idx1\';', + regexp => qr/^ + \QCREATE INDEX regress_coll_idx1 ON dump_test.regress_table_coll USING btree (val COLLATE "fr-x-icu");\E\n + \n + \Q-- For binary upgrade, restore dependent collation version.\E\n + \QSELECT pg_catalog.binary_upgrade_set_index_coll_version\E \(\d+,\ \d+,\ 'not_a_version'\);/xm, + like => { binary_upgrade => 1, binary_coll_compatible => 1, }, + icu => 1, + }, + "binary_upgrade_set_index_coll_version(?, ?, '')" => { + create_order => 102, + create_sql => ' + CREATE TABLE dump_test.regress_table_coll_no_ver(id integer, val text); + CREATE INDEX regress_coll_no_ver_idx1 ON dump_test.regress_table_coll_no_ver(val COLLATE "fr-x-icu"); + UPDATE pg_depend SET refobjversion = \'\' WHERE refobjversion IS NOT NULL AND objid::regclass::text = \'dump_test.regress_coll_no_ver_idx1\';', + regexp => qr/SELECT pg_catalog.binary_upgrade_set_index_coll_version\(\d+, \d+, ''\)/, + like => { binary_upgrade => 1}, + # should not appear in binary_coll_compatible case! + unlike => { binary_coll_compatible => 1}, + icu => 1, }); ######################################### # Create a PG instance to test actually dumping from -my $node = get_new_node('main'); -$node->init; -$node->start; +my $main_node = get_new_node('main'); +$main_node->init; +$main_node->start; + +my $port = $main_node->port; + +# And another instance to validate the binary dump +my $bin_node = get_new_node('binary'); +$bin_node->init; +$bin_node->start; -my $port = $node->port; +my $bin_port = $bin_node->port; + +# and add a $node variable pointing to main_node for now +my $node = $main_node; # We need to see if this system supports CREATE COLLATION or not # If it doesn't then we will skip all the COLLATION-related tests. @@ -3325,6 +3395,10 @@ $node->psql('postgres', 'create database regress_pg_dump_test;'); # command_fails_like is actually 2 tests) my $num_tests = 12; +# 4 more tests for restoring globals and binary_upgrade dump, dumping it again +# and regenerating the sql file +$num_tests+= 4; + foreach my $run (sort keys %pgdump_runs) { my $test_key = $run; @@ -3375,16 +3449,29 @@ foreach my $run (sort keys %pgdump_runs) next; } + # Skip any icu-related commands if there is no icu support + if ($ENV{with_icu} ne 'yes' && defined($tests{$test}->{icu})) + { + next; + } + # If there is a like entry, but no unlike entry, then we will test the like case if ($tests{$test}->{like}->{$test_key} && !defined($tests{$test}->{unlike}->{$test_key})) { $num_tests++; + + # binary_upgrade tests are also run after being restored and + # re-dumped. + $num_tests++ if ($test_key eq 'binary_upgrade'); } else { # We will test everything that isn't a 'like' $num_tests++; + # binary_upgrade tests are also run after being restored and + # re-dumped. + $num_tests++ if ($test_key eq 'binary_upgrade'); } } } @@ -3432,6 +3519,12 @@ foreach my $test ( next; } + # Skip any icu-related commands if there is no icu support + if ($ENV{with_icu} ne 'yes' && defined($tests{$test}->{icu})) + { + next; + } + # Add terminating semicolon $create_sql{$test_db} .= $tests{$test}->{create_sql} . ";"; } @@ -3485,79 +3578,116 @@ command_fails_like( ######################################### # Run all runs -foreach my $run (sort keys %pgdump_runs) +foreach my $pass (1, 2) { - my $test_key = $run; - my $run_db = 'postgres'; - - $node->command_ok(\@{ $pgdump_runs{$run}->{dump_cmd} }, - "$run: pg_dump runs"); - - if ($pgdump_runs{$run}->{restore_cmd}) + foreach my $run (sort keys %pgdump_runs) { - $node->command_ok(\@{ $pgdump_runs{$run}->{restore_cmd} }, - "$run: pg_restore runs"); - } - - if ($pgdump_runs{$run}->{test_key}) - { - $test_key = $pgdump_runs{$run}->{test_key}; - } - - my $output_file = slurp_file("$tempdir/${run}.sql"); + my $test_key = $run; + my $run_db = 'postgres'; - ######################################### - # Run all tests where this run is included - # as either a 'like' or 'unlike' test. + # we only test binary upgrade on the 2nd pass + next if ($pass == 2 and $test_key ne 'binary_upgrade'); - foreach my $test (sort keys %tests) - { - my $test_db = 'postgres'; + $node->command_ok(\@{ $pgdump_runs{$run}->{dump_cmd} }, + "$run: pg_dump runs"); - if (defined($pgdump_runs{$run}->{database})) + if ($pgdump_runs{$run}->{restore_cmd}) { - $run_db = $pgdump_runs{$run}->{database}; + $node->command_ok(\@{ $pgdump_runs{$run}->{restore_cmd} }, + "$run: pg_restore runs"); } - if (defined($tests{$test}->{database})) + if ($pgdump_runs{$run}->{test_key}) { - $test_db = $tests{$test}->{database}; + $test_key = $pgdump_runs{$run}->{test_key}; } - # Skip any collation-related commands if there is no collation support - if (!$collation_support && defined($tests{$test}->{collation})) - { - next; - } + my $output_file = slurp_file("$tempdir/${run}.sql"); - if ($run_db ne $test_db) - { - next; - } + ######################################### + # Run all tests where this run is included + # as either a 'like' or 'unlike' test. - # Run the test listed as a like, unless it is specifically noted - # as an unlike (generally due to an explicit exclusion or similar). - if ($tests{$test}->{like}->{$test_key} - && !defined($tests{$test}->{unlike}->{$test_key})) + foreach my $test (sort keys %tests) { - if (!ok($output_file =~ $tests{$test}->{regexp}, - "$run: should dump $test")) + my $test_db = 'postgres'; + + if (defined($pgdump_runs{$run}->{database})) { - diag("Review $run results in $tempdir"); + $run_db = $pgdump_runs{$run}->{database}; } - } - else - { - if (!ok($output_file !~ $tests{$test}->{regexp}, - "$run: should not dump $test")) + + if (defined($tests{$test}->{database})) + { + $test_db = $tests{$test}->{database}; + } + + # Skip any collation-related commands if there is no collation support + if (!$collation_support && defined($tests{$test}->{collation})) + { + next; + } + + # Skip any icu-related commands if there is no icu support + if ($ENV{with_icu} ne 'yes' && defined($tests{$test}->{icu})) + { + next; + } + + if ($run_db ne $test_db) + { + next; + } + + # Run the test listed as a like, unless it is specifically noted + # as an unlike (generally due to an explicit exclusion or similar). + if ($tests{$test}->{like}->{$test_key} + && !defined($tests{$test}->{unlike}->{$test_key})) { - diag("Review $run results in $tempdir"); + if (!ok($output_file =~ $tests{$test}->{regexp}, + "$run: should dump $test")) + { + diag("Review $run results in $tempdir"); + } + } + else + { + if (!ok($output_file !~ $tests{$test}->{regexp}, + "$run: should not dump $test")) + { + diag("Review $run results in $tempdir"); + } } } } + + # After all dump have been generated, restore the binary_upgrade dump with + # the required global objects on a suitable node, and continue with the 2nd + # pass. + if ($pass == 1) + { + # Stop the original database instance as we don't need it anymore. + $node->stop('fast'); + + $bin_node->command_ok(\@{['psql', + "-d", "postgres", "-f", "$tempdir/pg_dumpall_globals.sql"]}, + "Restore globals"); + + $bin_node->stop('fast'); + $bin_node->start(binary_start => 1); + $bin_node->command_ok(\@{['pg_restore', '-p', $bin_port, + '-d', 'postgres', + "$tempdir/binary_upgrade.dump"]}, + "Restore the binary_upgrade dump"); + $bin_node->stop('fast'); + $bin_node->start; + + # And change $node to point to the freshly restored node. + $node = $bin_node; + } } ######################################### # Stop the database instance, which will be removed at the end of the tests. -$node->stop('fast'); +$bin_node->stop('fast'); diff --git a/src/bin/pg_upgrade/dump.c b/src/bin/pg_upgrade/dump.c index 4d730adfe2..672ecda169 100644 --- a/src/bin/pg_upgrade/dump.c +++ b/src/bin/pg_upgrade/dump.c @@ -52,9 +52,11 @@ generate_old_dump(void) parallel_exec_prog(log_file_name, NULL, "\"%s/pg_dump\" %s --schema-only --quote-all-identifiers " - "--binary-upgrade --format=custom %s --file=\"%s\" %s", + "--binary-upgrade --format=custom %s %s --file=\"%s\" %s", new_cluster.bindir, cluster_conn_opts(&old_cluster), log_opts.verbose ? "--verbose" : "", + user_opts.coll_compat ? + "--unknown-collations-binary-compatible" : "", sql_file_name, escaped_connstr.data); termPQExpBuffer(&escaped_connstr); diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c index eed70fff4f..d36d6a283f 100644 --- a/src/bin/pg_upgrade/option.c +++ b/src/bin/pg_upgrade/option.c @@ -56,6 +56,7 @@ parseCommandLine(int argc, char *argv[]) {"socketdir", required_argument, NULL, 's'}, {"verbose", no_argument, NULL, 'v'}, {"clone", no_argument, NULL, 1}, + {"collation-binary-compatible", no_argument, NULL, 2}, {NULL, 0, NULL, 0} }; @@ -203,6 +204,10 @@ parseCommandLine(int argc, char *argv[]) user_opts.transfer_mode = TRANSFER_MODE_CLONE; break; + case 2: + user_opts.coll_compat = true; + break; + default: fprintf(stderr, _("Try \"%s --help\" for more information.\n"), os_info.progname); @@ -307,6 +312,8 @@ usage(void) printf(_(" -v, --verbose enable verbose internal logging\n")); printf(_(" -V, --version display version information, then exit\n")); printf(_(" --clone clone instead of copying files to new cluster\n")); + printf(_(" --collation-binary-compatible mark collations as depending on current collation\n" + " versions rather than unknown if they're unknown\n")); printf(_(" -?, --help show this help, then exit\n")); printf(_("\n" "Before running pg_upgrade you must:\n" diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h index b156b516cc..af34dff920 100644 --- a/src/bin/pg_upgrade/pg_upgrade.h +++ b/src/bin/pg_upgrade/pg_upgrade.h @@ -294,6 +294,8 @@ typedef struct transferMode transfer_mode; /* copy files or link them? */ int jobs; /* number of processes/threads to use */ char *socketdir; /* directory to use for Unix sockets */ + bool coll_compat; /* should we skip marking index collations as + * unknown version */ } UserOpts; typedef struct diff --git a/src/include/catalog/dependency.h b/src/include/catalog/dependency.h index 6de17e25e8..dc99b49b8e 100644 --- a/src/include/catalog/dependency.h +++ b/src/include/catalog/dependency.h @@ -130,6 +130,13 @@ typedef enum ObjectClass #define LAST_OCLASS OCLASS_TRANSFORM +/* Struct describing one forced collation version dependency */ +typedef struct NewCollationVersionDependency +{ + char *version; /* forced collation version */ + Oid oid; /* target collation oid */ +} NewCollationVersionDependency; + /* flag bits for performDeletion/performMultipleDeletions: */ #define PERFORM_DELETION_INTERNAL 0x0001 /* internal action */ #define PERFORM_DELETION_CONCURRENTLY 0x0002 /* concurrent drop */ diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h index c619d02465..69d163f3cc 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 void index_force_collation_versions(Oid indexid, Oid coll, + char *version); + extern void index_build(Relation heapRelation, Relation indexRelation, IndexInfo *indexInfo, diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index f6503428cc..bca564644c 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -10195,6 +10195,10 @@ proname => 'binary_upgrade_set_missing_value', provolatile => 'v', proparallel => 'u', prorettype => 'void', proargtypes => 'oid text text', prosrc => 'binary_upgrade_set_missing_value' }, +{ oid => '8178', descr => 'for use by pg_upgrade', + proname => 'binary_upgrade_set_index_coll_version', provolatile => 'v', + proparallel => 'u', prorettype => 'void', proargtypes => 'oid oid text', + prosrc => 'binary_upgrade_set_index_coll_version' }, # conversion functions { oid => '4302', diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm index 9575268bd7..bba07a2319 100644 --- a/src/test/perl/PostgresNode.pm +++ b/src/test/perl/PostgresNode.pm @@ -764,10 +764,14 @@ sub start local %ENV = %ENV; delete $ENV{PGAPPNAME}; + my $options = "--cluster-name=$name"; + + $options .= ' -b' if ($params{binary_start}); + # Note: We set the cluster_name here, not in postgresql.conf (in # sub init) so that it does not get copied to standbys. $ret = TestLib::system_log('pg_ctl', '-D', $self->data_dir, '-l', - $self->logfile, '-o', "--cluster-name=$name", 'start'); + $self->logfile, '-o', $options, 'start'); } if ($ret != 0) -- 2.20.1