Обсуждение: 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. +
On Thu, Apr 16, 2015 at 7:37 PM, Bruce Momjian <bruce@momjian.us> wrote:
Uh, I am confused how moving pg_upgrade or pg_upgrade_support wouldOn 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.
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
Вложения
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
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. +
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. +
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
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