Re: creating extension including dependencies

On 2015-07-25 14:37, Michael Paquier wrote:
> On Sat, Jul 25, 2015 at 12:59 AM, Petr Jelinek <petr@2ndquadrant.com> wrote:
>> On 2015-07-22 07:12, Michael Paquier wrote:
>>>
>>> On Tue, Jul 21, 2015 at 11:20 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>>>
>>>> Petr Jelinek <petr@2ndquadrant.com> writes:
>>>>>
>>>>> ... My main question is if we are
>>>>> ok with SCHEMA having different behavior with CASCADE vs without
>>>>> CASCADE. I went originally with "no" and added the DEFAULT flag to
>>>>> SCHEMA. If the answer is "yes" then we don't need the flag (in that case
>>>>> CASCADE acts as the flag).
>>>>
>>>>
>>>> Yeah, I was coming around to that position as well.  Insisting that
>>>> SCHEMA throw an error if the extension isn't relocatable makes sense
>>>> as long as only one extension is being considered, but once you say
>>>> CASCADE it seems like mostly a usability fail.  I think it's probably
>>>> OK if with CASCADE, SCHEMA is just "use if needed else ignore".
>>>
>>>
>>
>> Here is a patch implementing that...
>
> +          <para>
> +           If schema is not same as the one in extension's control file and
> +           the <literal>CASCADE</> parameter is not given, error will be
> +           thrown.
> +          </para>
> "If schema is not the same". Here I think that you may directly refer
> to schema_name...
>
> -               /* If the user is giving us the schema name, it must
> exist already */
> +               /* If the user is giving us the schema name, it must
> exist already. */
> Noise?

Intentional. Any back-patching there will be anyway complicated by the
change of the code couple lines bellow and above so I think it's OK to
fix the comment even if just cosmetically.

>
> With the patch:
> =# create extension adminpack schema popo2;
> ERROR:  3F000: schema "popo2" does not exist
> LOCATION:  get_namespace_oid, namespace.c:2873
> On HEAD:
> =# create extension adminpack schema popo2;
> ERROR:  0A000: extension "adminpack" must be installed in schema "pg_catalog"
> LOCATION:  CreateExtension, extension.c:1352
> Time: 0.978 ms
> It looks like a regression to me to check for the existence of the
> schema before checking if the extension is compatible with the options
> given by users (regression test welcome).
>

Yes that's what I meant by the change of checking order in the
explanation above. I did that because I thought code would be more
complicated otherwise, but apparently I was stupid...

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