From 0986575dfcc68dec2f5b76cb8acffa4bcbf1d7dd Mon Sep 17 00:00:00 2001 From: Julien Rouhaud Date: Thu, 7 Nov 2019 07:33:20 +0100 Subject: [PATCH v17 5/7] 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 collations. pg_dump will call this function to preserve the version information for all indexes, if run with the --binary-upgrade option. When pg_upgrade is used to upgrade from a version of PostgreSQL that didn't support per-object versions, the version will be recorded as unknown. If you believe that the collation definitions haven't changed since indexes were last built, you can use --collation-binary-compatible to tell pg_upgrade to assume that the current versions are compatible with the existing indexes, and it will pass --unknown-collations-binary-compatible through to pg_dump and mark them as having the current version (according to the collation provider). Author: Julien Rouhaud Reviewed-by: Peter Eisentraut, Thomas Munro 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/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 | 186 +++++++++++++++- 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 +- 15 files changed, 509 insertions(+), 69 deletions(-) diff --git a/doc/src/sgml/ref/pgupgrade.sgml b/doc/src/sgml/ref/pgupgrade.sgml index 5e396f41fb..7d4dbffe5e 100644 --- a/doc/src/sgml/ref/pgupgrade.sgml +++ b/doc/src/sgml/ref/pgupgrade.sgml @@ -212,6 +212,24 @@ + + + + + After upgrading from a release of PostgreSQL + older than 13, warnings may be reported when collation-dependent + indexes are first accessed. This is because + pg_upgrade records the collation versions + as unknown, so PostgreSQL considers the + indexes to be potentially corrupted. If you're certain that the + collation definitions used by your operating system or ICU haven't + changed since all indexes were created, you can use this flag + to mark all indexes as using the currently installed version. + Otherwise, see . + + + + diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 11a998d3d4..ad682f3395 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -3159,6 +3159,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/utils/adt/pg_upgrade_support.c b/src/backend/utils/adt/pg_upgrade_support.c index 18f2ee8226..07ff39f5aa 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 2c6b1bca62..e012258470 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" @@ -285,6 +286,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); @@ -385,6 +388,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} }; @@ -704,6 +708,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); @@ -6907,7 +6915,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++) @@ -6943,7 +6953,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, " @@ -6968,7 +7033,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) " @@ -7007,7 +7074,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 " @@ -7042,7 +7111,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 " @@ -7073,7 +7144,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 " @@ -7107,7 +7180,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 " @@ -7147,6 +7222,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)); @@ -7172,6 +7249,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); @@ -16456,7 +16535,8 @@ 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 + * 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. @@ -16523,6 +16603,10 @@ dumpIndex(Archive *fout, IndxInfo *indxinfo) "pg_catalog.pg_class", "INDEX", qqindxname); + 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) { @@ -16551,6 +16635,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) @@ -18527,6 +18626,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 if 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 e0c6444ef6..e3369451a7 100644 --- a/src/bin/pg_dump/pg_dump.h +++ b/src/bin/pg_dump/pg_dump.h @@ -365,6 +365,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 1b90cbd9b5..74c457f1e5 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, @@ -916,9 +935,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, }, }, @@ -1180,6 +1200,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, @@ -1205,6 +1226,7 @@ my %tests = ( }, unlike => { binary_upgrade => 1, + binary_coll_compatible => 1, exclude_dump_test_schema => 1, schema_only => 1, }, @@ -1240,6 +1262,7 @@ my %tests = ( }, unlike => { binary_upgrade => 1, + binary_coll_compatible => 1, exclude_dump_test_schema => 1, schema_only => 1, }, @@ -1262,6 +1285,7 @@ my %tests = ( }, unlike => { binary_upgrade => 1, + binary_coll_compatible => 1, exclude_dump_test_schema => 1, schema_only => 1, }, @@ -1283,6 +1307,7 @@ my %tests = ( }, unlike => { binary_upgrade => 1, + binary_coll_compatible => 1, exclude_dump_test_schema => 1, schema_only => 1, }, @@ -1304,6 +1329,7 @@ my %tests = ( }, unlike => { binary_upgrade => 1, + binary_coll_compatible => 1, exclude_dump_test_schema => 1, schema_only => 1, }, @@ -1669,6 +1695,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, }, }, @@ -1683,7 +1710,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' => { @@ -2351,6 +2378,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, }, }, @@ -2544,6 +2572,7 @@ my %tests = ( }, unlike => { binary_upgrade => 1, + binary_coll_compatible => 1, exclude_dump_test_schema => 1, }, }, @@ -2611,6 +2640,7 @@ my %tests = ( /xm, like => { binary_upgrade => 1, + binary_coll_compatible => 1, clean => 1, clean_if_exists => 1, createdb => 1, @@ -2682,6 +2712,7 @@ my %tests = ( /xm, like => { binary_upgrade => 1, + binary_coll_compatible => 1, clean => 1, clean_if_exists => 1, createdb => 1, @@ -3149,6 +3180,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, }, @@ -3164,6 +3196,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, }, @@ -3296,16 +3329,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. @@ -3329,6 +3399,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; @@ -3379,16 +3453,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'); } } } @@ -3436,6 +3523,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} . ";"; } @@ -3489,79 +3582,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 4ef2036ecd..be2d137376 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 8b90cefbe0..68e637ce8a 100644 --- a/src/bin/pg_upgrade/pg_upgrade.h +++ b/src/bin/pg_upgrade/pg_upgrade.h @@ -292,6 +292,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 e39e362524..663bc11c33 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 0899a203c0..9753b0cde2 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 c7c25e1eae..e403107946 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -10194,6 +10194,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