On Thu, Feb 11, 2021 at 04:08:34AM -0800, Noah Misch wrote:
> On Sun, Jan 17, 2021 at 12:00:06PM +0100, Vik Fearing wrote:
> > On 1/17/21 10:41 AM, Noah Misch wrote:
> > > On Sat, Jan 16, 2021 at 02:05:43PM +0100, Vik Fearing wrote:
> > >> On 12/30/20 12:59 PM, Noah Misch wrote:
> > >>> On Tue, Dec 29, 2020 at 05:49:24AM -0800, Noah Misch wrote:
> > >>>> https://postgr.es/m/20201031163518.gb4039...@rfd.leadboat.com gave
> > >>>> $SUBJECT as
> > >>>> one of the constituent projects for changing the public schema default
> > >>>> ACL.
> > >>>> This ended up being simple. Attached.
> > >>>
> > >>> This is defective; it fails to reproduce nspacl after "ALTER SCHEMA
> > >>> public
> > >>> OWNER TO pg_write_server_files; REVOKE ALL ON SCHEMA public FROM
> > >>> pg_write_server_files;". I will try again later.
>
> Fixed. The comment added to getNamespaces() explains what went wrong.
>
> Incidentally, --no-acl is fragile without --no-owner, because any REVOKE
> statements assume a particular owner. Since one can elect --no-owner at
> restore time, we can't simply adjust the REVOKE statements constructed at dump
> time. That's not new with this patch or specific to initdb-created objects.
>
> > >> Could I ask you to also include COMMENTs when you try again, please?
> > >
> > > That may work. I had not expected to hear of a person changing the
> > > comment on
> > > schema public. To what do you change it?
> >
> > It was a while ago and I don't remember because it didn't appear in the
> > dump so I stopped doing it. :(
> >
> > Mine was an actual comment, but there are some tools out there that
> > (ab)use COMMENTs as crude metadata for what they do. For example:
> > https://postgresql-anonymizer.readthedocs.io/en/stable/declare_masking_rules/#declaring-rules-with-comments
>
> I've attached a separate patch for this, which applies atop the ownership
> patch. This makes more restores fail for non-superusers, which is okay.
Oops, I botched a refactoring late in the development of that patch. Here's a
fixed pair of patches.
Author: Noah Misch <n...@leadboat.com>
Commit: Noah Misch <n...@leadboat.com>
Dump public schema ownership and security labels.
As a side effect, this corrects dumps of public schema ACLs in databases
where the DBA dropped and recreated that schema.
Reviewed by FIXME.
Discussion: https://postgr.es/m/20201229134924.ga1431...@rfd.leadboat.com
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index 60d306e..ea67e52 100644
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -725,6 +725,7 @@ void
buildACLQueries(PQExpBuffer acl_subquery, PQExpBuffer racl_subquery,
PQExpBuffer init_acl_subquery, PQExpBuffer
init_racl_subquery,
const char *acl_column, const char *acl_owner,
+ const char *initprivs_expr,
const char *obj_kind, bool binary_upgrade)
{
/*
@@ -765,23 +766,25 @@ buildACLQueries(PQExpBuffer acl_subquery, PQExpBuffer
racl_subquery,
"WITH ORDINALITY AS perm(acl,row_n) "
"WHERE NOT EXISTS ( "
"SELECT 1 FROM "
-
"pg_catalog.unnest(coalesce(pip.initprivs,pg_catalog.acldefault(%s,%s))) "
+
"pg_catalog.unnest(coalesce(%s,pg_catalog.acldefault(%s,%s))) "
"AS init(init_acl) WHERE acl =
init_acl)) as foo)",
acl_column,
obj_kind,
acl_owner,
+ initprivs_expr,
obj_kind,
acl_owner);
printfPQExpBuffer(racl_subquery,
"(SELECT pg_catalog.array_agg(acl
ORDER BY row_n) FROM "
"(SELECT acl, row_n FROM "
-
"pg_catalog.unnest(coalesce(pip.initprivs,pg_catalog.acldefault(%s,%s))) "
+
"pg_catalog.unnest(coalesce(%s,pg_catalog.acldefault(%s,%s))) "
"WITH ORDINALITY AS initp(acl,row_n) "
"WHERE NOT EXISTS ( "
"SELECT 1 FROM "
"pg_catalog.unnest(coalesce(%s,pg_catalog.acldefault(%s,%s))) "
"AS permp(orig_acl) WHERE acl =
orig_acl)) as foo)",
+ initprivs_expr,
obj_kind,
acl_owner,
acl_column,
@@ -807,12 +810,13 @@ buildACLQueries(PQExpBuffer acl_subquery, PQExpBuffer
racl_subquery,
printfPQExpBuffer(init_acl_subquery,
"CASE WHEN privtype = 'e'
THEN "
"(SELECT
pg_catalog.array_agg(acl ORDER BY row_n) FROM "
- "(SELECT acl, row_n FROM
pg_catalog.unnest(pip.initprivs) "
+ "(SELECT acl, row_n FROM
pg_catalog.unnest(%s) "
"WITH ORDINALITY AS
initp(acl,row_n) "
"WHERE NOT EXISTS ( "
"SELECT 1 FROM "
"pg_catalog.unnest(pg_catalog.acldefault(%s,%s)) "
"AS privm(orig_acl) WHERE acl
= orig_acl)) as foo) END",
+ initprivs_expr,
obj_kind,
acl_owner);
@@ -823,10 +827,11 @@ buildACLQueries(PQExpBuffer acl_subquery, PQExpBuffer
racl_subquery,
"pg_catalog.unnest(pg_catalog.acldefault(%s,%s)) "
"WITH ORDINALITY AS
privp(acl,row_n) "
"WHERE NOT EXISTS ( "
- "SELECT 1 FROM
pg_catalog.unnest(pip.initprivs) "
+ "SELECT 1 FROM
pg_catalog.unnest(%s) "
"AS initp(init_acl) WHERE acl
= init_acl)) as foo) END",
obj_kind,
- acl_owner);
+ acl_owner,
+ initprivs_expr);
}
else
{
diff --git a/src/bin/pg_dump/dumputils.h b/src/bin/pg_dump/dumputils.h
index 6e97da7..f5465f1 100644
--- a/src/bin/pg_dump/dumputils.h
+++ b/src/bin/pg_dump/dumputils.h
@@ -54,6 +54,7 @@ extern void emitShSecLabels(PGconn *conn, PGresult *res,
extern void buildACLQueries(PQExpBuffer acl_subquery, PQExpBuffer
racl_subquery,
PQExpBuffer
init_acl_subquery, PQExpBuffer init_racl_subquery,
const char *acl_column,
const char *acl_owner,
+ const char
*initprivs_expr,
const char *obj_kind,
bool binary_upgrade);
extern bool variable_is_guc_list_quote(const char *name);
diff --git a/src/bin/pg_dump/pg_backup_archiver.c
b/src/bin/pg_dump/pg_backup_archiver.c
index 1f82c64..a16d149 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -3597,10 +3597,13 @@ _printTocEntry(ArchiveHandle *AH, TocEntry *te, bool
isData)
* Actually print the definition.
*
* Really crude hack for suppressing AUTHORIZATION clause that old
pg_dump
- * versions put into CREATE SCHEMA. We have to do this when --no-owner
- * mode is selected. This is ugly, but I see no other good way ...
+ * versions put into CREATE SCHEMA. Don't mutate the modern definition
+ * for schema "public", which consists of a comment. We have to do this
+ * when --no-owner mode is selected. This is ugly, but I see no other
+ * good way ...
*/
- if (ropt->noOwner && strcmp(te->desc, "SCHEMA") == 0)
+ if (ropt->noOwner &&
+ strcmp(te->desc, "SCHEMA") == 0 && strncmp(te->defn, "--", 2)
!= 0)
{
ahprintf(AH, "CREATE SCHEMA %s;\n\n\n", fmtId(te->tag));
}
@@ -3612,11 +3615,16 @@ _printTocEntry(ArchiveHandle *AH, TocEntry *te, bool
isData)
/*
* If we aren't using SET SESSION AUTH to determine ownership, we must
- * instead issue an ALTER OWNER command. We assume that anything
without
- * a DROP command is not a separately ownable object. All the
categories
- * with DROP commands must appear in one list or the other.
+ * instead issue an ALTER OWNER command. Schema "public" is special; a
+ * dump never creates it, so we use ALTER OWNER even when using SET
+ * SESSION for all other objects. We assume that anything without a
DROP
+ * command is not a separately ownable object. All the categories with
+ * DROP commands must appear in one list or the other.
*/
- if (!ropt->noOwner && !ropt->use_setsessauth &&
+ if (!ropt->noOwner &&
+ (!ropt->use_setsessauth ||
+ (strcmp(te->tag, "public") == 0
+ && strcmp(te->desc, "SCHEMA") == 0)) &&
te->owner && strlen(te->owner) > 0 &&
te->dropStmt && strlen(te->dropStmt) > 0)
{
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index d99b61e..e8ea385 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -44,6 +44,7 @@
#include "catalog/pg_aggregate_d.h"
#include "catalog/pg_am_d.h"
#include "catalog/pg_attribute_d.h"
+#include "catalog/pg_authid_d.h"
#include "catalog/pg_cast_d.h"
#include "catalog/pg_class_d.h"
#include "catalog/pg_collation_d.h"
@@ -1567,10 +1568,14 @@ selectDumpableNamespace(NamespaceInfo *nsinfo, Archive
*fout)
* no-mans-land between being a system object and a user
object. We
* don't want to dump creation or comment commands for it,
because
* that complicates matters for non-superuser use of pg_dump.
But we
- * should dump any ACL changes that have occurred for it, and of
- * course we should dump contained objects.
+ * should dump any ownership changes, security labels, and ACL
+ * changes, and of course we should dump contained objects.
pg_dump
+ * ties ownership to DUMP_COMPONENT_DEFINITION. Hence, unless
the
+ * owner is the default, dump a "definition" bearing just a
comment.
*/
- nsinfo->dobj.dump = DUMP_COMPONENT_ACL;
+ nsinfo->dobj.dump = DUMP_COMPONENT_ALL &
~DUMP_COMPONENT_COMMENT;
+ if (nsinfo->nspowner == BOOTSTRAP_SUPERUSERID)
+ nsinfo->dobj.dump &= ~DUMP_COMPONENT_DEFINITION;
nsinfo->dobj.dump_contains = DUMP_COMPONENT_ALL;
}
else
@@ -3352,8 +3357,8 @@ getBlobs(Archive *fout)
PQExpBuffer init_racl_subquery = createPQExpBuffer();
buildACLQueries(acl_subquery, racl_subquery, init_acl_subquery,
- init_racl_subquery, "l.lomacl",
"l.lomowner", "'L'",
- dopt->binary_upgrade);
+ init_racl_subquery, "l.lomacl",
"l.lomowner",
+ "pip.initprivs", "'L'",
dopt->binary_upgrade);
appendPQExpBuffer(blobQry,
"SELECT l.oid, (%s
l.lomowner) AS rolname, "
@@ -4754,6 +4759,7 @@ getNamespaces(Archive *fout, int *numNamespaces)
int i_tableoid;
int i_oid;
int i_nspname;
+ int i_nspowner;
int i_rolname;
int i_nspacl;
int i_rnspacl;
@@ -4773,11 +4779,27 @@ getNamespaces(Archive *fout, int *numNamespaces)
PQExpBuffer init_acl_subquery = createPQExpBuffer();
PQExpBuffer init_racl_subquery = createPQExpBuffer();
+ /*
+ * Bypass pg_init_privs.initprivs for the public schema.
Dropping and
+ * recreating the schema detaches it from its pg_init_privs
row, but
+ * an empty destination database starts with this ACL
nonetheless.
+ * Also, we support dump/reload of public schema ownership
changes.
+ * ALTER SCHEMA OWNER filters nspacl through aclnewowner(), but
+ * initprivs continues to reflect the initial owner (the
bootstrap
+ * superuser). Hence, synthesize the value that nspacl will
have
+ * after the restore's ALTER SCHEMA OWNER.
+ */
buildACLQueries(acl_subquery, racl_subquery, init_acl_subquery,
- init_racl_subquery, "n.nspacl",
"n.nspowner", "'n'",
- dopt->binary_upgrade);
+ init_racl_subquery, "n.nspacl",
"n.nspowner",
+ "CASE WHEN n.nspname = 'public'
THEN array["
+ " format('%s=UC/%s', "
+ " n.nspowner::regrole,
n.nspowner::regrole),"
+ " format('=UC/%s',
n.nspowner::regrole)]::aclitem[] "
+ "ELSE pip.initprivs END",
+ "'n'", dopt->binary_upgrade);
appendPQExpBuffer(query, "SELECT n.tableoid, n.oid, n.nspname, "
+ "n.nspowner, "
"(%s nspowner) AS rolname, "
"%s as nspacl, "
"%s as rnspacl, "
@@ -4802,7 +4824,7 @@ getNamespaces(Archive *fout, int *numNamespaces)
destroyPQExpBuffer(init_racl_subquery);
}
else
- appendPQExpBuffer(query, "SELECT tableoid, oid, nspname, "
+ appendPQExpBuffer(query, "SELECT tableoid, oid, nspname,
nspowner, "
"(%s nspowner) AS rolname, "
"nspacl, NULL as rnspacl, "
"NULL AS initnspacl, NULL as
initrnspacl "
@@ -4818,6 +4840,7 @@ getNamespaces(Archive *fout, int *numNamespaces)
i_tableoid = PQfnumber(res, "tableoid");
i_oid = PQfnumber(res, "oid");
i_nspname = PQfnumber(res, "nspname");
+ i_nspowner = PQfnumber(res, "nspowner");
i_rolname = PQfnumber(res, "rolname");
i_nspacl = PQfnumber(res, "nspacl");
i_rnspacl = PQfnumber(res, "rnspacl");
@@ -4831,6 +4854,7 @@ getNamespaces(Archive *fout, int *numNamespaces)
nsinfo[i].dobj.catId.oid = atooid(PQgetvalue(res, i, i_oid));
AssignDumpId(&nsinfo[i].dobj);
nsinfo[i].dobj.name = pg_strdup(PQgetvalue(res, i, i_nspname));
+ nsinfo[i].nspowner = atooid(PQgetvalue(res, i, i_nspowner));
nsinfo[i].rolname = pg_strdup(PQgetvalue(res, i, i_rolname));
nsinfo[i].nspacl = pg_strdup(PQgetvalue(res, i, i_nspacl));
nsinfo[i].rnspacl = pg_strdup(PQgetvalue(res, i, i_rnspacl));
@@ -5022,8 +5046,8 @@ getTypes(Archive *fout, int *numTypes)
PQExpBuffer initracl_subquery = createPQExpBuffer();
buildACLQueries(acl_subquery, racl_subquery, initacl_subquery,
- initracl_subquery, "t.typacl",
"t.typowner", "'T'",
- dopt->binary_upgrade);
+ initracl_subquery, "t.typacl",
"t.typowner",
+ "pip.initprivs", "'T'",
dopt->binary_upgrade);
appendPQExpBuffer(query, "SELECT t.tableoid, t.oid, t.typname, "
"t.typnamespace, "
@@ -5724,8 +5748,8 @@ getAggregates(Archive *fout, int *numAggs)
const char *agg_check;
buildACLQueries(acl_subquery, racl_subquery, initacl_subquery,
- initracl_subquery, "p.proacl",
"p.proowner", "'f'",
- dopt->binary_upgrade);
+ initracl_subquery, "p.proacl",
"p.proowner",
+ "pip.initprivs", "'f'",
dopt->binary_upgrade);
agg_check = (fout->remoteVersion >= 110000 ? "p.prokind = 'a'"
: "p.proisagg");
@@ -5937,8 +5961,8 @@ getFuncs(Archive *fout, int *numFuncs)
const char *not_agg_check;
buildACLQueries(acl_subquery, racl_subquery, initacl_subquery,
- initracl_subquery, "p.proacl",
"p.proowner", "'f'",
- dopt->binary_upgrade);
+ initracl_subquery, "p.proacl",
"p.proowner",
+ "pip.initprivs", "'f'",
dopt->binary_upgrade);
not_agg_check = (fout->remoteVersion >= 110000 ? "p.prokind <>
'a'"
: "NOT p.proisagg");
@@ -6234,13 +6258,14 @@ getTables(Archive *fout, int *numTables)
buildACLQueries(acl_subquery, racl_subquery, initacl_subquery,
initracl_subquery, "c.relacl",
"c.relowner",
+ "pip.initprivs",
"CASE WHEN c.relkind = "
CppAsString2(RELKIND_SEQUENCE)
" THEN 's' ELSE 'r'
END::\"char\"",
dopt->binary_upgrade);
buildACLQueries(attacl_subquery, attracl_subquery,
attinitacl_subquery,
- attinitracl_subquery,
"at.attacl", "c.relowner", "'c'",
- dopt->binary_upgrade);
+ attinitracl_subquery,
"at.attacl", "c.relowner",
+ "pip.initprivs", "'c'",
dopt->binary_upgrade);
appendPQExpBuffer(query,
"SELECT c.tableoid, c.oid,
c.relname, "
@@ -8238,8 +8263,8 @@ getProcLangs(Archive *fout, int *numProcLangs)
PQExpBuffer initracl_subquery = createPQExpBuffer();
buildACLQueries(acl_subquery, racl_subquery, initacl_subquery,
- initracl_subquery, "l.lanacl",
"l.lanowner", "'l'",
- dopt->binary_upgrade);
+ initracl_subquery, "l.lanacl",
"l.lanowner",
+ "pip.initprivs", "'l'",
dopt->binary_upgrade);
/* pg_language has a laninline column */
appendPQExpBuffer(query, "SELECT l.tableoid, l.oid, "
@@ -9420,8 +9445,8 @@ getForeignDataWrappers(Archive *fout, int
*numForeignDataWrappers)
PQExpBuffer initracl_subquery = createPQExpBuffer();
buildACLQueries(acl_subquery, racl_subquery, initacl_subquery,
- initracl_subquery, "f.fdwacl",
"f.fdwowner", "'F'",
- dopt->binary_upgrade);
+ initracl_subquery, "f.fdwacl",
"f.fdwowner",
+ "pip.initprivs", "'F'",
dopt->binary_upgrade);
appendPQExpBuffer(query, "SELECT f.tableoid, f.oid, f.fdwname, "
"(%s f.fdwowner) AS rolname, "
@@ -9587,8 +9612,8 @@ getForeignServers(Archive *fout, int *numForeignServers)
PQExpBuffer initracl_subquery = createPQExpBuffer();
buildACLQueries(acl_subquery, racl_subquery, initacl_subquery,
- initracl_subquery, "f.srvacl",
"f.srvowner", "'S'",
- dopt->binary_upgrade);
+ initracl_subquery, "f.srvacl",
"f.srvowner",
+ "pip.initprivs", "'S'",
dopt->binary_upgrade);
appendPQExpBuffer(query, "SELECT f.tableoid, f.oid, f.srvname, "
"(%s f.srvowner) AS rolname, "
@@ -9734,6 +9759,7 @@ getDefaultACLs(Archive *fout, int *numDefaultACLs)
buildACLQueries(acl_subquery, racl_subquery, initacl_subquery,
initracl_subquery, "defaclacl",
"defaclrole",
+ "pip.initprivs",
"CASE WHEN defaclobjtype = 'S'
THEN 's' ELSE defaclobjtype END::\"char\"",
dopt->binary_upgrade);
@@ -10351,9 +10377,19 @@ dumpNamespace(Archive *fout, NamespaceInfo *nspinfo)
qnspname = pg_strdup(fmtId(nspinfo->dobj.name));
- appendPQExpBuffer(delq, "DROP SCHEMA %s;\n", qnspname);
-
- appendPQExpBuffer(q, "CREATE SCHEMA %s;\n", qnspname);
+ if (strcmp(nspinfo->dobj.name, "public") == 0)
+ {
+ /* see selectDumpableNamespace() */
+ appendPQExpBufferStr(delq,
+ "-- *not* dropping
schema, since initdb creates it\n");
+ appendPQExpBufferStr(q,
+ "-- *not* creating
schema, since initdb creates it\n");
+ }
+ else
+ {
+ appendPQExpBuffer(delq, "DROP SCHEMA %s;\n", qnspname);
+ appendPQExpBuffer(q, "CREATE SCHEMA %s;\n", qnspname);
+ }
if (dopt->binary_upgrade)
binary_upgrade_extension_member(q, &nspinfo->dobj,
@@ -15501,8 +15537,8 @@ dumpTable(Archive *fout, TableInfo *tbinfo)
PQExpBuffer initracl_subquery = createPQExpBuffer();
buildACLQueries(acl_subquery, racl_subquery,
initacl_subquery,
- initracl_subquery,
"at.attacl", "c.relowner", "'c'",
- dopt->binary_upgrade);
+ initracl_subquery,
"at.attacl", "c.relowner",
+ "pip.initprivs", "'c'",
dopt->binary_upgrade);
appendPQExpBuffer(query,
"SELECT at.attname, "
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index 1290f96..51f5c03 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -142,6 +142,7 @@ typedef struct _dumpableObject
typedef struct _namespaceInfo
{
DumpableObject dobj;
+ Oid nspowner;
char *rolname; /* name of owner, or empty string */
char *nspacl;
char *rnspacl;
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 737e464..392da3b 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -125,6 +125,14 @@ my %pgdump_runs = (
'regress_pg_dump_test',
],
},
+ defaults_public_owner => {
+ database => 'regress_public_owner',
+ dump_cmd => [
+ 'pg_dump', '--no-sync', '-f',
+ "$tempdir/defaults_public_owner.sql",
+ 'regress_public_owner',
+ ],
+ },
# Do not use --no-sync to give test coverage for data sync.
defaults_custom_format => {
@@ -605,6 +613,24 @@ my %tests = (
unlike => { no_owner => 1, },
},
+ 'ALTER SCHEMA public OWNER TO' => {
+ # see test "REVOKE CREATE ON SCHEMA public" for causative
create_sql
+ regexp => qr/^ALTER SCHEMA public OWNER TO .+;/m,
+ like => {
+ %full_runs, section_pre_data => 1,
+ },
+ unlike => { no_owner => 1, },
+ },
+
+ 'ALTER SCHEMA public OWNER TO (w/o ACL changes)' => {
+ database => 'regress_public_owner',
+ create_order => 100,
+ create_sql =>
+ 'ALTER SCHEMA public OWNER TO "regress_quoted \"" role";',
+ regexp => qr/^(GRANT|REVOKE)/m,
+ unlike => { defaults_public_owner => 1 },
+ },
+
'ALTER SEQUENCE test_table_col1_seq' => {
regexp => qr/^
\QALTER SEQUENCE dump_test.test_table_col1_seq OWNED BY
dump_test.test_table.col1;\E
@@ -940,6 +966,13 @@ my %tests = (
like => {},
},
+ 'COMMENT ON SCHEMA public' => {
+ regexp => qr/^COMMENT ON SCHEMA public IS .+;/m,
+
+ # this shouldn't ever get emitted
+ like => {},
+ },
+
'COMMENT ON TABLE dump_test.test_table' => {
create_order => 36,
create_sql => 'COMMENT ON TABLE dump_test.test_table
@@ -1370,6 +1403,18 @@ my %tests = (
},
},
+ 'CREATE ROLE regress_quoted...' => {
+ create_order => 1,
+ create_sql => 'CREATE ROLE "regress_quoted \"" role";',
+ regexp => qr/^\QCREATE ROLE "regress_quoted \""
role";\E/m,
+ like => {
+ pg_dumpall_dbprivs => 1,
+ pg_dumpall_exclude => 1,
+ pg_dumpall_globals => 1,
+ pg_dumpall_globals_clean => 1,
+ },
+ },
+
'CREATE ACCESS METHOD gist2' => {
create_order => 52,
create_sql =>
@@ -3261,6 +3306,23 @@ my %tests = (
unlike => { no_privs => 1, },
},
+ # With the exception of the public schema, we don't dump ownership
changes
+ # for objects originating at initdb. Hence, any GRANT or REVOKE
affecting
+ # owner privileges for those objects should reference the bootstrap
+ # superuser, not the dump-time owner.
+ 'REVOKE EXECUTE ON FUNCTION pg_stat_reset FROM regress_dump_test_role'
=>
+ {
+ create_order => 15,
+ create_sql => '
+ ALTER FUNCTION pg_stat_reset OWNER TO
regress_dump_test_role;
+ REVOKE EXECUTE ON FUNCTION pg_stat_reset
+ FROM regress_dump_test_role;',
+ regexp => qr/^[^-].*pg_stat_reset.* regress_dump_test_role/m,
+
+ # this shouldn't ever get emitted
+ like => {},
+ },
+
'REVOKE SELECT ON TABLE pg_proc FROM public' => {
create_order => 45,
create_sql => 'REVOKE SELECT ON TABLE pg_proc FROM public;',
@@ -3272,9 +3334,13 @@ my %tests = (
'REVOKE CREATE ON SCHEMA public FROM public' => {
create_order => 16,
- create_sql => 'REVOKE CREATE ON SCHEMA public FROM public;',
- regexp => qr/^
- \QREVOKE ALL ON SCHEMA public FROM PUBLIC;\E
+ create_sql => '
+ REVOKE CREATE ON SCHEMA public FROM public;
+ ALTER SCHEMA public OWNER TO "regress_quoted \"" role";
+ REVOKE ALL ON SCHEMA public FROM "regress_quoted \""
role";',
+ regexp => qr/^
+ \QREVOKE ALL ON SCHEMA public FROM "regress_quoted \""
role";\E
+ \n\QREVOKE ALL ON SCHEMA public FROM PUBLIC;\E
\n\QGRANT USAGE ON SCHEMA public TO PUBLIC;\E
/xm,
like => { %full_runs, section_pre_data => 1, },
@@ -3376,8 +3442,9 @@ if ($collation_check_stderr !~ /ERROR: /)
$collation_support = 1;
}
-# Create a second database for certain tests to work against
+# Create additional databases for mutations of schema public
$node->psql('postgres', 'create database regress_pg_dump_test;');
+$node->psql('postgres', 'create database regress_public_owner;');
# Start with number of command_fails_like()*2 tests below (each
# command_fails_like is actually 2 tests)
Author: Noah Misch <n...@leadboat.com>
Commit: Noah Misch <n...@leadboat.com>
Dump COMMENT ON SCHEMA public.
As we do for other attributes of the public schema, omit the COMMENT
command when its payload would match what initdb had installed. For
dumps that do carry this new COMMENT command, non-superusers restoring
them are likely to get an error.
Reviewed by FIXME.
Discussion:
https://postgr.es/m/ab48a34c-60f6-e388-502a-3e5fe46a2...@postgresfriends.org
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index e8ea385..0a3f40d 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -1565,15 +1565,12 @@ selectDumpableNamespace(NamespaceInfo *nsinfo, Archive
*fout)
{
/*
* The public schema is a strange beast that sits in a sort of
- * no-mans-land between being a system object and a user
object. We
- * don't want to dump creation or comment commands for it,
because
- * that complicates matters for non-superuser use of pg_dump.
But we
- * should dump any ownership changes, security labels, and ACL
- * changes, and of course we should dump contained objects.
pg_dump
- * ties ownership to DUMP_COMPONENT_DEFINITION. Hence, unless
the
- * owner is the default, dump a "definition" bearing just a
comment.
+ * no-mans-land between being a system object and a user object.
+ * CREATE SCHEMA would fail, so its DUMP_COMPONENT_DEFINITION
is just
+ * a comment and an indication of ownership. If the owner is
the
+ * default, that DUMP_COMPONENT_DEFINITION is superfluous.
*/
- nsinfo->dobj.dump = DUMP_COMPONENT_ALL &
~DUMP_COMPONENT_COMMENT;
+ nsinfo->dobj.dump = DUMP_COMPONENT_ALL;
if (nsinfo->nspowner == BOOTSTRAP_SUPERUSERID)
nsinfo->dobj.dump &= ~DUMP_COMPONENT_DEFINITION;
nsinfo->dobj.dump_contains = DUMP_COMPONENT_ALL;
@@ -9914,6 +9911,28 @@ dumpComment(Archive *fout, const char *type, const char
*name,
ncomments--;
}
+ /*
+ * Skip dumping the initdb-provided public schema comment, which would
+ * complicate matters for non-superuser use of pg_dump. When the DBA
has
+ * removed initdb's comment, replicate that.
+ */
+ if (strcmp(type, "SCHEMA") == 0 && strcmp(name, "public") == 0)
+ {
+ static CommentItem empty_comment = {.descr = ""};
+
+ if (ncomments == 0)
+ {
+ comments = &empty_comment;
+ ncomments = 1;
+ }
+ else if (strcmp(comments->descr, (fout->remoteVersion >= 80300 ?
+
"standard public schema" :
+
"Standard public schema")) == 0)
+ {
+ ncomments = 0;
+ }
+ }
+
/* If a comment exists, build COMMENT ON statement */
if (ncomments > 0)
{
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 392da3b..4693292 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -968,9 +968,19 @@ my %tests = (
'COMMENT ON SCHEMA public' => {
regexp => qr/^COMMENT ON SCHEMA public IS .+;/m,
+ # regress_public_owner emits this, due to create_sql of next
test
+ like => {
+ pg_dumpall_dbprivs => 1,
+ pg_dumpall_exclude => 1,
+ },
+ },
- # this shouldn't ever get emitted
- like => {},
+ 'COMMENT ON SCHEMA public IS NULL' => {
+ database => 'regress_public_owner',
+ create_order => 100,
+ create_sql => 'COMMENT ON SCHEMA public IS NULL;',
+ regexp => qr/^COMMENT ON SCHEMA public IS '';/m,
+ like => { defaults_public_owner => 1 },
},
'COMMENT ON TABLE dump_test.test_table' => {
diff --git a/src/include/catalog/pg_namespace.dat
b/src/include/catalog/pg_namespace.dat
index 2ed136b..988f1c4 100644
--- a/src/include/catalog/pg_namespace.dat
+++ b/src/include/catalog/pg_namespace.dat
@@ -18,6 +18,7 @@
{ oid => '99', oid_symbol => 'PG_TOAST_NAMESPACE',
descr => 'reserved schema for TOAST tables',
nspname => 'pg_toast', nspacl => '_null_' },
+# update dumpComment() if changing this descr
{ oid => '2200', oid_symbol => 'PG_PUBLIC_NAMESPACE',
descr => 'standard public schema',
nspname => 'public', nspacl => '_null_' },
