Re: creating extension including dependencies

On 2015-07-15 06:07, Michael Paquier wrote:
> On Fri, Jul 10, 2015 at 11:28 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Andres Freund <andres@anarazel.de> writes:
>>> On July 10, 2015 4:16:59 PM GMT+02:00, Tom Lane <tgl@sss.pgh.pa.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).

>
> I just had a look at this patch, and here are some comments:
> +             [ RECURSIVE ]
> After reading the thread, CASCADE sounds like a good thing as well to me.

Yep, got that much :)

>
> +                               /* Create and execute new CREATE
> EXTENSION statement. */
> +                               ces = makeNode(CreateExtensionStmt);
> +                               ces->extname = curreq;
> +                               ces->if_not_exists = false;
> +                               parents =
> lappend(list_copy(recursive_parents), stmt->extname);
> +                               ces->options =
> list_make1(makeDefElem("recursive",
> +
>                                     (Node *) parents));
> +                               CreateExtension(ces);
> +                               list_free(parents);
> ces should be free'd after calling CreateExtension perhaps?
>

Yeah and the exercise with list_copy and list_free on parents is
probably not needed.

> The test_ext*--*.sql files should not be completely empty. They should
> include a header like this one (hoge is the Japanese foo...):
> /* src/test/modules/test_extension/hoge--1.0.sql */
> -- complain if script is sourced in psql, rather than via CREATE EXTENSION
> \echo Use "CREATE EXTENSION hoge" to load this file. \quit
>

Done.


> 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.

> Regression tests of contrib/ modules doing transforms should be
> updated to use this new stuff IMO. That's not part of the core patch
> obviously, but it looks worth including them as well.
>

Done.


--
  Petr Jelinek                  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training & Services