On Sat, Jul 25, 2015 at 12:59 AM, Petr Jelinek <p...@2ndquadrant.com> wrote: > On 2015-07-22 07:12, Michael Paquier wrote: >> >> On Tue, Jul 21, 2015 at 11:20 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: >>> >>> Petr Jelinek <p...@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. Note that the checks are now done in > different order for non-relocatable extension when SCHEMA is specified than > previously. Before the patch, the SCHEMA was first checked for conflict with > the extension's schema and then there was check for schema existence. This > patch always checks for schema existence first, mainly to keep code saner > (to my eyes).
+ In case the extension specifies schema in its control file, the schema + can't be overriden using <literal>SCHEMA</> parameter. The actual + behavior of the <literal>SCHEMA</> parameter in this case will depend + on circumstances: SCHEMA is not a parameter, it is a clause. Same for CASCADE. Also "schema" here should use the markup structfield as it is referenced as the parameter of a control file. + <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? +# test_ddl_deparse must be first +REGRESS = test_extensions Why is test_ddl_deparse mentioned here? 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). -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
