From c8db7e6b415dc85410f5e240a5ee575bbc373d36 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 Two new commands are added. ALTER INDEX i_name DEPENDS ON COLLATION c_name VERSION v_name to force a dependency on a collation specific version, and ALTER INDEX i_name DEPENDS ON ANY COLLATION UNKOWN VERSION to specify that the version is unknown for all collations on a specific index. Both commands are only allowed in binary upgrade mode. Also teach pg_dump to emit such commands 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 the alter index commands 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/commands/tablecmds.c | 83 +++++++++++ src/backend/nodes/copyfuncs.c | 2 + src/backend/parser/gram.y | 27 ++++ src/bin/pg_dump/Makefile | 2 + src/bin/pg_dump/pg_backup.h | 1 + src/bin/pg_dump/pg_dump.c | 193 ++++++++++++++++++++++-- src/bin/pg_dump/pg_dump.h | 3 + src/bin/pg_dump/t/002_pg_dump.pl | 244 +++++++++++++++++++++++-------- src/bin/pg_upgrade/dump.c | 4 +- src/bin/pg_upgrade/option.c | 7 + src/bin/pg_upgrade/pg_upgrade.h | 2 + src/include/nodes/parsenodes.h | 5 +- src/test/perl/PostgresNode.pm | 6 +- 14 files changed, 524 insertions(+), 73 deletions(-) diff --git a/doc/src/sgml/ref/pgupgrade.sgml b/doc/src/sgml/ref/pgupgrade.sgml index d4da566d76..6c55f3b55a 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/commands/tablecmds.c b/src/backend/commands/tablecmds.c index f599393473..b3cf5641fd 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -207,6 +207,13 @@ typedef struct NewColumnValue bool is_generated; /* is it a GENERATED expression? */ } NewColumnValue; +/* Struct describing one forced collation version dependency */ +typedef struct NewCollationVersionDependency +{ + char *version; /* forced collation version */ + Oid oid; /* target collation oid */ +} NewCollationVersionDependency; + /* * Error-reporting support for RemoveRelations */ @@ -553,6 +560,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 +3880,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 +4052,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 +4628,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); @@ -17263,3 +17292,57 @@ ATDetachCheckNoForeignKeyRefs(Relation partition) table_close(rel, NoLock); } } + +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; +} + +/* 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/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index bf4f793ba2..8b1e8e73c0 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -3174,7 +3174,9 @@ _copyAlterTableCmd(const AlterTableCmd *from) COPY_SCALAR_FIELD(subtype); COPY_STRING_FIELD(name); + COPY_NODE_FIELD(object); COPY_SCALAR_FIELD(num); + COPY_STRING_FIELD(version); COPY_NODE_FIELD(newowner); COPY_NODE_FIELD(def); COPY_SCALAR_FIELD(behavior); diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index b6d6a0e239..1437123540 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -2569,6 +2569,33 @@ alter_table_cmd: n->subtype = AT_NoForceRowSecurity; $$ = (Node *)n; } + /* ALTER INDEX DEPENDS ON COLLATION ... VERSION ... */ + | DEPENDS ON COLLATION any_name VERSION_P Sconst + { + AlterTableCmd *n = makeNode(AlterTableCmd); + n->subtype = AT_DependsOnCollationVersion; + n->object = $4; + n->version = $6; + $$ = (Node *)n; + } + /* ALTER INDEX DEPENDS ON COLLATION ... UNKNOWN VERSION */ + | DEPENDS ON COLLATION any_name UNKNOWN VERSION_P + { + AlterTableCmd *n = makeNode(AlterTableCmd); + n->subtype = AT_DependsOnCollationVersion; + n->object = $4; + n->version = ""; + $$ = (Node *)n; + } + /* ALTER INDEX DEPENDS ON ANY COLLATION UNKNOWN VERSION */ + | DEPENDS ON ANY COLLATION UNKNOWN VERSION_P + { + AlterTableCmd *n = makeNode(AlterTableCmd); + n->subtype = AT_DependsOnCollationVersion; + n->object = NIL; + n->version = ""; + $$ = (Node *)n; + } | alter_generic_options { AlterTableCmd *n = makeNode(AlterTableCmd); 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..5efb26d5df 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::pg_catalog.regcollation 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(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,78 @@ 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 ALTER INDEX ... DEPENDS ON COLLATION ... VERSION .... + */ +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); + + appendPQExpBuffer(buffer, "ALTER INDEX %s", + fmtQualifiedDumpable(indxinfo)); + appendPQExpBuffer(buffer, " DEPENDS ON ANY COLLATION UNKNOWN VERSION;\n"); + return; + } + + parsePGArray(inddependoids, &inddependoidsarray, &ninddependoids); + parsePGArray(inddependversions, &inddependversionsarray, &ninddependversions); + + Assert(ninddependoids == ninddependversions); + + for (i = 0; i < ninddependoids; i++) + { + if (strcmp(inddependversionsarray[i], "") == 0) + { + if (!unknown_coll_compat) + { + appendPQExpBuffer(buffer, "ALTER INDEX %s", + fmtQualifiedDumpable(indxinfo)); + appendPQExpBuffer(buffer, " DEPENDS ON COLLATION %s " + "UNKNOWN VERSION;\n", + inddependoidsarray[i]); + } + } + else + { + appendPQExpBuffer(buffer, "ALTER INDEX %s", + fmtQualifiedDumpable(indxinfo)); + appendPQExpBuffer(buffer, " DEPENDS ON COLLATION %s VERSION ", + inddependoidsarray[i]); + appendStringLiteral(buffer, inddependversionsarray[i], enc, true); + appendPQExpBuffer(buffer, ";\n"); + } + } + + 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..57172b57b3 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,49 @@ my %tests = ( %full_runs, %dump_test_schema_runs, section_pre_data => 1, }, unlike => { exclude_dump_test_schema => 1 }, + }, + + 'ALTER INDEX DEPENDS ON COLLATION 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/ALTER INDEX dump_test.regress_coll_idx1 DEPENDS ON COLLATION "fr-x-icu" VERSION 'not_a_version'/, + like => { binary_upgrade => 1, binary_coll_compatible => 1, }, + icu => 1, + }, + 'ALTER INDEX DEPENDS ON COLLATION UNKNOWN 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/ALTER INDEX dump_test.regress_coll_no_ver_idx1 DEPENDS ON COLLATION "fr-x-icu" UNKNOWN VERSION/, + 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 +3391,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 +3445,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 +3515,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 +3574,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 8d207b20ed..7daa251d4c 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/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index c96d027362..b36824ddfe 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_DependsOnCollationVersion /* DEPENDS ON COLLATION ... VERSION ... */ } AlterTableType; typedef struct ReplicaIdentityStmt @@ -1860,8 +1861,10 @@ 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 */ + char *version; /* version reference for collation dependency */ RoleSpec *newowner; Node *def; /* definition of new column, index, * constraint, or parent table */ diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm index bf095a7adb..d5c7866d2f 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