Re: creating extension including dependencies - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: creating extension including dependencies
Date
Msg-id CAB7nPqRG65P7NsdXzASQtQ-utgvVOo5QEnFU1LZc8Cj=dTHsyQ@mail.gmail.com
Whole thread Raw
In response to Re: creating extension including dependencies  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: creating extension including dependencies  (Petr Jelinek <petr@2ndquadrant.com>)
List pgsql-hackers
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 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.

+                               /* 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?

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

That is good practice compared to the other modules, and this way
there is no need to have Makefile for example touch'ing those files
before installing them (I have created them manually to test this
patch).

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.

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



pgsql-hackers by date:

Previous
From: Ted Toth
Date:
Subject: Re: security labels on databases are bad for dump & restore
Next
From: Kouhei Kaigai
Date:
Subject: Re: security labels on databases are bad for dump & restore