pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Bruce Momjian <bruce@momjian.us>
To: keiko713@gmail.com, pgsql-bugs@lists.postgresql.org, PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: BUG #15112: Unable to run pg_upgrade with earthdistance extension
Date: Mon, 2 Apr 2018 16:24:02 -0400
Message-ID: <20180402202402.GA24073@momjian.us> (raw)
In-Reply-To: <20180330205229.GS8476@momjian.us>
References: <152106914669.1223.5104148605998271987@wrigleys.postgresql.org>
	<20180330205229.GS8476@momjian.us>

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

view thread (4+ messages)  latest in thread

Message-ID: <20180402202402.GA24073@momjian.us>
Permalink:  ../20180402202402.GA24073@momjian.us/
Also on:    postgresql.org/message-id/20180402202402.GA24073@momjian.us

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-bugs@postgresql.org
  Cc: bruce@momjian.us, pgsql-hackers@postgresql.org
  Subject: Re: BUG #15112: Unable to run pg_upgrade with earthdistance extension
  In-Reply-To: <20180402202402.GA24073@momjian.us>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

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