pgsql: Fix pg_dump to dump shell types.
Fix pg_dump to dump shell types.
Per discussion, it really ought to do this. The original choice to
exclude shell types was probably made in the dark ages before we made
it harder to accidentally create shell types; but that was in 7.3.
Also, cause the standard regression tests to leave a shell type behind,
for convenience in testing the case in pg_dump and pg_upgrade.
Back-patch to all supported branches.
Branch
------
REL9_0_STABLE
Details
-------
http://git.postgresql.org/pg/commitdiff/5d175be17b46374f7aaf15b2fc6b0ac5d75a3467
Modified Files
--------------
src/bin/pg_dump/pg_dump.c | 68 ++++++++++++++++++++++++++---
src/bin/pg_dump/pg_dump.h | 2 +-
src/test/regress/expected/create_type.out | 2 +
src/test/regress/sql/create_type.sql | 3 ++
4 files changed, 68 insertions(+), 7 deletions(-)
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
On 08/04/2015 07:34 PM, Tom Lane wrote:
Fix pg_dump to dump shell types.
Per discussion, it really ought to do this. The original choice to
exclude shell types was probably made in the dark ages before we made
it harder to accidentally create shell types; but that was in 7.3.Also, cause the standard regression tests to leave a shell type behind,
for convenience in testing the case in pg_dump and pg_upgrade.Back-patch to all supported branches.
Still not quite there. If either 9.0 or 9.1 is upgraded to 9.2 or later,
they fail like this:
pg_restore: creating TYPE "public"."myshell"
pg_restore: setting owner and privileges for TYPE "public"."myshell"
pg_restore: setting owner and privileges for ACL "public"."myshell"
pg_restore: [archiver (db)] Error from TOC entry 4293; 0 0 ACL
myshell buildfarm
pg_restore: [archiver (db)] could not execute query: ERROR: type
"myshell" is only a shell
Command was: REVOKE ALL ON TYPE "myshell" FROM PUBLIC;
REVOKE ALL ON TYPE "myshell" FROM "buildfarm";
GRANT ALL ON TYPE "myshell" TO PUBL...
We could just declare that we don't support this for versions older than
9.2, in which case I would just remove the type from the test database
before testing cross-version upgrade. But if it's easily fixed then
let's do it.
cheers
andrew
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
Andrew Dunstan <andrew@dunslane.net> writes:
Still not quite there. If either 9.0 or 9.1 is upgraded to 9.2 or later,
they fail like this:
pg_restore: creating TYPE "public"."myshell"
pg_restore: setting owner and privileges for TYPE "public"."myshell"
pg_restore: setting owner and privileges for ACL "public"."myshell"
pg_restore: [archiver (db)] Error from TOC entry 4293; 0 0 ACL
myshell buildfarm
pg_restore: [archiver (db)] could not execute query: ERROR: type
"myshell" is only a shell
Command was: REVOKE ALL ON TYPE "myshell" FROM PUBLIC;
REVOKE ALL ON TYPE "myshell" FROM "buildfarm";
GRANT ALL ON TYPE "myshell" TO PUBL...
We could just declare that we don't support this for versions older than
9.2, in which case I would just remove the type from the test database
before testing cross-version upgrade. But if it's easily fixed then
let's do it.
It looks to me like the reason for this is that pg_dump forces the
"typacl" of a type to be '{=U}' when reading the schema data for a
pre-9.2 type, rather than reading it as NULL (ie default permissions)
which would result in not printing any grant/revoke commands for
the object.
I do not see a good reason for that; quite aside from this problem,
it means there is one more place that knows the default permissions
for a type than there needs to be. Peter, what was the rationale?
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 8/9/15 6:23 PM, Tom Lane wrote:
It looks to me like the reason for this is that pg_dump forces the
"typacl" of a type to be '{=U}' when reading the schema data for a
pre-9.2 type, rather than reading it as NULL (ie default permissions)
which would result in not printing any grant/revoke commands for
the object.I do not see a good reason for that; quite aside from this problem,
it means there is one more place that knows the default permissions
for a type than there needs to be. Peter, what was the rationale?
This was probably just copied from how proacl and lanacl are handled,
which predate typacl by quite a bit. Maybe there was a reason in those
days.
It might also have something to do with how owner privileges are
handled. An explicit '{=U}' doesn't create owner privileges, unlike a
null value in that field. Maybe this is necessary if you dump and
restore between databases with different user names.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Peter Eisentraut <peter_e@gmx.net> writes:
On 8/9/15 6:23 PM, Tom Lane wrote:
It looks to me like the reason for this is that pg_dump forces the
"typacl" of a type to be '{=U}' when reading the schema data for a
pre-9.2 type, rather than reading it as NULL (ie default permissions)
which would result in not printing any grant/revoke commands for
the object.I do not see a good reason for that; quite aside from this problem,
it means there is one more place that knows the default permissions
for a type than there needs to be. Peter, what was the rationale?
This was probably just copied from how proacl and lanacl are handled,
which predate typacl by quite a bit. Maybe there was a reason in those
days.
Hm ... I wonder whether those are well-thought-out either.
It might also have something to do with how owner privileges are
handled. An explicit '{=U}' doesn't create owner privileges, unlike a
null value in that field. Maybe this is necessary if you dump and
restore between databases with different user names.
But now that you mention it, isn't that completely broken? What pg_dump
actually prints given this made-up data is
REVOKE ALL ON TYPE myshell FROM PUBLIC;
REVOKE ALL ON TYPE myshell FROM postgres;
GRANT ALL ON TYPE myshell TO PUBLIC;
which seems like a completely insane interpretation. There is no way
that dumping a type from a pre-typacl database and restoring it into
a newer one should end up with the type's owner having no privileges
on it. I'm astonished that we've not gotten bug reports about that.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
I wrote:
But now that you mention it, isn't that completely broken? What pg_dump
actually prints given this made-up data is
REVOKE ALL ON TYPE myshell FROM PUBLIC;
REVOKE ALL ON TYPE myshell FROM postgres;
GRANT ALL ON TYPE myshell TO PUBLIC;
which seems like a completely insane interpretation. There is no way
that dumping a type from a pre-typacl database and restoring it into
a newer one should end up with the type's owner having no privileges
on it. I'm astonished that we've not gotten bug reports about that.
... of course, the reason for no field reports is that the owner still
has privileges, as she inherits them from PUBLIC. Doesn't make this
any less broken though.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
I wrote:
Peter Eisentraut <peter_e@gmx.net> writes:
This was probably just copied from how proacl and lanacl are handled,
which predate typacl by quite a bit. Maybe there was a reason in those
days.
Hm ... I wonder whether those are well-thought-out either.
They're not. Testing with ancient servers shows that we dump very silly
grant/revoke state for functions and languages as well, if the source
server is too old to have proacl or lanacl (ie, pre-7.3). As with typacl,
the silliness is accidentally masked as long as the owner doesn't do
something like revoke the privileges granted to PUBLIC.
Things work far more sanely with the attached patch, to wit we just leave
all object privileges as default if dumping from a version too old to have
privileges on that type of object. I think we should back-patch this into
all supported branches; it's considerably more likely that older branches
would be used to dump from ancient servers.
regards, tom lane
Attachments:
fix-dumping-of-old-privileges.patchtext/x-diff; charset=us-ascii; name=fix-dumping-of-old-privileges.patchDownload
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 51b6d3a..87dadbf 100644
*** a/src/bin/pg_dump/pg_dump.c
--- b/src/bin/pg_dump/pg_dump.c
*************** getTypes(Archive *fout, int *numTypes)
*** 3513,3519 ****
else if (fout->remoteVersion >= 80300)
{
appendPQExpBuffer(query, "SELECT tableoid, oid, typname, "
! "typnamespace, '{=U}' AS typacl, "
"(%s typowner) AS rolname, "
"typinput::oid AS typinput, "
"typoutput::oid AS typoutput, typelem, typrelid, "
--- 3513,3519 ----
else if (fout->remoteVersion >= 80300)
{
appendPQExpBuffer(query, "SELECT tableoid, oid, typname, "
! "typnamespace, NULL AS typacl, "
"(%s typowner) AS rolname, "
"typinput::oid AS typinput, "
"typoutput::oid AS typoutput, typelem, typrelid, "
*************** getTypes(Archive *fout, int *numTypes)
*** 3528,3534 ****
else if (fout->remoteVersion >= 70300)
{
appendPQExpBuffer(query, "SELECT tableoid, oid, typname, "
! "typnamespace, '{=U}' AS typacl, "
"(%s typowner) AS rolname, "
"typinput::oid AS typinput, "
"typoutput::oid AS typoutput, typelem, typrelid, "
--- 3528,3534 ----
else if (fout->remoteVersion >= 70300)
{
appendPQExpBuffer(query, "SELECT tableoid, oid, typname, "
! "typnamespace, NULL AS typacl, "
"(%s typowner) AS rolname, "
"typinput::oid AS typinput, "
"typoutput::oid AS typoutput, typelem, typrelid, "
*************** getTypes(Archive *fout, int *numTypes)
*** 3542,3548 ****
else if (fout->remoteVersion >= 70100)
{
appendPQExpBuffer(query, "SELECT tableoid, oid, typname, "
! "0::oid AS typnamespace, '{=U}' AS typacl, "
"(%s typowner) AS rolname, "
"typinput::oid AS typinput, "
"typoutput::oid AS typoutput, typelem, typrelid, "
--- 3542,3548 ----
else if (fout->remoteVersion >= 70100)
{
appendPQExpBuffer(query, "SELECT tableoid, oid, typname, "
! "0::oid AS typnamespace, NULL AS typacl, "
"(%s typowner) AS rolname, "
"typinput::oid AS typinput, "
"typoutput::oid AS typoutput, typelem, typrelid, "
*************** getTypes(Archive *fout, int *numTypes)
*** 3558,3564 ****
appendPQExpBuffer(query, "SELECT "
"(SELECT oid FROM pg_class WHERE relname = 'pg_type') AS tableoid, "
"oid, typname, "
! "0::oid AS typnamespace, '{=U}' AS typacl, "
"(%s typowner) AS rolname, "
"typinput::oid AS typinput, "
"typoutput::oid AS typoutput, typelem, typrelid, "
--- 3558,3564 ----
appendPQExpBuffer(query, "SELECT "
"(SELECT oid FROM pg_class WHERE relname = 'pg_type') AS tableoid, "
"oid, typname, "
! "0::oid AS typnamespace, NULL AS typacl, "
"(%s typowner) AS rolname, "
"typinput::oid AS typinput, "
"typoutput::oid AS typoutput, typelem, typrelid, "
*************** getAggregates(Archive *fout, DumpOptions
*** 4249,4255 ****
"CASE WHEN aggbasetype = 0 THEN 0 ELSE 1 END AS pronargs, "
"aggbasetype AS proargtypes, "
"(%s aggowner) AS rolname, "
! "'{=X}' AS aggacl "
"FROM pg_aggregate "
"where oid > '%u'::oid",
username_subquery,
--- 4249,4255 ----
"CASE WHEN aggbasetype = 0 THEN 0 ELSE 1 END AS pronargs, "
"aggbasetype AS proargtypes, "
"(%s aggowner) AS rolname, "
! "NULL AS aggacl "
"FROM pg_aggregate "
"where oid > '%u'::oid",
username_subquery,
*************** getAggregates(Archive *fout, DumpOptions
*** 4264,4270 ****
"CASE WHEN aggbasetype = 0 THEN 0 ELSE 1 END AS pronargs, "
"aggbasetype AS proargtypes, "
"(%s aggowner) AS rolname, "
! "'{=X}' AS aggacl "
"FROM pg_aggregate "
"where oid > '%u'::oid",
username_subquery,
--- 4264,4270 ----
"CASE WHEN aggbasetype = 0 THEN 0 ELSE 1 END AS pronargs, "
"aggbasetype AS proargtypes, "
"(%s aggowner) AS rolname, "
! "NULL AS aggacl "
"FROM pg_aggregate "
"where oid > '%u'::oid",
username_subquery,
*************** getFuncs(Archive *fout, DumpOptions *dop
*** 4408,4414 ****
appendPQExpBuffer(query,
"SELECT tableoid, oid, proname, prolang, "
"pronargs, proargtypes, prorettype, "
! "'{=X}' AS proacl, "
"0::oid AS pronamespace, "
"(%s proowner) AS rolname "
"FROM pg_proc "
--- 4408,4414 ----
appendPQExpBuffer(query,
"SELECT tableoid, oid, proname, prolang, "
"pronargs, proargtypes, prorettype, "
! "NULL AS proacl, "
"0::oid AS pronamespace, "
"(%s proowner) AS rolname "
"FROM pg_proc "
*************** getFuncs(Archive *fout, DumpOptions *dop
*** 4424,4430 ****
" WHERE relname = 'pg_proc') AS tableoid, "
"oid, proname, prolang, "
"pronargs, proargtypes, prorettype, "
! "'{=X}' AS proacl, "
"0::oid AS pronamespace, "
"(%s proowner) AS rolname "
"FROM pg_proc "
--- 4424,4430 ----
" WHERE relname = 'pg_proc') AS tableoid, "
"oid, proname, prolang, "
"pronargs, proargtypes, prorettype, "
! "NULL AS proacl, "
"0::oid AS pronamespace, "
"(%s proowner) AS rolname "
"FROM pg_proc "
*************** getProcLangs(Archive *fout, int *numProc
*** 6317,6323 ****
/* pg_language has a laninline column */
appendPQExpBuffer(query, "SELECT tableoid, oid, "
"lanname, lanpltrusted, lanplcallfoid, "
! "laninline, lanvalidator, lanacl, "
"(%s lanowner) AS lanowner "
"FROM pg_language "
"WHERE lanispl "
--- 6317,6323 ----
/* pg_language has a laninline column */
appendPQExpBuffer(query, "SELECT tableoid, oid, "
"lanname, lanpltrusted, lanplcallfoid, "
! "laninline, lanvalidator, lanacl, "
"(%s lanowner) AS lanowner "
"FROM pg_language "
"WHERE lanispl "
*************** getProcLangs(Archive *fout, int *numProc
*** 6329,6335 ****
/* pg_language has a lanowner column */
appendPQExpBuffer(query, "SELECT tableoid, oid, "
"lanname, lanpltrusted, lanplcallfoid, "
! "lanvalidator, lanacl, "
"(%s lanowner) AS lanowner "
"FROM pg_language "
"WHERE lanispl "
--- 6329,6335 ----
/* pg_language has a lanowner column */
appendPQExpBuffer(query, "SELECT tableoid, oid, "
"lanname, lanpltrusted, lanplcallfoid, "
! "0 AS laninline, lanvalidator, lanacl, "
"(%s lanowner) AS lanowner "
"FROM pg_language "
"WHERE lanispl "
*************** getProcLangs(Archive *fout, int *numProc
*** 6339,6345 ****
else if (fout->remoteVersion >= 80100)
{
/* Languages are owned by the bootstrap superuser, OID 10 */
! appendPQExpBuffer(query, "SELECT tableoid, oid, *, "
"(%s '10') AS lanowner "
"FROM pg_language "
"WHERE lanispl "
--- 6339,6347 ----
else if (fout->remoteVersion >= 80100)
{
/* Languages are owned by the bootstrap superuser, OID 10 */
! appendPQExpBuffer(query, "SELECT tableoid, oid, "
! "lanname, lanpltrusted, lanplcallfoid, "
! "0 AS laninline, lanvalidator, lanacl, "
"(%s '10') AS lanowner "
"FROM pg_language "
"WHERE lanispl "
*************** getProcLangs(Archive *fout, int *numProc
*** 6349,6375 ****
else if (fout->remoteVersion >= 70400)
{
/* Languages are owned by the bootstrap superuser, sysid 1 */
! appendPQExpBuffer(query, "SELECT tableoid, oid, *, "
"(%s '1') AS lanowner "
"FROM pg_language "
"WHERE lanispl "
"ORDER BY oid",
username_subquery);
}
! else if (fout->remoteVersion >= 70100)
{
/* No clear notion of an owner at all before 7.4 ... */
! appendPQExpBufferStr(query, "SELECT tableoid, oid, * FROM pg_language "
! "WHERE lanispl "
! "ORDER BY oid");
}
else
{
! appendPQExpBufferStr(query, "SELECT "
! "(SELECT oid FROM pg_class WHERE relname = 'pg_language') AS tableoid, "
! "oid, * FROM pg_language "
! "WHERE lanispl "
! "ORDER BY oid");
}
res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
--- 6351,6397 ----
else if (fout->remoteVersion >= 70400)
{
/* Languages are owned by the bootstrap superuser, sysid 1 */
! appendPQExpBuffer(query, "SELECT tableoid, oid, "
! "lanname, lanpltrusted, lanplcallfoid, "
! "0 AS laninline, lanvalidator, lanacl, "
"(%s '1') AS lanowner "
"FROM pg_language "
"WHERE lanispl "
"ORDER BY oid",
username_subquery);
}
! else if (fout->remoteVersion >= 70300)
{
/* No clear notion of an owner at all before 7.4 ... */
! appendPQExpBuffer(query, "SELECT tableoid, oid, "
! "lanname, lanpltrusted, lanplcallfoid, "
! "0 AS laninline, lanvalidator, lanacl, "
! "NULL AS lanowner "
! "FROM pg_language "
! "WHERE lanispl "
! "ORDER BY oid");
! }
! else if (fout->remoteVersion >= 70100)
! {
! appendPQExpBuffer(query, "SELECT tableoid, oid, "
! "lanname, lanpltrusted, lanplcallfoid, "
! "0 AS laninline, 0 AS lanvalidator, NULL AS lanacl, "
! "NULL AS lanowner "
! "FROM pg_language "
! "WHERE lanispl "
! "ORDER BY oid");
}
else
{
! appendPQExpBuffer(query, "SELECT "
! "(SELECT oid FROM pg_class WHERE relname = 'pg_language') AS tableoid, "
! "oid, "
! "lanname, lanpltrusted, lanplcallfoid, "
! "0 AS laninline, 0 AS lanvalidator, NULL AS lanacl, "
! "NULL AS lanowner "
! "FROM pg_language "
! "WHERE lanispl "
! "ORDER BY oid");
}
res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
*************** getProcLangs(Archive *fout, int *numProc
*** 6385,6391 ****
i_lanname = PQfnumber(res, "lanname");
i_lanpltrusted = PQfnumber(res, "lanpltrusted");
i_lanplcallfoid = PQfnumber(res, "lanplcallfoid");
- /* these may fail and return -1: */
i_laninline = PQfnumber(res, "laninline");
i_lanvalidator = PQfnumber(res, "lanvalidator");
i_lanacl = PQfnumber(res, "lanacl");
--- 6407,6412 ----
*************** getProcLangs(Archive *fout, int *numProc
*** 6401,6422 ****
planginfo[i].dobj.name = pg_strdup(PQgetvalue(res, i, i_lanname));
planginfo[i].lanpltrusted = *(PQgetvalue(res, i, i_lanpltrusted)) == 't';
planginfo[i].lanplcallfoid = atooid(PQgetvalue(res, i, i_lanplcallfoid));
! if (i_laninline >= 0)
! planginfo[i].laninline = atooid(PQgetvalue(res, i, i_laninline));
! else
! planginfo[i].laninline = InvalidOid;
! if (i_lanvalidator >= 0)
! planginfo[i].lanvalidator = atooid(PQgetvalue(res, i, i_lanvalidator));
! else
! planginfo[i].lanvalidator = InvalidOid;
! if (i_lanacl >= 0)
! planginfo[i].lanacl = pg_strdup(PQgetvalue(res, i, i_lanacl));
! else
! planginfo[i].lanacl = pg_strdup("{=U}");
! if (i_lanowner >= 0)
! planginfo[i].lanowner = pg_strdup(PQgetvalue(res, i, i_lanowner));
! else
! planginfo[i].lanowner = pg_strdup("");
if (fout->remoteVersion < 70300)
{
--- 6422,6431 ----
planginfo[i].dobj.name = pg_strdup(PQgetvalue(res, i, i_lanname));
planginfo[i].lanpltrusted = *(PQgetvalue(res, i, i_lanpltrusted)) == 't';
planginfo[i].lanplcallfoid = atooid(PQgetvalue(res, i, i_lanplcallfoid));
! planginfo[i].laninline = atooid(PQgetvalue(res, i, i_laninline));
! planginfo[i].lanvalidator = atooid(PQgetvalue(res, i, i_lanvalidator));
! planginfo[i].lanacl = pg_strdup(PQgetvalue(res, i, i_lanacl));
! planginfo[i].lanowner = pg_strdup(PQgetvalue(res, i, i_lanowner));
if (fout->remoteVersion < 70300)
{
On 08/10/2015 07:29 PM, Tom Lane wrote:
I wrote:
Peter Eisentraut <peter_e@gmx.net> writes:
This was probably just copied from how proacl and lanacl are handled,
which predate typacl by quite a bit. Maybe there was a reason in those
days.Hm ... I wonder whether those are well-thought-out either.
They're not. Testing with ancient servers shows that we dump very silly
grant/revoke state for functions and languages as well, if the source
server is too old to have proacl or lanacl (ie, pre-7.3). As with typacl,
the silliness is accidentally masked as long as the owner doesn't do
something like revoke the privileges granted to PUBLIC.Things work far more sanely with the attached patch, to wit we just leave
all object privileges as default if dumping from a version too old to have
privileges on that type of object. I think we should back-patch this into
all supported branches; it's considerably more likely that older branches
would be used to dump from ancient servers.
FYI this has fixed the problem that was encountered in cross-version
upgrade testing.
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers