Date: Mon, 9 Jul 2018 18:43:08 -0700
From: Noah Misch <noah@leadboat.com>
To: Bruce Momjian <bruce@momjian.us>
Cc: keiko713@gmail.com, pgsql-bugs@lists.postgresql.org,
	PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Extension relocation vs. schema qualification
Message-ID: <20180710014308.GA805781@rfd.leadboat.com>
References: <152106914669.1223.5104148605998271987@wrigleys.postgresql.org>
 <20180330205229.GS8476@momjian.us>
 <20180402202402.GA24073@momjian.us>
 <20180405065957.GA2276534@rfd.leadboat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20180405065957.GA2276534@rfd.leadboat.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Precedence: bulk

On Wed, Apr 04, 2018 at 11:59:57PM -0700, Noah Misch wrote:
> 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

Extension "earthdistance" creates sql-language functions that call functions
of extension "cube", which may appear outside @extschema@ and may relocate at
any moment.  Hence, this case is not as simple as using @extschema@.  While
the cube/earthdistance system happens to be revealing this problem, it would
arise in many cases of a function referring to an object of a relocatable
extension.  I see these options:

1. Stop using relocatable=true in core extensions (by adding a new version
   number and versioned control file).  To relocate an extension, drop and
   recreate it.  Deprecate relocatable=true.  Sub-options:

   1a. Require that "earthdistance" and "cube" appear in the same schema by
   forcing an error[1] when they don't.

   1b. Expand @DEPNAME_schema@ in extension SQL files.  Use @cube_schema@ to
   refer to the right objects.

   1c. Use plpgsql to query pg_extension.extnamespace, then EXECUTE a CREATE
   FUNCTION statement after substituting the right schema names.

2. Like (1), including all sub-options, but warn about the problem without
   deprecating relocatable=true.  Drop relocatable=true from extensions that
   have cause to do so: cube, earthdistance, pageinspect, pg_freespacemap,
   xml2.  Do likewise for others as needed in the future.

3. Make "earthdistance" dynamically discover the location of "cube" during
   each function call.  This entails rewriting earthdistance sql-language
   functions in C.  (One could use plpgsql, but that would add a substantial
   performance loss and a runtime dependency.)

4. Re-implement the earthdistance sql functions in C, not calling "cube"
   functions at all.

5. Create copies in "earthdistance" of the "cube" functions it uses[2].  This
   violates modularity.  It makes \dx+ uglier.

6. Allow an extension to ship SQL commands for re-binding to schemas when it
   or a dependency relocates.  This would allow relocatable=true in extensions
   that refer to @extschema@.  Include (1b) in this project.

7. Augment function system and LANGUAGE sql to offer the ability to parse at
   CREATE time, storing a Query tree like we do for views/rules.  (This would
   be a complex feature.)  (One can simulate this today with a rule[3], but it
   defeats inline_function().)

Overall, I lean toward (2b).  It's a self-contained project that doesn't
uglify contrib and that sets a reasonable example for non-core extensions.
While (7) would solve this and other problems nicely, it's a poor back-patch
candidate.  I liked (1b) for awhile, but it would be overkill if we ever get
(7).  Other ideas or preferences?

Thanks,
nm

[1] SELECT 'cube and earthdistance must appear in the same schema',
    1 / (count(DISTINCT extnamespace) = 1)::int
    FROM pg_extension WHERE extname IN ('cube', 'earthdistance');

[2] CREATE FUNCTION _earthdistance_cube(float8) RETURNS cube
    AS '$libdir/cube', 'cube_f8'
    LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE;

[3] CREATE TABLE sum_scratch (addend0 int, addend1 int, sum int);
    CREATE VIEW sum_impl AS SELECT * FROM sum_scratch;
    CREATE RULE sum_calc AS
     ON INSERT TO sum_impl DO INSTEAD INSERT INTO sum_scratch
     VALUES (NEW.addend0, NEW.addend1, NEW.addend0 + NEW.addend1) RETURNING *;
    CREATE FUNCTION sum(int, int) RETURNS int LANGUAGE sql AS
     $$INSERT INTO sum_impl VALUES ($1, $2) RETURNING sum$$;
    SELECT sum(4, 3);