Re: creating extension including dependencies

On Sat, Jul 18, 2015 at 8:00 AM, Petr Jelinek <petr(at)2ndquadrant(dot)com> wrote:
> On 2015-07-15 06:07, Michael Paquier wrote:
>>
>> On Fri, Jul 10, 2015 at 11:28 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>>
>>> Andres Freund <andres(at)anarazel(dot)de> writes:
>>>>
>>>> On July 10, 2015 4:16:59 PM GMT+02:00, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
>>>> wrote:
>>>>>
>>>>> Would that propagate down through multiple levels of CASCADE?
>>>>> (Although
>>>>> I'm not sure it would be sensible for a non-relocatable extension to
>>>>> depend on a relocatable one, so maybe the need doesn't arise in
>>>>> practice.)
>>>
>>>
>>>> I'd day so. I was thinking it'd adding a flag that allows to pass a
>>>> schema to a non relocatable extension. That'd then be passed down. I
>>>> agree that it's unlikely to be used often.
>>>
>>>
>>> Yeah, I was visualizing it slightly differently, as a separate
>>> internal-only option "schema_if_needed", but it works out to the
>>> same thing either way.
>
>
> I added DEFAULT SCHEMA option into CREATE EXTENSION which behaves like
> SCHEMA but only for extension that don't specify their schema and is
> mutually exclusive with just SCHEMA. The DEFAULT SCHEMA propagates when
> CASCADE is used while the SCHEMA option does not propagate. I'd like to hear
> opinions about this behavior. It would be possible to propagate SCHEMA as
> DEFAULT SCHEMA but that might have surprising results (installing
> dependencies in same schema as the current ext).

Hm...

First, the current patch has a surprising behavior because it fails to
create an extension in cascade when creation is attempted on a custom
schema:
=# create schema po;
CREATE SCHEMA
=# create extension hstore_plperl with schema po cascade;
NOTICE: 00000: installing required extension "hstore"
LOCATION: CreateExtension, extension.c:1483
NOTICE: 00000: installing required extension "plperl"
LOCATION: CreateExtension, extension.c:1483
ERROR: 42704: type "hstore" does not exist
LOCATION: typenameType, parse_type.c:258
Time: 30.071 ms
To facilitate the life of users, I think that the parent extensions
should be created on the same schema as its child by default. In this
case hstore should be created in po, not public.

And actually by looking at the code I can guess that when DEFAULT
SCHEMA is used and that a non-relocatable extension with a schema
defined is created in cascade this will error-out as well. That's not
really user-friendly..

Hence, as a schema can only be specified in a control file for a
non-relocatable extension, I think that the schema name propagated to
the root extensions should be the one specified in the control file of
the child if it is defined in it instead of the one specified by user
(imagine for example an extension created in cascade that requires
adminpack, extension that can only be deployed in pg_catalog), and
have the root node use its own schema if it has one in its control
file by default.

For example in this case:
foo1 (WITH SCHEMA hoge) -----> foo2 (schema = pg_catalog) -----> foo2_1
|
|--> foo2_2
---> foo3 (no schema)
With this command:
CREATE EXTENSION foo1 WITH SCHEMA hoge;
foo3 is created on schema po. foo2, as well its required dependencies
foo2_1 and foo2_2 are created on pg_catalog.

Now DEFAULT SCHEMA is trying to achieve: "Hey, I want to define foo2_1
and foo2_2 on schema hoge". This may be worth achieving (though IMO I
think that foo1 should have direct dependencies with foo2_1 and
foo2_2), but I think that we should decide what is the default
behavior we want, and implement the additional behavior in a second
patch, separated from the patch of this thread. Personally I am more a
fan of propagating to parent extensions the schema of the child and
not of its grand-child by default, but I am not alone here :)

>> The list of contrib modules excluded from build in the case of MSVC
>> needs to include test_extensions ($contrib_excludes in
>> src/tools/msvc/Mkvcbuild.pm) or build on Windows using MS of VC will
>> fail. commit_ts does that for example.
>>
>
> Done, hopefully correctly, I don't have access to MSVC.

That's done correctly.

+ /*
+ * Propagate the CASCADE option and
add current extenssion
+ * to the list for checking the cyclic
dependencies.
+ */
s/extenssion/extension/

+ /* Check for cyclic dependency between
extension. */
s/extension/extensions/

psql tab completion should be completed with cascade. See the part
with WITH SCHEMA in tab-complete.c.

Regards,
--
Michael