pg_upgrade from 9.6 fails if old cluster had non-standard ACL on pg_catalog functions that have changed between versions, for example pg_stop_backup(boolean).
Error: pg_restore: creating ACL "pg_catalog.FUNCTION "pg_stop_backup"()"pg_restore: creating ACL "pg_catalog.FUNCTION "pg_stop_backup"("exclusive" boolean, OUT "lsn" "pg_lsn", OUT "labelfile" "text", OUT "spcmapfile" "text")"
pg_restore: [archiver (db)] Error while PROCESSING TOC:pg_restore: [archiver (db)] Error from TOC entry 2169; 0 0 ACL FUNCTION "pg_stop_backup"("exclusive" boolean, OUT "lsn" "pg_lsn", OUT "labelfile" "text", OUT "spcmapfile" "text") anastasia pg_restore: [archiver (db)] could not execute query: ERROR: function pg_catalog.pg_stop_backup(boolean) does not exist Command was: GRANT ALL ON FUNCTION "pg_catalog"."pg_stop_backup"("exclusive" boolean, OUT "lsn" "pg_lsn", OUT "labelfile" "text", OUT "spcmapfile" "text") TO "backup";
Steps to reproduce: 1) create a database with pg9.6 2) create a user and change grants on pg_stop_backup(boolean): CREATE ROLE backup WITH LOGIN; GRANT USAGE ON SCHEMA pg_catalog TO backup; GRANT EXECUTE ON FUNCTION pg_stop_backup() TO backup; GRANT EXECUTE ON FUNCTION pg_stop_backup(boolean) TO backup; 3) perform pg_upgrade to v10 (or any version above) The problem exists since we added to pg_dump support of ACL changes of pg_catalog functions in commit 23f34fa4b.I think this is a bug since it unpredictably affects user experience, so I propose to backpatch the fix. Script to reproduce the problem and the patch to fix it (credit to Arthur Zakirov) are attached.
Current patch contains a flag for pg_dump --change-old-names to enforce correct behavior.
I wonder, if we can make it default behavior for pg_upgrade? -- Anastasia Lubennikova Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
pg_dump_ACL_test.sh
Description: application/shellscript
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index bebec79..3c17a67 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -146,6 +146,7 @@ typedef struct _dumpOptions
/* flags for various command-line long options */
int disable_dollar_quoting;
int dump_inserts;
+ int change_old_names;
int column_inserts;
int if_exists;
int no_comments;
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 384b32b..4345d4e 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -245,7 +245,7 @@ static void makeTableDataInfo(DumpOptions *dopt, TableInfo *tbinfo, bool oids);
static void buildMatViewRefreshDependencies(Archive *fout);
static void getTableDataFKConstraints(void);
static char *format_function_arguments(FuncInfo *finfo, char *funcargs,
- bool is_agg);
+ bool is_agg, bool change_old_names);
static char *format_function_arguments_old(Archive *fout,
FuncInfo *finfo, int nallargs,
char **allargtypes,
@@ -360,6 +360,7 @@ main(int argc, char **argv)
*/
{"attribute-inserts", no_argument, &dopt.column_inserts, 1},
{"binary-upgrade", no_argument, &dopt.binary_upgrade, 1},
+ {"change-old-names", no_argument, &dopt.change_old_names, 1},
{"column-inserts", no_argument, &dopt.column_inserts, 1},
{"disable-dollar-quoting", no_argument, &dopt.disable_dollar_quoting, 1},
{"disable-triggers", no_argument, &dopt.disable_triggers, 1},
@@ -11538,6 +11539,65 @@ dumpProcLang(Archive *fout, ProcLangInfo *plang)
destroyPQExpBuffer(delqry);
}
+typedef struct FunctionMapping
+{
+ const char *old_name;
+ const char *old_args;
+ const char *new_name;
+ const char *new_args;
+} FunctionMapping;
+
+/*
+ * This array is used to map old names and arguments with new values.
+ */
+static FunctionMapping func_mappings[] =
+{
+ /* Renames */
+ { "pg_current_xlog_flush_location", NULL, "pg_current_wal_flush_lsn", NULL},
+ { "pg_current_xlog_insert_location", NULL, "pg_current_wal_insert_lsn", NULL},
+ { "pg_current_xlog_location", NULL, "pg_current_wal_lsn", NULL},
+ { "pg_is_xlog_replay_paused", NULL, "pg_is_wal_replay_paused", NULL},
+ { "pg_last_xlog_receive_location", NULL, "pg_last_wal_receive_lsn", NULL},
+ { "pg_last_xlog_replay_location", NULL, "pg_last_wal_replay_lsn", NULL},
+ { "pg_switch_xlog", NULL, "pg_switch_wal", NULL},
+ { "pg_xlog_location_diff", NULL, "pg_wal_lsn_diff", NULL},
+ { "pg_xlog_replay_pause", NULL, "pg_wal_replay_pause", NULL},
+ { "pg_xlog_replay_resume", NULL, "pg_wal_replay_resume", NULL},
+ { "pg_xlogfile_name", NULL, "pg_walfile_name", NULL},
+ { "pg_xlogfile_name_offset", NULL, "pg_walfile_name_offset", NULL},
+
+ /* Argument changes */
+
+ {
+ "pg_create_logical_replication_slot",
+ "\"slot_name\" \"name\", \"plugin\" \"name\", OUT \"slot_name\" \"text\", OUT \"xlog_position\" \"pg_lsn\"",
+ "pg_create_logical_replication_slot",
+ "\"slot_name\" \"name\", \"plugin\" \"name\", \"temporary\" boolean, OUT \"slot_name\" \"text\", OUT \"lsn\" \"pg_lsn\""
+ },
+ {
+ "pg_create_physical_replication_slot",
+ "\"slot_name\" \"name\", \"immediately_reserve\" boolean DEFAULT false, OUT \"slot_name\" \"name\", OUT \"xlog_position\" \"pg_lsn\"",
+ "pg_create_physical_replication_slot",
+ "\"slot_name\" \"name\", \"immediately_reserve\" boolean DEFAULT false, \"temporary\" boolean DEFAULT false, OUT \"slot_name\" \"name\", OUT \"lsn\" \"pg_lsn\"",
+ },
+ /* pg_create_physical_replication_slot without defaults */
+ {
+ "pg_create_physical_replication_slot",
+ "\"slot_name\" \"name\", \"immediately_reserve\" boolean, OUT \"slot_name\" \"name\", OUT \"xlog_position\" \"pg_lsn\"",
+ "pg_create_physical_replication_slot",
+ "\"slot_name\" \"name\", \"immediately_reserve\" boolean, \"temporary\" boolean, OUT \"slot_name\" \"name\", OUT \"lsn\" \"pg_lsn\"",
+ },
+ {
+ "pg_stop_backup",
+ "\"exclusive\" boolean, OUT \"lsn\" \"pg_lsn\", OUT \"labelfile\" \"text\", OUT \"spcmapfile\" \"text\"",
+ "pg_stop_backup",
+ "\"exclusive\" boolean, \"wait_for_archive\" boolean, OUT \"lsn\" \"pg_lsn\", OUT \"labelfile\" \"text\", OUT \"spcmapfile\" \"text\""
+ },
+
+ /* end of mappings */
+ { NULL }
+};
+
/*
* format_function_arguments: generate function name and argument list
*
@@ -11546,16 +11606,52 @@ dumpProcLang(Archive *fout, ProcLangInfo *plang)
* does not special-case zero-argument aggregates.
*/
static char *
-format_function_arguments(FuncInfo *finfo, char *funcargs, bool is_agg)
+format_function_arguments(FuncInfo *finfo, char *funcargs, bool is_agg,
+ bool change_old_names)
{
PQExpBufferData fn;
+ const char *name = finfo->dobj.name;
+ const char *args = funcargs;
+ int i;
+
+ /*
+ * Map old names and arguments with new names. If --change-old-names is set
+ * then old names and arguments are replaced and new values will be wrote in
+ * pg_dump output.
+ *
+ * Currently mapping works only if --quote-all-identifiers is set. Otherwise
+ * it requires to handle mappings without quotes. But it is enough for
+ * pg_upgrade wich runs pg_dump with --quote-all-identifiers
+ */
+ if (change_old_names && quote_all_identifiers)
+ {
+ for (i = 0; func_mappings[i].old_name; i++)
+ {
+ FunctionMapping *map = &func_mappings[i];
+
+ /* If arguments don't matter */
+ if (strcmp(map->old_name, name) == 0 && map->old_args == NULL)
+ {
+ name = map->new_name;
+ break;
+ }
+ /* Rename with arguments */
+ else if (strcmp(map->old_name, name) == 0 &&
+ strcmp(map->old_args, args) == 0)
+ {
+ name = map->new_name;
+ args = map->new_args;
+ break;
+ }
+ }
+ }
initPQExpBuffer(&fn);
- appendPQExpBufferStr(&fn, fmtId(finfo->dobj.name));
+ appendPQExpBufferStr(&fn, fmtId(name));
if (is_agg && finfo->nargs == 0)
appendPQExpBufferStr(&fn, "(*)");
else
- appendPQExpBuffer(&fn, "(%s)", funcargs);
+ appendPQExpBuffer(&fn, "(%s)", args);
return fn.data;
}
@@ -12002,9 +12098,18 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
if (funcargs)
{
+ bool change_old_names;
+
+ /* pre-10, map old names and arguments with new names if necessary */
+ change_old_names = fout->remoteVersion < 100000 &&
+ dopt->change_old_names == 1 &&
+ strcmp(finfo->dobj.namespace->dobj.name, "pg_catalog") == 0;
+
/* 8.4 or later; we rely on server-side code for most of the work */
- funcfullsig = format_function_arguments(finfo, funcargs, false);
- funcsig = format_function_arguments(finfo, funciargs, false);
+ funcfullsig = format_function_arguments(finfo, funcargs, false,
+ change_old_names);
+ funcsig = format_function_arguments(finfo, funciargs, false,
+ change_old_names);
}
else
/* pre-8.4, do it ourselves */
@@ -14009,11 +14114,19 @@ dumpAgg(Archive *fout, AggInfo *agginfo)
/* 8.4 or later; we rely on server-side code for most of the work */
char *funcargs;
char *funciargs;
+ bool change_old_names;
+
+ /* pre-10, map old names and arguments with new names if necessary */
+ change_old_names = fout->remoteVersion < 100000 &&
+ dopt->change_old_names == 1 &&
+ strcmp(agginfo->aggfn.dobj.namespace->dobj.name, "pg_catalog") == 0;
funcargs = PQgetvalue(res, 0, PQfnumber(res, "funcargs"));
funciargs = PQgetvalue(res, 0, PQfnumber(res, "funciargs"));
- aggfullsig = format_function_arguments(&agginfo->aggfn, funcargs, true);
- aggsig = format_function_arguments(&agginfo->aggfn, funciargs, true);
+ aggfullsig = format_function_arguments(&agginfo->aggfn, funcargs, true,
+ change_old_names);
+ aggsig = format_function_arguments(&agginfo->aggfn, funciargs, true,
+ change_old_names);
}
else
/* pre-8.4, do it ourselves */
diff --git a/src/bin/pg_upgrade/dump.c b/src/bin/pg_upgrade/dump.c
index def22c6..697a7b5 100644
--- a/src/bin/pg_upgrade/dump.c
+++ b/src/bin/pg_upgrade/dump.c
@@ -54,6 +54,7 @@ generate_old_dump(void)
parallel_exec_prog(log_file_name, NULL,
"\"%s/pg_dump\" %s --schema-only --quote-all-identifiers "
+ "--change-old-names "
"--binary-upgrade --format=custom %s --file=\"%s\" %s",
new_cluster.bindir, cluster_conn_opts(&old_cluster),
log_opts.verbose ? "--verbose" : "",
