pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #15112: Unable to run pg_upgrade with earthdistance extension
4+ messages / 3 participants
[nested] [flat]
* BUG #15112: Unable to run pg_upgrade with earthdistance extension
@ 2018-03-14 23:12 PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 4+ messages in thread
From: PG Bug reporting form @ 2018-03-14 23:12 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: keiko713@gmail.com
The following bug has been logged on the website:
Bug reference: 15112
Logged by: Keiko Oda
Email address: keiko713@gmail.com
PostgreSQL version: 10.3
Operating system: Ubuntu
Description:
This is similar to the bug report from 2016
(https://www.postgresql.org/message-id/20161015001424.1413.93990@wrigleys.postgresql.org).
where earthdistance extension is assuming that it's executed with public
schema in search_path.
With 10.3 release, Postgres has tighten the search_path during pg_upgrade
only to pg_catalog. This is problematic as it's now impossible to run
pg_upgrade with earthdistance extension (it fails with create index with
ll_to_earth function).
Prior to this, we were at least able to workaround by adding public schema
in search_path with pg_upgrade.
As it's recommended in the release note, earthdistance should adjust these
functions to not assume anything about what search path they are invoked
under.
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: BUG #15112: Unable to run pg_upgrade with earthdistance extension
@ 2018-03-30 20:52 Bruce Momjian <bruce@momjian.us>
parent: PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 4+ messages in thread
From: Bruce Momjian @ 2018-03-30 20:52 UTC (permalink / raw)
To: keiko713@gmail.com; pgsql-bugs@lists.postgresql.org
On Wed, Mar 14, 2018 at 11:12:26PM +0000, PG Bug reporting form wrote:
> The following bug has been logged on the website:
>
> Bug reference: 15112
> Logged by: Keiko Oda
> Email address: keiko713@gmail.com
> PostgreSQL version: 10.3
> Operating system: Ubuntu
> Description:
>
> This is similar to the bug report from 2016
> (https://www.postgresql.org/message-id/20161015001424.1413.93990@wrigleys.postgresql.org).
> where earthdistance extension is assuming that it's executed with public
> schema in search_path.
>
> With 10.3 release, Postgres has tighten the search_path during pg_upgrade
> only to pg_catalog. This is problematic as it's now impossible to run
> pg_upgrade with earthdistance extension (it fails with create index with
> ll_to_earth function).
> Prior to this, we were at least able to workaround by adding public schema
> in search_path with pg_upgrade.
>
> As it's recommended in the release note, earthdistance should adjust these
> functions to not assume anything about what search path they are invoked
> under.
Uh, I can reproduce this failure. :-(
I tested it by installing earchdistance (and cube) and an index using an
earchdistance function in the old cluster and ran pg_upgrade. I used
the instructions in the referenced email:
CREATE TABLE zip_code_geo_poly_data (
id serial NOT NULL PRIMARY KEY,
zip_code TEXT,
latitude NUMERIC,
longitude NUMERIC
);
CREATE INDEX zip_code_geo_poly_data_ll_to_earth
ON zip_code_geo_poly_data
USING gist(ll_to_earth(latitude, longitude));
The failure is actually in pg_dump/restore, but pg_upgrade relies on
that, so it fails too. The failure output is:
LINE 1: ...ians($1))*sin(radians($2))),earth()*sin(radians($1)))::earth
^
QUERY: SELECT cube(cube(cube(earth()*cos(radians($1))*cos(radians($2))),earth()*cos(radians($1))*sin(radians($2))),earth()*sin(radians($1)))::earth
CONTEXT: SQL function "ll_to_earth" during inlining
Command was:
-- For binary upgrade, must preserve pg_class oids
SELECT pg_catalog.binary_upgrade_set_next_index_pg_class_oid('16498'::pg_catalog.oid);
CREATE INDEX "zip_code_geo_poly_data_ll_to_earth" ON "public"."zip_code_geo_poly_data" USING "gist" ("public"."ll_to_earth"(("latitude")::double precision, ("longitude")::double precision));
The function definition of ll_to_earth() is (as defined in
earthdistance--1.1.sql):
CREATE FUNCTION ll_to_earth(float8, float8)
RETURNS earth
LANGUAGE SQL
IMMUTABLE STRICT
PARALLEL SAFE
AS 'SELECT cube(cube(cube(earth()*cos(radians($1))*cos(radians($2))),earth()*cos(radians($1))*sin(radians($2))),earth()*sin(radians($1)))::earth';
As you can see, the SQL function is doing a cast to ::earth, but in this
test case the earth cast is stored in the public schema, and the restore
doesn't reference it.
I am not sure we can fix this without requiring people to drop and
recreate such indexes. However, I am even at a loss in how to fix the
CREATE FUNCTION to reference a cast in the same schema as the function,
in this case 'public'. We can rewrite the cast to not use :: and use a
function call with schema qualification. e.g. public.earth(), but how do
we know what schema that is in, i.e. what if the extension is loaded
into a schema other than public?
FYI, earthdistance is certainly not the only case of this problem.
This is also part of a larger problem with our new schema qualification
approach. While we are qualifying where the object is to be created, we
are not always qualifying the object's contents, i.e. in this case, the
SQL function body.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ As you are, so once was I. As I am, so you will be. +
+ Ancient Roman grave inscription +
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: BUG #15112: Unable to run pg_upgrade with earthdistance extension
@ 2018-04-02 20:24 Bruce Momjian <bruce@momjian.us>
parent: Bruce Momjian <bruce@momjian.us>
0 siblings, 1 reply; 4+ messages in thread
From: Bruce Momjian @ 2018-04-02 20:24 UTC (permalink / raw)
To: keiko713@gmail.com, pgsql-bugs@lists.postgresql.org, PostgreSQL-development <pgsql-hackers@postgresql.org>
On Fri, Mar 30, 2018 at 04:52:29PM -0400, Bruce Momjian wrote:
> On Wed, Mar 14, 2018 at 11:12:26PM +0000, PG Bug reporting form wrote:
> > The following bug has been logged on the website:
> >
> > Bug reference: 15112
> > Logged by: Keiko Oda
> > Email address: keiko713@gmail.com
> > PostgreSQL version: 10.3
> > Operating system: Ubuntu
> > Description:
> >
> > This is similar to the bug report from 2016
> > (https://www.postgresql.org/message-id/20161015001424.1413.93990@wrigleys.postgresql.org).
> > where earthdistance extension is assuming that it's executed with public
> > schema in search_path.
> >
> > With 10.3 release, Postgres has tighten the search_path during pg_upgrade
> > only to pg_catalog. This is problematic as it's now impossible to run
> > pg_upgrade with earthdistance extension (it fails with create index with
> > ll_to_earth function).
> > Prior to this, we were at least able to workaround by adding public schema
> > in search_path with pg_upgrade.
> >
> > As it's recommended in the release note, earthdistance should adjust these
> > functions to not assume anything about what search path they are invoked
> > under.
>
> Uh, I can reproduce this failure. :-(
>
> I tested it by installing earchdistance (and cube) and an index using an
> earchdistance function in the old cluster and ran pg_upgrade. I used
> the instructions in the referenced email:
>
> CREATE TABLE zip_code_geo_poly_data (
> id serial NOT NULL PRIMARY KEY,
> zip_code TEXT,
> latitude NUMERIC,
> longitude NUMERIC
> );
>
> CREATE INDEX zip_code_geo_poly_data_ll_to_earth
> ON zip_code_geo_poly_data
> USING gist(ll_to_earth(latitude, longitude));
>
> The failure is actually in pg_dump/restore, but pg_upgrade relies on
> that, so it fails too. The failure output is:
>
> LINE 1: ...ians($1))*sin(radians($2))),earth()*sin(radians($1)))::earth
> ^
> QUERY: SELECT cube(cube(cube(earth()*cos(radians($1))*cos(radians($2))),earth()*cos(radians($1))*sin(radians($2))),earth()*sin(radians($1)))::earth
> CONTEXT: SQL function "ll_to_earth" during inlining
> Command was:
> -- For binary upgrade, must preserve pg_class oids
> SELECT pg_catalog.binary_upgrade_set_next_index_pg_class_oid('16498'::pg_catalog.oid);
>
> CREATE INDEX "zip_code_geo_poly_data_ll_to_earth" ON "public"."zip_code_geo_poly_data" USING "gist" ("public"."ll_to_earth"(("latitude")::double precision, ("longitude")::double precision));
>
> The function definition of ll_to_earth() is (as defined in
> earthdistance--1.1.sql):
>
> CREATE FUNCTION ll_to_earth(float8, float8)
> RETURNS earth
> LANGUAGE SQL
> IMMUTABLE STRICT
> PARALLEL SAFE
> AS 'SELECT cube(cube(cube(earth()*cos(radians($1))*cos(radians($2))),earth()*cos(radians($1))*sin(radians($2))),earth()*sin(radians($1)))::earth';
>
> As you can see, the SQL function is doing a cast to ::earth, but in this
> test case the earth cast is stored in the public schema, and the restore
> doesn't reference it.
>
> I am not sure we can fix this without requiring people to drop and
> recreate such indexes. However, I am even at a loss in how to fix the
> CREATE FUNCTION to reference a cast in the same schema as the function,
> in this case 'public'. We can rewrite the cast to not use :: and use a
> function call with schema qualification. e.g. public.earth(), but how do
> we know what schema that is in, i.e. what if the extension is loaded
> into a schema other than public?
>
> FYI, earthdistance is certainly not the only case of this problem.
>
> This is also part of a larger problem with our new schema qualification
> approach. While we are qualifying where the object is to be created, we
> are not always qualifying the object's contents, i.e. in this case, the
> SQL function body.
I have not received a reply to this so I am CCing the hackers list for
more input. (If this is not the proper procedure, please let me know.)
The only additional thoughts I have are that when we removed public from
pg_dump's search_path and decided to instead fully qualify object
creation names, I was concerned about function creation failing because
of 'public' schema object references in the function body, but someone
pointed out that we don't check function bodies during restore because
of:
SET check_function_bodies = false;
However, what I did not consider was creation of indexes based on
function bodies with 'public' schema references.
I have ran some tests and found out that only SQL functions cause such
failures. I believe this is because they are inlined during CREATE
INDEX. The attached pg_dump output file recreates the failure, and
shows that prefixing it with "public" avoids the failure, and shows that
indexing of a PL/pgSQL function doesn't fail (see SQL comments).
A more general issue with functions is that if you schema-qualify a call
to a function, you will still need all object references in the function
to reference the schema where the object is installed if they are not in
search_path, i.e. schema qualifying the object doesn't propagate that
schema qualification into the function body (functions look in
search_path, not necessarily in the schema in which they are defined).
I am not sure what to suggest, but I don't think we can ignore this
problem.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ As you are, so once was I. As I am, so you will be. +
+ Ancient Roman grave inscription +
Attachments:
schema_qual_failure.sql (application/x-sql, 1.8K) download
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: BUG #15112: Unable to run pg_upgrade with earthdistance extension
@ 2018-04-05 06:59 Noah Misch <noah@leadboat.com>
parent: Bruce Momjian <bruce@momjian.us>
0 siblings, 0 replies; 4+ messages in thread
From: Noah Misch @ 2018-04-05 06:59 UTC (permalink / raw)
To: Bruce Momjian <bruce@momjian.us>; +Cc: keiko713@gmail.com, pgsql-bugs@lists.postgresql.org, PostgreSQL-development <pgsql-hackers@postgresql.org>
On Mon, Apr 02, 2018 at 04:24:02PM -0400, Bruce Momjian wrote:
> > I am not sure we can fix this without requiring people to drop and
> > recreate such indexes. However, I am even at a loss in how to fix the
> > CREATE FUNCTION to reference a cast in the same schema as the function,
> > in this case 'public'. We can rewrite the cast to not use :: and use a
> > function call with schema qualification. e.g. public.earth(), but how do
> > we know what schema that is in, i.e. what if the extension is loaded
> > into a schema other than public?
The task is to convert it to being a non-relocatable extension that uses
@extschema@, like here:
https://www.postgresql.org/docs/devel/static/extend-extensions.html#EXTEND-EXTENSIONS-EXAMPLE
Keiko Oda, you can hack around this by modifying the problematic extension
function in each database:
CREATE OR REPLACE FUNCTION public.ll_to_earth(float8, float8)
RETURNS public.earth
LANGUAGE SQL
IMMUTABLE STRICT
PARALLEL SAFE
AS 'SELECT public.cube(public.cube(public.cube(public.earth()*cos(radians($1))*cos(radians($2))),public.earth()*cos(radians($1))*sin(radians($2))),public.earth()*sin(radians($1)))::public.earth';
There's no need to drop and recreate indexes. However, if the table is small
and nothing depends on the index, that is an easy workaround that one can
employ right now. (Drop before upgrade and re-create after upgrade.)
> > FYI, earthdistance is certainly not the only case of this problem.
True; I've been expecting a report like this for contrib/xml2 especially.
^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2018-04-05 06:59 UTC | newest]
Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2018-03-14 23:12 BUG #15112: Unable to run pg_upgrade with earthdistance extension PG Bug reporting form <noreply@postgresql.org>
2018-03-30 20:52 ` Bruce Momjian <bruce@momjian.us>
2018-04-02 20:24 ` Bruce Momjian <bruce@momjian.us>
2018-04-05 06:59 ` Noah Misch <noah@leadboat.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox