Re: BUG #15112: Unable to run pg_upgrade with earthdistance extension

From: Bruce Momjian <bruce(at)momjian(dot)us>
To: keiko713(at)gmail(dot)com, pgsql-bugs(at)lists(dot)postgresql(dot)org
Subject: Re: BUG #15112: Unable to run pg_upgrade with earthdistance extension
Date: 2018-03-30 20:52:29
Message-ID: 20180330205229.GS8476@momjian.us
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-bugs pgsql-hackers

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(at)gmail(dot)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(at)momjian(dot)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 +

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2018-03-30 21:09:15 Re: Parallel safety of binary_upgrade_create_empty_extension
Previous Message Matheus de Oliveira 2018-03-30 20:51:28 Re: [PATCH] btree_gin, add support for uuid, bool, name, bpchar and anyrange types

Browse pgsql-bugs by date

  From Date Subject
Next Message Andres Freund 2018-03-31 00:19:46 Re: BUG #14941: Vacuum crashes
Previous Message Bossart, Nathan 2018-03-30 18:39:01 Re: BUG #14941: Vacuum crashes