pg_upgrade in 9.5 broken for adminpack
pg_upgrade was recently broken for use upgrading from a system with
adminpack installed.
Breaking commit is:
commit 30982be4e5019684e1772dd9170aaa53f5a8e894
Author: Peter Eisentraut <peter_e@gmx.net>
Integrate pg_upgrade_support module into backend
from pg_upgrade_dump_12870.log
pg_restore: creating EXTENSION "adminpack"
pg_restore: creating COMMENT "EXTENSION "adminpack""
pg_restore: [archiver (db)] Error while PROCESSING TOC:
pg_restore: [archiver (db)] Error from TOC entry 2806; 0 0 COMMENT
EXTENSION "adminpack"
pg_restore: [archiver (db)] could not execute query: ERROR: extension
"adminpack" does not exist
Command was: COMMENT ON EXTENSION "adminpack" IS 'administrative
functions for PostgreSQL';
I get the same error whether the source database is 9.2.10 or 9.5.HEAD.
Cheers,
Jeff
On Thu, Apr 16, 2015 at 07:33:50PM -0700, Jeff Janes wrote:
pg_upgrade was recently broken for use upgrading from a system with adminpack
installed.Breaking commit is:
commit 30982be4e5019684e1772dd9170aaa53f5a8e894
Author: Peter Eisentraut <peter_e@gmx.net>� � Integrate pg_upgrade_support module into backend
from pg_upgrade_dump_12870.log
pg_restore: creating EXTENSION "adminpack"
pg_restore: creating COMMENT "EXTENSION "adminpack""
pg_restore: [archiver (db)] Error while PROCESSING TOC:
pg_restore: [archiver (db)] Error from TOC entry 2806; 0 0 COMMENT EXTENSION
"adminpack"
pg_restore: [archiver (db)] could not execute query: ERROR: �extension
"adminpack" does not exist
� � Command was: COMMENT ON EXTENSION "adminpack" IS 'administrative functions
for PostgreSQL';I get the same error whether the source database is 9.2.10 or 9.5.HEAD.
Uh, I am confused how moving pg_upgrade or pg_upgrade_support would
break the loading of the "adminpack" extension.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Apr 16, 2015 at 7:37 PM, Bruce Momjian <bruce@momjian.us> wrote:
On Thu, Apr 16, 2015 at 07:33:50PM -0700, Jeff Janes wrote:
pg_upgrade was recently broken for use upgrading from a system with
adminpack
installed.
Breaking commit is:
commit 30982be4e5019684e1772dd9170aaa53f5a8e894
Author: Peter Eisentraut <peter_e@gmx.net>Integrate pg_upgrade_support module into backend
from pg_upgrade_dump_12870.log
pg_restore: creating EXTENSION "adminpack"
pg_restore: creating COMMENT "EXTENSION "adminpack""
pg_restore: [archiver (db)] Error while PROCESSING TOC:
pg_restore: [archiver (db)] Error from TOC entry 2806; 0 0 COMMENTEXTENSION
"adminpack"
pg_restore: [archiver (db)] could not execute query: ERROR: extension
"adminpack" does not exist
Command was: COMMENT ON EXTENSION "adminpack" IS 'administrativefunctions
for PostgreSQL';
I get the same error whether the source database is 9.2.10 or 9.5.HEAD.
Uh, I am confused how moving pg_upgrade or pg_upgrade_support would
break the loading of the "adminpack" extension.
Yeah, I'm confused to. But I see it isn't just adminpack, it is all
extensions.
The query:
SELECT pg_catalog.binary_upgrade_create_empty_extension('adminpack',
'pg_catalog', false, '1.0', NULL, NULL, ARRAY[]::pg_catalog.text[]);
doesnt' seem to do anything. It returns NULL, but doesn't create an
extension. I set a gdb breakpoint on binary_upgrade_create_empty_extension
and it never trips when manually running the above query.
If the SQL function never calls the C function, what is it doing?
Cheers,
Jeff
On Thu, Apr 16, 2015 at 11:04 PM, Jeff Janes <jeff.janes@gmail.com> wrote:
On Thu, Apr 16, 2015 at 7:37 PM, Bruce Momjian <bruce@momjian.us> wrote:
On Thu, Apr 16, 2015 at 07:33:50PM -0700, Jeff Janes wrote:
pg_upgrade was recently broken for use upgrading from a system with
adminpack
installed.
...
The query:
SELECT pg_catalog.binary_upgrade_create_empty_extension('adminpack',
'pg_catalog', false, '1.0', NULL, NULL, ARRAY[]::pg_catalog.text[]);doesnt' seem to do anything. It returns NULL, but doesn't create an
extension. I set a gdb breakpoint on binary_upgrade_create_empty_extension
and it never trips when manually running the above query.If the SQL function never calls the C function, what is it doing?
Of course after sending that it became obvious. The C function is not
getting called because the SQL function is marked as being strict, yet is
called with NULL arguments.
Trivial patch attached to unset strict flag in pg_proc.h.
But CATALOG_VERSION_NO probably needs another bump as well.
Cheers,
Jeff
Attachments:
pg_upgrade_support_strict.patchapplication/octet-stream; name=pg_upgrade_support_strict.patchDownload
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
new file mode 100644
index 8469c82..619d996
*** a/src/include/catalog/pg_proc.h
--- b/src/include/catalog/pg_proc.h
*************** DATA(insert OID = 3589 ( binary_upgrade_
*** 5198,5204 ****
DESCR("for use by pg_upgrade");
DATA(insert OID = 3590 ( binary_upgrade_set_next_pg_authid_oid PGNSP PGUID 12 1 0 0 0 f f f f t f v 1 0 2278 "26" _null_ _null_ _null_ _null_ binary_upgrade_set_next_pg_authid_oid _null_ _null_ _null_ ));
DESCR("for use by pg_upgrade");
! DATA(insert OID = 3591 ( binary_upgrade_create_empty_extension PGNSP PGUID 12 1 0 0 0 f f f f t f v 7 0 2278 "25 25 16 25 1028 1009 1009" _null_ _null_ _null_ _null_ binary_upgrade_create_empty_extension _null_ _null_ _null_ ));
DESCR("for use by pg_upgrade");
--- 5198,5204 ----
DESCR("for use by pg_upgrade");
DATA(insert OID = 3590 ( binary_upgrade_set_next_pg_authid_oid PGNSP PGUID 12 1 0 0 0 f f f f t f v 1 0 2278 "26" _null_ _null_ _null_ _null_ binary_upgrade_set_next_pg_authid_oid _null_ _null_ _null_ ));
DESCR("for use by pg_upgrade");
! DATA(insert OID = 3591 ( binary_upgrade_create_empty_extension PGNSP PGUID 12 1 0 0 0 f f f f f f v 7 0 2278 "25 25 16 25 1028 1009 1009" _null_ _null_ _null_ _null_ binary_upgrade_create_empty_extension _null_ _null_ _null_ ));
DESCR("for use by pg_upgrade");
On Fri, Apr 17, 2015 at 3:29 PM, Jeff Janes wrote:
Of course after sending that it became obvious. The C function is not
getting called because the SQL function is marked as being strict, yet is
called with NULL arguments.Trivial patch attached to unset strict flag in pg_proc.h.
But CATALOG_VERSION_NO probably needs another bump as well.
This looks good to me. I have tested your fix and the problem goes
away. create_empty_extension() was the only function not declared as
STRICT before 30982be4 (see install_support_functions_in_new_db() in
contrib/pg_upgrade/function.c).
Regards,
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Apr 17, 2015 at 04:11:44PM +0900, Michael Paquier wrote:
On Fri, Apr 17, 2015 at 3:29 PM, Jeff Janes wrote:
Of course after sending that it became obvious. The C function is not
getting called because the SQL function is marked as being strict, yet is
called with NULL arguments.Trivial patch attached to unset strict flag in pg_proc.h.
But CATALOG_VERSION_NO probably needs another bump as well.
This looks good to me. I have tested your fix and the problem goes
away. create_empty_extension() was the only function not declared as
STRICT before 30982be4 (see install_support_functions_in_new_db() in
contrib/pg_upgrade/function.c).
OK, as I am not the author of that change, I am going to wait a little
while for the author to commit the fix. I can see it is an easy mistake
to make.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Apr 16, 2015 at 11:29:07PM -0700, Jeff Janes wrote:
Of course after sending that it became obvious.� The C function is not getting
called because the SQL function is marked as being strict, yet is called with
NULL arguments.Trivial patch attached to unset strict flag in pg_proc.h.
But��CATALOG_VERSION_NO probably needs another bump as well.
Patch applied and catversion bumped. Thanks.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Bruce Momjian writes:
On Thu, Apr 16, 2015 at 11:29:07PM -0700, Jeff Janes wrote:
Of course after sending that it became obvious.� The C function is not getting
called because the SQL function is marked as being strict, yet is called with
NULL arguments.Trivial patch attached to unset strict flag in pg_proc.h.
But��CATALOG_VERSION_NO probably needs another bump as well.
Patch applied and catversion bumped. Thanks.
Shouldn't there be some validation of arguments now that the function is
no longer marked strict? Currently, unprivileged users can crash the
server calling binary_upgrade_create_empty_extension with null
arguments. Found using sqlsmith.
regards,
Andreas
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Andreas Seltenreich <seltenreich@gmx.de> writes:
Shouldn't there be some validation of arguments now that the function is
no longer marked strict? Currently, unprivileged users can crash the
server calling binary_upgrade_create_empty_extension with null
arguments. Found using sqlsmith.
Good catch, thanks! Will fix.
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