Hi,
Thanks for your review.
I created an updated patch according to your notices.
1 - Patch does not apply cleanly on latest git repository, although
there is no hunk failed but there are some hunk succeeded messages.
Rebased to the current HEAD.
2- Patch contains unnecessary spaces and tabs which makes the patch
unnecessarily big. IMHO please read the patch before sending and make
sure that patch only contains the changes you intended to send.
Yes, there were trailing whitespaces in the original files which
were removed by the previous patch. The attached version leaves them as is.
3 - We should follow the coding standards of existing code
I tried, of course, but this escaped my observation.
4 - pg_restore gives error wile restoring custom format backup
5 - Do you really want to write this code like this
Fixed.
I also need some feedback about the role support in pg_restore (not implemented
yet).
Currently pg_restore sets the role during the restore process according to the
TOC
entry in the archive. It may also support the --role option (just like pg_dump).
If specified it can be used to cancel the effect of the TOC entry and force the
emitting of the SET ROLE ... command. With emtpy argument it can be used to omit
the SET ROLE even if it is specified in the archieve. What do you think?
Thank you again.
doc/src/sgml/ref/pg_dump.sgml | 16 ++++++++++
doc/src/sgml/ref/pg_dumpall.sgml | 15 +++++++++
src/bin/pg_dump/pg_backup.h | 2 +
src/bin/pg_dump/pg_backup_archiver.c | 36 +++++++++++++++++++++-
src/bin/pg_dump/pg_dump.c | 53 ++++++++++++++++++++++++++++++++++
src/bin/pg_dump/pg_dumpall.c | 23 ++++++++++++++
6 files changed, 143 insertions(+), 2 deletions(-)
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 2e30906..de139c3 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -698,6 +698,22 @@ PostgreSQL documentation
</para>
</listitem>
</varlistentry>
+
+ <varlistentry>
+ <term><option>--role=<replaceable class="parameter">rolename</replaceable></option></term>
+ <listitem>
+ <para>
+ Specifies the user identifier used by the dump session. This cause
+ <application>pg_dump</application> to issue a
+ <command>SET ROLE TO <replaceable class="parameter">rolename</replaceable></command>
+ command just after a successful database connection. It is useful in cases when
+ the logged in user specified by the -U option has not enough privileges needed by
+ <application>pg_dump</application> but can switch to a role with the needed rights.
+ The SET ROLE command is reserved in the archive because most of the time this
+ user identifier also needed for the restore to succeed.
+ </para>
+ </listitem>
+ </varlistentry>
</variablelist>
</para>
</refsect1>
diff --git a/doc/src/sgml/ref/pg_dumpall.sgml b/doc/src/sgml/ref/pg_dumpall.sgml
index ec40890..e3016cd 100644
--- a/doc/src/sgml/ref/pg_dumpall.sgml
+++ b/doc/src/sgml/ref/pg_dumpall.sgml
@@ -417,6 +417,21 @@ PostgreSQL documentation
</para>
</listitem>
</varlistentry>
+
+ <varlistentry>
+ <term><option>--role=<replaceable class="parameter">rolename</replaceable></option></term>
+ <listitem>
+ <para>
+ Specifies the user identifier used by the dump session. This option is passed
+ to <application>pg_dump</> too and cause these applications to issue a
+ <command>SET ROLE TO <replaceable class="parameter">rolename</replaceable></command>
+ command just after a successful database connection. It is useful in cases when
+ the logged in user specified by the -U option has not enough privileges needed by
+ <application>pg_dumpall</application> but can switch to a role with the needed rights.
+ The SET ROLE command is reserved in the archive by <application>pg_dump</application>.
+ </para>
+ </listitem>
+ </varlistentry>
</variablelist>
</para>
</refsect1>
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index c57bb22..cbe4d46 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -70,6 +70,8 @@ typedef struct _Archive
int encoding; /* libpq code for client_encoding */
bool std_strings; /* standard_conforming_strings */
+ const char *rolename; /* role name */
+
/* error handling */
bool exit_on_error; /* whether to exit on SQL errors... */
int n_errors; /* number of errors (if no die) */
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 7bd44f2..53a469d 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -56,6 +56,7 @@ static void _selectOutputSchema(ArchiveHandle *AH, const char *schemaName);
static void _selectTablespace(ArchiveHandle *AH, const char *tablespace);
static void processEncodingEntry(ArchiveHandle *AH, TocEntry *te);
static void processStdStringsEntry(ArchiveHandle *AH, TocEntry *te);
+static void processRolenameEntry(ArchiveHandle *AH, TocEntry *te);
static teReqs _tocEntryRequired(TocEntry *te, RestoreOptions *ropt, bool include_acls);
static void _disableTriggersIfNecessary(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt);
static void _enableTriggersIfNecessary(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt);
@@ -1979,6 +1980,8 @@ ReadToc(ArchiveHandle *AH)
processEncodingEntry(AH, te);
else if (strcmp(te->desc, "STDSTRINGS") == 0)
processStdStringsEntry(AH, te);
+ else if (strcmp(te->desc, "ROLENAME") == 0)
+ processRolenameEntry(AH, te);
}
}
@@ -2026,14 +2029,38 @@ processStdStringsEntry(ArchiveHandle *AH, TocEntry *te)
te->defn);
}
+static void
+processRolenameEntry(ArchiveHandle *AH, TocEntry *te)
+{
+ /* te->defn should have the form SET role = 'foo'; */
+ char *defn = strdup(te->defn);
+ char *ptr1;
+ char *ptr2 = NULL;
+
+ ptr1 = strchr(defn, '\'');
+ if (ptr1)
+ ptr2 = strchr(++ptr1, '\'');
+ if (ptr2)
+ {
+ *ptr2 = '\0';
+ AH->public.rolename = strdup(ptr1);
+ }
+ else
+ die_horribly(AH, modulename, "invalid ROLENAME item: %s\n",
+ te->defn);
+
+ free(defn);
+}
+
static teReqs
_tocEntryRequired(TocEntry *te, RestoreOptions *ropt, bool include_acls)
{
teReqs res = REQ_ALL;
- /* ENCODING and STDSTRINGS items are dumped specially, so always reject */
+ /* ENCODING, STDSTRINGS and ROLENAME items are dumped specially, so always reject */
if (strcmp(te->desc, "ENCODING") == 0 ||
- strcmp(te->desc, "STDSTRINGS") == 0)
+ strcmp(te->desc, "STDSTRINGS") == 0 ||
+ strcmp(te->desc, "ROLENAME") == 0)
return 0;
/* If it's an ACL, maybe ignore it */
@@ -2146,6 +2173,11 @@ _doSetFixedOutputState(ArchiveHandle *AH)
ahprintf(AH, "SET standard_conforming_strings = %s;\n",
AH->public.std_strings ? "on" : "off");
+ /* Select the role to be used during restore */
+ if (AH->public.rolename)
+ ahprintf(AH, "SET role = %s;\n",
+ fmtId(AH->public.rolename));
+
/* Make sure function checking is disabled */
ahprintf(AH, "SET check_function_bodies = false;\n");
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 4f6a088..dde9495 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -189,6 +189,7 @@ static int dumpBlobComments(Archive *AH, void *arg);
static void dumpDatabase(Archive *AH);
static void dumpEncoding(Archive *AH);
static void dumpStdStrings(Archive *AH);
+static void dumpRole(Archive *AH);
static const char *getAttrName(int attrnum, TableInfo *tblInfo);
static const char *fmtCopyColumnList(const TableInfo *ti);
static void do_sql_command(PGconn *conn, const char *query);
@@ -230,6 +231,10 @@ main(int argc, char **argv)
static int outputNoTablespaces = 0;
static int use_setsessauth = 0;
+ const char *pgrole = NULL;
+ PQExpBuffer roleQry;
+ PGresult *res;
+
static struct option long_options[] = {
{"data-only", no_argument, NULL, 'a'},
{"blobs", no_argument, NULL, 'b'},
@@ -270,6 +275,7 @@ main(int argc, char **argv)
{"lock-wait-timeout", required_argument, NULL, 2},
{"no-tablespaces", no_argument, &outputNoTablespaces, 1},
{"use-set-session-authorization", no_argument, &use_setsessauth, 1},
+ {"role", required_argument, NULL, 3},
{NULL, 0, NULL, 0}
};
@@ -447,6 +453,10 @@ main(int argc, char **argv)
lockWaitTimeout = optarg;
break;
+ case 3: /* role */
+ pgrole = optarg;
+ break;
+
default:
fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
exit(1);
@@ -556,6 +566,20 @@ main(int argc, char **argv)
}
}
+ /* Set the role if requested */
+ if (pgrole)
+ {
+ roleQry = createPQExpBuffer();
+ appendPQExpBuffer(roleQry, "SET ROLE TO %s;\n", fmtId(pgrole));
+ res = PQexec(g_conn, roleQry->data);
+ check_sql_result(res, g_conn, roleQry->data, PGRES_COMMAND_OK);
+ PQclear(res);
+ destroyPQExpBuffer(roleQry);
+ g_fout->rolename = pgrole;
+ }
+ else
+ g_fout->rolename = NULL;
+
/*
* Get the active encoding and the standard_conforming_strings setting, so
* we know how to escape strings.
@@ -716,6 +740,8 @@ main(int argc, char **argv)
/* First the special ENCODING and STDSTRINGS entries. */
dumpEncoding(g_fout);
dumpStdStrings(g_fout);
+ if (pgrole)
+ dumpRole(g_fout);
/* The database item is always next, unless we don't want it at all */
if (include_everything && !dataOnly)
@@ -801,6 +827,7 @@ help(const char *progname)
printf(_(" --use-set-session-authorization\n"
" use SESSION AUTHORIZATION commands instead of\n"
" ALTER OWNER commands to set ownership\n"));
+ printf(_(" --role set role before dump\n"));
printf(_("\nConnection options:\n"));
printf(_(" -h, --host=HOSTNAME database server host or socket directory\n"));
@@ -1801,6 +1828,32 @@ dumpStdStrings(Archive *AH)
/*
+ * dumpRole: put the role change into the archive
+ */
+static void
+dumpRole(Archive *AH)
+{
+ const char *rolename = AH->rolename;
+ PQExpBuffer qry = createPQExpBuffer();
+
+ if (g_verbose)
+ write_msg(NULL, "saving rolename = %s\n", rolename);
+
+ appendPQExpBuffer(qry, "SET role = ");
+ appendStringLiteralAH(qry, rolename, AH);
+ appendPQExpBuffer(qry, ";\n");
+
+ ArchiveEntry(AH, nilCatalogId, createDumpId(),
+ "ROLENAME", NULL, NULL, "",
+ false, "ROLENAME", qry->data, "", NULL,
+ NULL, 0,
+ NULL, NULL);
+
+ destroyPQExpBuffer(qry);
+}
+
+
+/*
* hasBlobs:
* Test whether database contains any large objects
*/
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index b00fb5a..b5d6ef2 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -89,6 +89,9 @@ main(int argc, char *argv[])
int c,
ret;
+ const char *pgrole = NULL;
+ PQExpBuffer roleQry;
+
static struct option long_options[] = {
{"data-only", no_argument, NULL, 'a'},
{"clean", no_argument, NULL, 'c'},
@@ -121,6 +124,7 @@ main(int argc, char *argv[])
{"no-tablespaces", no_argument, &no_tablespaces, 1},
{"use-set-session-authorization", no_argument, &use_setsessauth, 1},
{"lock-wait-timeout", required_argument, NULL, 2},
+ {"role", required_argument, NULL, 3},
{NULL, 0, NULL, 0}
};
@@ -311,6 +315,15 @@ main(int argc, char *argv[])
appendPQExpBuffer(pgdumpopts, optarg);
break;
+ case 3:
+ pgrole = optarg;
+#ifndef WIN32
+ appendPQExpBuffer(pgdumpopts, " --role '%s'", optarg);
+#else
+ appendPQExpBuffer(pgdumpopts, " --role \"%s\"", optarg);
+#endif
+ break;
+
default:
fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
exit(1);
@@ -426,6 +439,15 @@ main(int argc, char *argv[])
if (!std_strings)
std_strings = "off";
+ /* Set the role if requested */
+ if (pgrole)
+ {
+ roleQry = createPQExpBuffer();
+ appendPQExpBuffer(roleQry, "SET ROLE TO %s;\n", fmtId(pgrole));
+ executeCommand(conn, roleQry->data);
+ destroyPQExpBuffer(roleQry);
+ }
+
fprintf(OPF, "--\n-- PostgreSQL database cluster dump\n--\n\n");
if (verbose)
dumpTimestamp("Started on");
@@ -516,6 +538,7 @@ help(void)
printf(_(" --use-set-session-authorization\n"
" use SESSION AUTHORIZATION commands instead of\n"
" OWNER TO commands\n"));
+ printf(_(" --role set role before dump\n"));
printf(_("\nConnection options:\n"));
printf(_(" -h, --host=HOSTNAME database server host or socket directory\n"));
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
