Re: creating extension including dependencies

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Petr Jelinek <petr(at)2ndquadrant(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: creating extension including dependencies
Date: 2015-07-15 04:07:23
Message-ID: CAB7nPqRG65P7NsdXzASQtQ-utgvVOo5QEnFU1LZc8Cj=dTHsyQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kouhei Kaigai 2015-07-15 04:09:56 Re: security labels on databases are bad for dump & restore
Previous Message Ted Toth 2015-07-15 03:43:49 Re: security labels on databases are bad for dump & restore