From 650c05c8c2c9d214860c6db6d8ab7709c42ad9e3 Mon Sep 17 00:00:00 2001 From: Julien Rouhaud Date: Thu, 7 Nov 2019 07:33:20 +0100 Subject: [PATCH 5/5] Preserve index dependencies on collation during pg_upgrade A new command ALTER INDEX i_name DEPENDS ON COLLATION c_oid VERSION v_name is added to force a dependency on a collation specific version, which is 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. --- src/backend/commands/tablecmds.c | 64 +++++++++++++ src/backend/nodes/copyfuncs.c | 1 + src/backend/parser/gram.y | 9 ++ src/bin/pg_dump/pg_dump.c | 153 ++++++++++++++++++++++++++++--- src/bin/pg_dump/pg_dump.h | 3 + src/include/nodes/parsenodes.h | 4 +- 6 files changed, 222 insertions(+), 12 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 5597be6e3d..1e94c0f5e2 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -203,6 +203,13 @@ typedef struct NewColumnValue ExprState *exprstate; /* execution state */ } NewColumnValue; +/* Struct describing one forced collation version dependency */ +typedef struct NewCollationVersionDependency +{ + NameData version; /* forced collation version */ + Oid oid; /* target collation oid */ +} NewCollationVersionDependency; + /* * Error-reporting support for RemoveRelations */ @@ -531,6 +538,7 @@ static void refuseDupeIndexAttach(Relation parentIdx, Relation partIdx, Relation partitionTbl); static List *GetParentedForeignKeyRefs(Relation partition); static void ATDetachCheckNoForeignKeyRefs(Relation partition); +static void ATExecDependsOnCollationVersion(Relation rel, Oid oid, char *version); /* ---------------------------------------------------------------- @@ -3799,6 +3807,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); @@ -3952,6 +3965,16 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, /* This command never recurses */ pass = AT_PASS_MISC; break; + case AT_DependsOnCollationVersion: /* DEPENDS ON COLLATION ... + * 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); @@ -4476,6 +4499,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->oid, cmd->name); + break; default: /* oops */ elog(ERROR, "unrecognized alter table type: %d", (int) cmd->subtype); @@ -16723,3 +16751,39 @@ ATDetachCheckNoForeignKeyRefs(Relation partition) table_close(rel, NoLock); } } + +static NameData * +index_force_collation_version(const ObjectAddress *otherObject, + const NameData *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 (otherObject->objectId != forced_dependency->oid) + return NULL; + + return &forced_dependency->version; +} + +static void +ATExecDependsOnCollationVersion(Relation rel, Oid oid, char *version) +{ + ObjectAddress object; + NewCollationVersionDependency forced_dependency; + + forced_dependency.oid = oid; + strncpy(NameStr(forced_dependency.version), version, sizeof(NameData)); + + object.classId = RelationRelationId; + object.objectId = rel->rd_id; + object.objectSubId = 0; + visitDependentObjects(&object, &index_force_collation_version, + &forced_dependency); +} diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 872a2d8dfc..fd3b535eff 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -3167,6 +3167,7 @@ _copyAlterTableCmd(const AlterTableCmd *from) COPY_SCALAR_FIELD(subtype); COPY_STRING_FIELD(name); COPY_SCALAR_FIELD(num); + COPY_SCALAR_FIELD(oid); 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 27c8aae569..7299499868 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -2547,6 +2547,15 @@ alter_table_cmd: n->subtype = AT_NoForceRowSecurity; $$ = (Node *)n; } + /* ALTER INDEX DEPENDS ON COLLATION ... VERSION ... */ + | DEPENDS ON COLLATION Iconst VERSION_P IDENT + { + AlterTableCmd *n = makeNode(AlterTableCmd); + n->subtype = AT_DependsOnCollationVersion; + n->name = $6; + n->oid = (Oid)$4; + $$ = (Node *)n; + } | alter_generic_options { AlterTableCmd *n = makeNode(AlterTableCmd); diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index bf69adc2f4..8ad5319885 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -288,6 +288,7 @@ 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); static void appendReloptionsArrayAH(PQExpBuffer buffer, const char *reloptions, const char *prefix, Archive *fout); static char *get_synchronized_snapshot(Archive *fout); @@ -6841,7 +6842,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 +6880,7 @@ 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, " @@ -6902,7 +6905,64 @@ 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, " + "(SELECT pg_catalog.array_agg(refobjid ORDER BY refobjid) " + " FROM pg_catalog.pg_depend " + " WHERE classid = 1259 AND " + " objid = i.indexrelid AND " + " objsubid = 0 AND " + " refclassid = 3456 AND " + " refobjversion != '') AS inddependoids, " + "(SELECT pg_catalog.array_agg(refobjversion ORDER BY refobjid) " + " FROM pg_catalog.pg_depend " + " WHERE classid = 1259 AND " + " objid = i.indexrelid AND " + " objsubid = 0 AND " + " refclassid = 3456 AND " + " refobjversion != '') 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, " + "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, " + "'' 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 +7001,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 +7038,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 +7071,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 +7107,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 +7149,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 +7176,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); @@ -16372,10 +16444,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) { @@ -16435,6 +16508,9 @@ dumpIndex(Archive *fout, IndxInfo *indxinfo) } } + if (dopt->binary_upgrade) + appendIndexCollationVersion(q, indxinfo); + /* If the index defines identity, we need to record that. */ if (indxinfo->indisreplident) { @@ -16464,6 +16540,20 @@ dumpIndex(Archive *fout, IndxInfo *indxinfo) if (indstatvalsarray) free(indstatvalsarray); } + else if (dopt->binary_upgrade) + { + appendIndexCollationVersion(q, indxinfo); + + 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) @@ -18421,6 +18511,47 @@ 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) +{ + char *inddependoids = indxinfo->inddependoids; + char *inddependversions = indxinfo->inddependversions; + char **inddependoidsarray = NULL; + char **inddependversionsarray = NULL; + int ninddependoids; + int ninddependversions; + int i; + + if (strcmp(inddependoids, "") == 0) + { + Assert(strcmp(inddependversions, "") == 0); + return; + } + + parsePGArray(inddependoids, &inddependoidsarray, &ninddependoids); + parsePGArray(inddependversions, &inddependversionsarray, &ninddependversions); + + Assert(ninddependoids == ninddependversions); + + for (i = 0; i < ninddependoids; i++) + { + appendPQExpBuffer(buffer, "ALTER INDEX %s ", + fmtQualifiedDumpable(indxinfo)); + appendPQExpBuffer(buffer, " DEPENDS ON COLLATION %s VERSION %s\n", + inddependoidsarray[i], + fmtQualifiedId(NULL, 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 7b2c1524a5..6f67d195dd 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/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 16a3b46bb3..467ce01359 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -1824,7 +1824,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 @@ -1842,6 +1843,7 @@ typedef struct AlterTableCmd /* one subcommand of an ALTER TABLE */ * or tablespace */ int16 num; /* attribute number for columns referenced by * number */ + Oid oid; /* oid reference for collation dependency */ RoleSpec *newowner; Node *def; /* definition of new column, index, * constraint, or parent table */ -- 2.20.1