Re: [HACKERS] creating extension including dependencies

2015-10-03 Thread Petr Jelinek

On 2015-10-03 17:16, Andres Freund wrote:

On 2015-10-03 17:15:54 +0200, Andres Freund wrote:

Here's an updated patch. Petr, could you please expand the test to
handle a bit more complex cascading setups?




Okay, I changed the test to make the dependencies bit more complex - 
more than one dependency per extension + also non-cyclic 
interdependency. It still works as expected.


--
 Petr Jelinek  http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
>From 54ce5b981e6d0009572562a989a287371a8c7146 Mon Sep 17 00:00:00 2001
From: Petr Jelinek 
Date: Sat, 3 Oct 2015 17:48:13 +0200
Subject: [PATCH] Add CASCADE support for CREATE EXTENSION.

Without CASCADE, if an extension has an unfullfilled dependency on
another extension, CREATE EXTENSION ERRORs out with "required extension
... is not installed". That is annoying, especially when that dependency
is an implementation detail of the extension, rather than something the
extension's user can make sense of.

In addition to CASCADE this also includes a small set of regression
tests around CREATE EXTENSION.

Author: Petr Jelinek, editorialized by Michael Paquier, Andres Freund
Reviewed-By: Michael Paquier, Andres Freund, Jeff Janes
Discussion: 557e0520.3040...@2ndquadrant.com
---
 contrib/hstore_plperl/expected/hstore_plperl.out   |   6 +-
 contrib/hstore_plperl/expected/hstore_plperlu.out  |   6 +-
 contrib/hstore_plperl/sql/hstore_plperl.sql|   4 +-
 contrib/hstore_plperl/sql/hstore_plperlu.sql   |   4 +-
 .../hstore_plpython/expected/hstore_plpython.out   |   4 +-
 contrib/hstore_plpython/sql/hstore_plpython.sql|   3 +-
 contrib/ltree_plpython/expected/ltree_plpython.out |   4 +-
 contrib/ltree_plpython/sql/ltree_plpython.sql  |   3 +-
 doc/src/sgml/ref/create_extension.sgml |  42 
 src/backend/commands/extension.c   | 217 ++---
 src/backend/parser/gram.y  |   4 +
 src/bin/psql/tab-complete.c|   7 +-
 src/test/modules/Makefile  |   1 +
 src/test/modules/test_extensions/.gitignore|   4 +
 src/test/modules/test_extensions/Makefile  |  23 +++
 .../test_extensions/expected/test_extensions.out   |  37 
 .../test_extensions/sql/test_extensions.sql|  15 ++
 .../modules/test_extensions/test_ext1--1.0.sql |   3 +
 src/test/modules/test_extensions/test_ext1.control |   5 +
 .../modules/test_extensions/test_ext2--1.0.sql |   3 +
 src/test/modules/test_extensions/test_ext2.control |   4 +
 .../modules/test_extensions/test_ext3--1.0.sql |   3 +
 src/test/modules/test_extensions/test_ext3.control |   3 +
 .../modules/test_extensions/test_ext4--1.0.sql |   3 +
 src/test/modules/test_extensions/test_ext4.control |   4 +
 .../modules/test_extensions/test_ext5--1.0.sql |   3 +
 src/test/modules/test_extensions/test_ext5.control |   3 +
 .../test_extensions/test_ext_cyclic1--1.0.sql  |   3 +
 .../test_extensions/test_ext_cyclic1.control   |   4 +
 .../test_extensions/test_ext_cyclic2--1.0.sql  |   3 +
 .../test_extensions/test_ext_cyclic2.control   |   4 +
 src/tools/msvc/Mkvcbuild.pm|   3 +-
 32 files changed, 346 insertions(+), 89 deletions(-)
 create mode 100644 src/test/modules/test_extensions/.gitignore
 create mode 100644 src/test/modules/test_extensions/Makefile
 create mode 100644 src/test/modules/test_extensions/expected/test_extensions.out
 create mode 100644 src/test/modules/test_extensions/sql/test_extensions.sql
 create mode 100644 src/test/modules/test_extensions/test_ext1--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext1.control
 create mode 100644 src/test/modules/test_extensions/test_ext2--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext2.control
 create mode 100644 src/test/modules/test_extensions/test_ext3--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext3.control
 create mode 100644 src/test/modules/test_extensions/test_ext4--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext4.control
 create mode 100644 src/test/modules/test_extensions/test_ext5--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext5.control
 create mode 100644 src/test/modules/test_extensions/test_ext_cyclic1--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext_cyclic1.control
 create mode 100644 src/test/modules/test_extensions/test_ext_cyclic2--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext_cyclic2.control

diff --git a/contrib/hstore_plperl/expected/hstore_plperl.out b/contrib/hstore_plperl/expected/hstore_plperl.out
index cf384eb..25fc506 100644
--- a/contrib/hstore_plperl/expected/hstore_plperl.out
+++ b/contrib/hstore_plperl/expected/hstore_plperl.out
@@ -1,6 +1,6 @@
-CREATE EXTENSION hstore;
-CREATE EXTENSION plperl;
-CREATE EXTENSION hstore_plperl;

Re: [HACKERS] creating extension including dependencies

2015-10-03 Thread Andres Freund
On 2015-10-03 17:15:54 +0200, Andres Freund wrote:
> Here's an updated patch. Petr, could you please expand the test to
> handle a bit more complex cascading setups?

...
>From fa11dc75500eb91b68baeeb07a00a789ed0050b3 Mon Sep 17 00:00:00 2001
From: Andres Freund 
Date: Sat, 3 Oct 2015 17:01:32 +0200
Subject: [PATCH] Add CASCADE support for CREATE EXTENSION.

Without CASCADE, if an extension has an unfullfilled dependency on
another extension, CREATE EXTENSION ERRORs out with "required extension
... is not installed". That is annoying, especially when that dependency
is an implementation detail of the extension, rather than something the
extension's user can make sense of.

In addition to CASCADE this also includes a small set of regression
tests around CREATE EXTENSION.

Author: Petr Jelinek, editorialized by Michael Paquier, Andres Freund
Reviewed-By: Michael Paquier, Andres Freund, Jeff Janes,
Discussion: 557e0520.3040...@2ndquadrant.com
---
 contrib/hstore_plperl/expected/hstore_plperl.out   |   6 +-
 contrib/hstore_plperl/expected/hstore_plperlu.out  |   6 +-
 contrib/hstore_plperl/sql/hstore_plperl.sql|   4 +-
 contrib/hstore_plperl/sql/hstore_plperlu.sql   |   4 +-
 .../hstore_plpython/expected/hstore_plpython.out   |   4 +-
 contrib/hstore_plpython/sql/hstore_plpython.sql|   3 +-
 contrib/ltree_plpython/expected/ltree_plpython.out |   4 +-
 contrib/ltree_plpython/sql/ltree_plpython.sql  |   3 +-
 doc/src/sgml/ref/create_extension.sgml |  42 
 src/backend/commands/extension.c   | 217 ++---
 src/backend/parser/gram.y  |   4 +
 src/bin/psql/tab-complete.c|   7 +-
 src/test/modules/Makefile  |   1 +
 src/test/modules/test_extensions/.gitignore|   4 +
 src/test/modules/test_extensions/Makefile  |  21 ++
 .../test_extensions/expected/test_extensions.out   |  30 +++
 .../test_extensions/sql/test_extensions.sql|  18 ++
 .../modules/test_extensions/test_ext1--1.0.sql |   3 +
 src/test/modules/test_extensions/test_ext1.control |   5 +
 .../modules/test_extensions/test_ext2--1.0.sql |   3 +
 src/test/modules/test_extensions/test_ext2.control |   4 +
 .../modules/test_extensions/test_ext3--1.0.sql |   3 +
 src/test/modules/test_extensions/test_ext3.control |   3 +
 .../test_extensions/test_ext_cyclic1--1.0.sql  |   3 +
 .../test_extensions/test_ext_cyclic1.control   |   4 +
 .../test_extensions/test_ext_cyclic2--1.0.sql  |   3 +
 .../test_extensions/test_ext_cyclic2.control   |   4 +
 src/tools/msvc/Mkvcbuild.pm|   3 +-
 28 files changed, 327 insertions(+), 89 deletions(-)
 create mode 100644 src/test/modules/test_extensions/.gitignore
 create mode 100644 src/test/modules/test_extensions/Makefile
 create mode 100644 src/test/modules/test_extensions/expected/test_extensions.out
 create mode 100644 src/test/modules/test_extensions/sql/test_extensions.sql
 create mode 100644 src/test/modules/test_extensions/test_ext1--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext1.control
 create mode 100644 src/test/modules/test_extensions/test_ext2--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext2.control
 create mode 100644 src/test/modules/test_extensions/test_ext3--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext3.control
 create mode 100644 src/test/modules/test_extensions/test_ext_cyclic1--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext_cyclic1.control
 create mode 100644 src/test/modules/test_extensions/test_ext_cyclic2--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext_cyclic2.control

diff --git a/contrib/hstore_plperl/expected/hstore_plperl.out b/contrib/hstore_plperl/expected/hstore_plperl.out
index cf384eb..25fc506 100644
--- a/contrib/hstore_plperl/expected/hstore_plperl.out
+++ b/contrib/hstore_plperl/expected/hstore_plperl.out
@@ -1,6 +1,6 @@
-CREATE EXTENSION hstore;
-CREATE EXTENSION plperl;
-CREATE EXTENSION hstore_plperl;
+CREATE EXTENSION hstore_plperl CASCADE;
+NOTICE:  installing required extension "hstore"
+NOTICE:  installing required extension "plperl"
 SELECT transforms.udt_schema, transforms.udt_name,
routine_schema, routine_name,
group_name, transform_type
diff --git a/contrib/hstore_plperl/expected/hstore_plperlu.out b/contrib/hstore_plperl/expected/hstore_plperlu.out
index c97fd3f..b09fb78 100644
--- a/contrib/hstore_plperl/expected/hstore_plperlu.out
+++ b/contrib/hstore_plperl/expected/hstore_plperlu.out
@@ -1,6 +1,6 @@
-CREATE EXTENSION hstore;
-CREATE EXTENSION plperlu;
-CREATE EXTENSION hstore_plperlu;
+CREATE EXTENSION hstore_plperlu CASCADE;
+NOTICE:  installing required extension "hstore"
+NOTICE:  installing required extension "plperlu"
 SELECT transforms.udt_schema, transforms.udt_name,
routine_schema, routine_name,

Re: [HACKERS] creating extension including dependencies

2015-10-03 Thread Andres Freund
On 2015-09-20 16:38:58 +0200, Petr Jelinek wrote:
> Here it is.

I went over the patch, trying to commit it. Changed a bunch of stylistic
issues (comments, NOTICE location, ...) . But also found a bug: Namely
cascade_parent was set wrongly in a bunch of situations: When an
extension has multiple dependencies the current name would end up
multiple times on the list, and more importantly a parent's
cascade_parent would be corrupted because the list was being modified
in-place in the child.

Here's an updated patch. Petr, could you please expand the test to
handle a bit more complex cascading setups?

Michael: Why did you exclude test_extensions in Mkvcbuild.pm?

Greetings,

Andres Freund


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-10-03 Thread Andres Freund
On 2015-10-03 17:56:07 +0200, Petr Jelinek wrote:
> On 2015-10-03 17:16, Andres Freund wrote:
> >On 2015-10-03 17:15:54 +0200, Andres Freund wrote:
> >>Here's an updated patch. Petr, could you please expand the test to
> >>handle a bit more complex cascading setups?
> >
> 
> Okay, I changed the test to make the dependencies bit more complex - more
> than one dependency per extension + also non-cyclic interdependency. It
> still works as expected.

Ok, pushed this way. We can update the windows testing bit later if
necessary.

Thanks!

Andres


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-10-03 Thread Michael Paquier
On Sun, Oct 4, 2015 at 12:15 AM, Andres Freund  wrote:
> On 2015-09-20 16:38:58 +0200, Petr Jelinek wrote:
> Michael: Why did you exclude test_extensions in Mkvcbuild.pm?

test_extensions contains nothing that should be compiled, only things
that should be installed.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-09-20 Thread Petr Jelinek

On 2015-09-18 04:52, Petr Jelinek wrote:

On 2015-09-17 17:31, Jeff Janes wrote:


If I fail to specify CASCADE and get an ERROR, I think there should be a
HINT which suggests the use of CASCADE.


create extension earthdistance ;
ERROR:  required extension "cube" is not installed

(no hint)

There is a HINT on the reverse operation:
drop extension cube;
ERROR:  cannot drop extension cube because other objects depend on it
DETAIL:  extension earthdistance depends on extension cube
HINT:  Use DROP ... CASCADE to drop the dependent objects too.


Makes sense.



Here it is.


--
 Petr Jelinek  http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
>From a6cc566f4d8600398d6dde1ab478b1565fe9871b Mon Sep 17 00:00:00 2001
From: Petr Jelinek 
Date: Sat, 13 Jun 2015 19:49:10 +0200
Subject: [PATCH] CREATE EXTENSION CASCADE

---
 contrib/hstore_plperl/expected/hstore_plperl.out   |   6 +-
 contrib/hstore_plperl/expected/hstore_plperlu.out  |   6 +-
 contrib/hstore_plperl/sql/hstore_plperl.sql|   4 +-
 contrib/hstore_plperl/sql/hstore_plperlu.sql   |   4 +-
 .../hstore_plpython/expected/hstore_plpython.out   |   4 +-
 contrib/hstore_plpython/sql/hstore_plpython.sql|   3 +-
 contrib/ltree_plpython/expected/ltree_plpython.out |   4 +-
 contrib/ltree_plpython/sql/ltree_plpython.sql  |   3 +-
 doc/src/sgml/ref/create_extension.sgml |  47 -
 src/backend/commands/extension.c   | 210 ++---
 src/backend/parser/gram.y  |   4 +
 src/bin/psql/tab-complete.c|   7 +-
 src/test/modules/Makefile  |   1 +
 src/test/modules/test_extensions/.gitignore|   4 +
 src/test/modules/test_extensions/Makefile  |  21 +++
 .../test_extensions/expected/test_extensions.out   |  31 +++
 .../test_extensions/sql/test_extensions.sql|  18 ++
 .../modules/test_extensions/test_ext1--1.0.sql |   3 +
 src/test/modules/test_extensions/test_ext1.control |   5 +
 .../modules/test_extensions/test_ext2--1.0.sql |   3 +
 src/test/modules/test_extensions/test_ext2.control |   4 +
 .../modules/test_extensions/test_ext3--1.0.sql |   3 +
 src/test/modules/test_extensions/test_ext3.control |   3 +
 .../test_extensions/test_ext_cyclic1--1.0.sql  |   3 +
 .../test_extensions/test_ext_cyclic1.control   |   4 +
 .../test_extensions/test_ext_cyclic2--1.0.sql  |   3 +
 .../test_extensions/test_ext_cyclic2.control   |   4 +
 src/tools/msvc/Mkvcbuild.pm|   3 +-
 28 files changed, 328 insertions(+), 87 deletions(-)
 create mode 100644 src/test/modules/test_extensions/.gitignore
 create mode 100644 src/test/modules/test_extensions/Makefile
 create mode 100644 src/test/modules/test_extensions/expected/test_extensions.out
 create mode 100644 src/test/modules/test_extensions/sql/test_extensions.sql
 create mode 100644 src/test/modules/test_extensions/test_ext1--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext1.control
 create mode 100644 src/test/modules/test_extensions/test_ext2--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext2.control
 create mode 100644 src/test/modules/test_extensions/test_ext3--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext3.control
 create mode 100644 src/test/modules/test_extensions/test_ext_cyclic1--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext_cyclic1.control
 create mode 100644 src/test/modules/test_extensions/test_ext_cyclic2--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext_cyclic2.control

diff --git a/contrib/hstore_plperl/expected/hstore_plperl.out b/contrib/hstore_plperl/expected/hstore_plperl.out
index cf384eb..25fc506 100644
--- a/contrib/hstore_plperl/expected/hstore_plperl.out
+++ b/contrib/hstore_plperl/expected/hstore_plperl.out
@@ -1,6 +1,6 @@
-CREATE EXTENSION hstore;
-CREATE EXTENSION plperl;
-CREATE EXTENSION hstore_plperl;
+CREATE EXTENSION hstore_plperl CASCADE;
+NOTICE:  installing required extension "hstore"
+NOTICE:  installing required extension "plperl"
 SELECT transforms.udt_schema, transforms.udt_name,
routine_schema, routine_name,
group_name, transform_type
diff --git a/contrib/hstore_plperl/expected/hstore_plperlu.out b/contrib/hstore_plperl/expected/hstore_plperlu.out
index c97fd3f..b09fb78 100644
--- a/contrib/hstore_plperl/expected/hstore_plperlu.out
+++ b/contrib/hstore_plperl/expected/hstore_plperlu.out
@@ -1,6 +1,6 @@
-CREATE EXTENSION hstore;
-CREATE EXTENSION plperlu;
-CREATE EXTENSION hstore_plperlu;
+CREATE EXTENSION hstore_plperlu CASCADE;
+NOTICE:  installing required extension "hstore"
+NOTICE:  installing required extension "plperlu"
 SELECT transforms.udt_schema, transforms.udt_name,
routine_schema, routine_name,
group_name, transform_type
diff --git 

Re: [HACKERS] creating extension including dependencies

2015-09-18 Thread Jeff Janes
On Sep 17, 2015 7:52 PM, "Petr Jelinek"  wrote:
>
> On 2015-09-17 17:31, Jeff Janes wrote:
>>
>>
>> Also, It would be nice to have psql tab complete the word CASCADE.
>>
>
> Hmm, it already does?

Indeed it does.  Oops.  I need to run the program I just compiled, and not
some other version that happens to be in my $PATH.  I've learned that for
pg_ctl mostly, but still forget for psql.

Cheers,


Jeff


Re: [HACKERS] creating extension including dependencies

2015-09-17 Thread Jeff Janes
On Tue, Sep 15, 2015 at 8:44 PM, Petr Jelinek  wrote:

> On 2015-09-08 04:06, Michael Paquier wrote:
>
>> On Tue, Sep 8, 2015 at 10:44 AM, Michael Paquier
>>  wrote:
>>
>>>
>>> Attached are as well changes for the documentation that I spotted in
>>> earlier reviews but were not included in the last version sent by Petr
>>> yesterday. Feel free to discard them if you think they are not
>>> adapted, the patch attached applies on top of Petr's patch.
>>>
>>
>> And /log/ is missing in src/test/modules/extensions/.gitignore.
>>
>>
> Ah sorry, I based it on my branch which didn't contain your changes.
> Merged.


If I fail to specify CASCADE and get an ERROR, I think there should be a
HINT which suggests the use of CASCADE.


create extension earthdistance ;
ERROR:  required extension "cube" is not installed

(no hint)

There is a HINT on the reverse operation:
drop extension cube;
ERROR:  cannot drop extension cube because other objects depend on it
DETAIL:  extension earthdistance depends on extension cube
HINT:  Use DROP ... CASCADE to drop the dependent objects too.

Also, It would be nice to have psql tab complete the word CASCADE.

Cheers,

Jeff


Re: [HACKERS] creating extension including dependencies

2015-09-17 Thread Petr Jelinek

On 2015-09-17 17:31, Jeff Janes wrote:


If I fail to specify CASCADE and get an ERROR, I think there should be a
HINT which suggests the use of CASCADE.


create extension earthdistance ;
ERROR:  required extension "cube" is not installed

(no hint)

There is a HINT on the reverse operation:
drop extension cube;
ERROR:  cannot drop extension cube because other objects depend on it
DETAIL:  extension earthdistance depends on extension cube
HINT:  Use DROP ... CASCADE to drop the dependent objects too.


Makes sense.



Also, It would be nice to have psql tab complete the word CASCADE.



Hmm, it already does?


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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-09-16 Thread Andres Freund
On 2015-09-16 05:44:22 +0200, Petr Jelinek wrote:
> On 2015-09-08 04:06, Michael Paquier wrote:
> >On Tue, Sep 8, 2015 at 10:44 AM, Michael Paquier
> > wrote:
> >>
> >>Attached are as well changes for the documentation that I spotted in
> >>earlier reviews but were not included in the last version sent by Petr
> >>yesterday. Feel free to discard them if you think they are not
> >>adapted, the patch attached applies on top of Petr's patch.
> >
> >And /log/ is missing in src/test/modules/extensions/.gitignore.
> >
> 
> Ah sorry, I based it on my branch which didn't contain your changes. Merged.

Please remember to update the commitfest entry...

> @@ -91,8 +92,38 @@ CREATE EXTENSION [ IF NOT EXISTS ]  class="parameter">extension_name
>  The name of the schema in which to install the extension's
>  objects, given that the extension allows its contents to be
>  relocated.  The named schema must already exist.
> -If not specified, and the extension's control file does not specify a
> -schema either, the current default object creation schema is used.
> +If not specified, and the extension control file does not define
> +schema either, the current default object creation
> +schema is used.
> +   

Imo still a spurious change.

Imo this is ready for committer. Will work on committing in the next few
days.

Greetings,

Andres Freund


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-09-16 Thread Alvaro Herrera
Andres Freund wrote:

> > @@ -91,8 +92,38 @@ CREATE EXTENSION [ IF NOT EXISTS ]  > class="parameter">extension_name
> >  The name of the schema in which to install the extension's
> >  objects, given that the extension allows its contents to be
> >  relocated.  The named schema must already exist.
> > -If not specified, and the extension's control file does not 
> > specify a
> > -schema either, the current default object creation schema is used.
> > +If not specified, and the extension control file does not define
> > +schema either, the current default object creation
> > +schema is used.
> > +   
> 
> Imo still a spurious change.

I think some more work is needed in this file -- ISTM the rules used to
determine the creation schema under CASCADE should not be placed within
the SCHEMA clause explanation.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-09-16 Thread Andres Freund
On 2015-09-16 19:46:10 -0300, Alvaro Herrera wrote:
> Andres Freund wrote:
> 
> > > @@ -91,8 +92,38 @@ CREATE EXTENSION [ IF NOT EXISTS ]  > > class="parameter">extension_name
> > >  The name of the schema in which to install the extension's
> > >  objects, given that the extension allows its contents to be
> > >  relocated.  The named schema must already exist.
> > > -If not specified, and the extension's control file does not 
> > > specify a
> > > -schema either, the current default object creation schema is 
> > > used.
> > > +If not specified, and the extension control file does not define
> > > +schema either, the current default object creation
> > > +schema is used.
> > > +   
> > 
> > Imo still a spurious change.
> 
> I think some more work is needed in this file -- ISTM the rules used to
> determine the creation schema under CASCADE should not be placed within
> the SCHEMA clause explanation.

Hm. Why not? Seems to make sense in the context of that page?

Greetings,

Andres Freund


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-09-15 Thread Petr Jelinek

On 2015-09-08 04:06, Michael Paquier wrote:

On Tue, Sep 8, 2015 at 10:44 AM, Michael Paquier
 wrote:


Attached are as well changes for the documentation that I spotted in
earlier reviews but were not included in the last version sent by Petr
yesterday. Feel free to discard them if you think they are not
adapted, the patch attached applies on top of Petr's patch.


And /log/ is missing in src/test/modules/extensions/.gitignore.



Ah sorry, I based it on my branch which didn't contain your changes. Merged.

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



create-extension-cascade-2015-09-16.patch
Description: binary/octet-stream

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-09-07 Thread Michael Paquier
On Tue, Sep 8, 2015 at 10:44 AM, Michael Paquier
 wrote:
> On Tue, Sep 8, 2015 at 6:14 AM, Petr Jelinek wrote:
>> Attached patch uses just boolean in cascade DefElem and splits the
>> CreateExtension into two functions, the cascade code now calls the
>> CreateExtensionInternal. One thing though - I am passing the DefElems
>> directly to the cascaded CreateExtensionStmt options, I think it's not
>> problem but want to give it extra visibility.
>>
>> Also the schema check was moved.
>
> OK, passing the list of extensions through the new routine is indeed a
> cleaner approach. One point of detail is that instead of doing this
> part:
> +   /* Handle the CASCADE option. */
> +   if (d_cascade)
> +   cascade = defGetBoolean(d_cascade);
> +   else
> +   cascade = false;
> You may as well just initialize cascade to false at the beginning of
> the routine and update it only if d_cascade is defined.
>
> Attached are as well changes for the documentation that I spotted in
> earlier reviews but were not included in the last version sent by Petr
> yesterday. Feel free to discard them if you think they are not
> adapted, the patch attached applies on top of Petr's patch.

And /log/ is missing in src/test/modules/extensions/.gitignore.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-09-07 Thread Michael Paquier
On Tue, Sep 8, 2015 at 6:14 AM, Petr Jelinek wrote:
> Attached patch uses just boolean in cascade DefElem and splits the
> CreateExtension into two functions, the cascade code now calls the
> CreateExtensionInternal. One thing though - I am passing the DefElems
> directly to the cascaded CreateExtensionStmt options, I think it's not
> problem but want to give it extra visibility.
>
> Also the schema check was moved.

OK, passing the list of extensions through the new routine is indeed a
cleaner approach. One point of detail is that instead of doing this
part:
+   /* Handle the CASCADE option. */
+   if (d_cascade)
+   cascade = defGetBoolean(d_cascade);
+   else
+   cascade = false;
You may as well just initialize cascade to false at the beginning of
the routine and update it only if d_cascade is defined.

Attached are as well changes for the documentation that I spotted in
earlier reviews but were not included in the last version sent by Petr
yesterday. Feel free to discard them if you think they are not
adapted, the patch attached applies on top of Petr's patch.
Regards,
-- 
Michael


20150908_extension_cascade_docs.patch
Description: binary/octet-stream

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-09-07 Thread Petr Jelinek

On 2015-09-07 21:28, Petr Jelinek wrote:

On 2015-09-07 21:09, Alvaro Herrera wrote:

Andres Freund wrote:

On 2015-09-07 20:56:50 +0200, Petr Jelinek wrote:

Yes that sounds cleaner. Just as a side note, List is a Node and
does have
copy support (and we pass List as DefElem->arg from gram.y in several
places).


I know - but the list element in this case don't have copy support, no?
You seem to have put plain C strings in there, right?


Seems slightly easier to use makeString(), no?



Yes, but I think Andres is correct when saying DefElem->arg is not
nicest place to put it to.



Attached patch uses just boolean in cascade DefElem and splits the 
CreateExtension into two functions, the cascade code now calls the 
CreateExtensionInternal. One thing though - I am passing the DefElems 
directly to the cascaded CreateExtensionStmt options, I think it's not 
problem but want to give it extra visibility.


Also the schema check was moved.

--
 Petr Jelinek  http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
From 5fc0c3245d82220deb7adcc3704b95b82893fcfe Mon Sep 17 00:00:00 2001
From: Petr Jelinek 
Date: Sat, 13 Jun 2015 19:49:10 +0200
Subject: [PATCH] CREATE EXTENSION CASCADE

---
 contrib/hstore_plperl/expected/hstore_plperl.out   |   6 +-
 contrib/hstore_plperl/expected/hstore_plperlu.out  |   6 +-
 contrib/hstore_plperl/sql/hstore_plperl.sql|   4 +-
 contrib/hstore_plperl/sql/hstore_plperlu.sql   |   4 +-
 .../hstore_plpython/expected/hstore_plpython.out   |   4 +-
 contrib/hstore_plpython/sql/hstore_plpython.sql|   3 +-
 contrib/ltree_plpython/expected/ltree_plpython.out |   4 +-
 contrib/ltree_plpython/sql/ltree_plpython.sql  |   3 +-
 doc/src/sgml/ref/create_extension.sgml |  42 +
 src/backend/commands/extension.c   | 209 +++--
 src/backend/parser/gram.y  |   4 +
 src/bin/psql/tab-complete.c|   7 +-
 src/test/modules/Makefile  |   1 +
 src/test/modules/test_extensions/.gitignore|   3 +
 src/test/modules/test_extensions/Makefile  |  21 +++
 .../test_extensions/expected/test_extensions.out   |  30 +++
 .../test_extensions/sql/test_extensions.sql|  18 ++
 .../modules/test_extensions/test_ext1--1.0.sql |   3 +
 src/test/modules/test_extensions/test_ext1.control |   5 +
 .../modules/test_extensions/test_ext2--1.0.sql |   3 +
 src/test/modules/test_extensions/test_ext2.control |   4 +
 .../modules/test_extensions/test_ext3--1.0.sql |   3 +
 src/test/modules/test_extensions/test_ext3.control |   3 +
 .../test_extensions/test_ext_cyclic1--1.0.sql  |   3 +
 .../test_extensions/test_ext_cyclic1.control   |   4 +
 .../test_extensions/test_ext_cyclic2--1.0.sql  |   3 +
 .../test_extensions/test_ext_cyclic2.control   |   4 +
 src/tools/msvc/Mkvcbuild.pm|   3 +-
 28 files changed, 323 insertions(+), 84 deletions(-)
 create mode 100644 src/test/modules/test_extensions/.gitignore
 create mode 100644 src/test/modules/test_extensions/Makefile
 create mode 100644 src/test/modules/test_extensions/expected/test_extensions.out
 create mode 100644 src/test/modules/test_extensions/sql/test_extensions.sql
 create mode 100644 src/test/modules/test_extensions/test_ext1--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext1.control
 create mode 100644 src/test/modules/test_extensions/test_ext2--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext2.control
 create mode 100644 src/test/modules/test_extensions/test_ext3--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext3.control
 create mode 100644 src/test/modules/test_extensions/test_ext_cyclic1--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext_cyclic1.control
 create mode 100644 src/test/modules/test_extensions/test_ext_cyclic2--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext_cyclic2.control

diff --git a/contrib/hstore_plperl/expected/hstore_plperl.out b/contrib/hstore_plperl/expected/hstore_plperl.out
index cf384eb..25fc506 100644
--- a/contrib/hstore_plperl/expected/hstore_plperl.out
+++ b/contrib/hstore_plperl/expected/hstore_plperl.out
@@ -1,6 +1,6 @@
-CREATE EXTENSION hstore;
-CREATE EXTENSION plperl;
-CREATE EXTENSION hstore_plperl;
+CREATE EXTENSION hstore_plperl CASCADE;
+NOTICE:  installing required extension "hstore"
+NOTICE:  installing required extension "plperl"
 SELECT transforms.udt_schema, transforms.udt_name,
routine_schema, routine_name,
group_name, transform_type
diff --git a/contrib/hstore_plperl/expected/hstore_plperlu.out b/contrib/hstore_plperl/expected/hstore_plperlu.out
index 8c689ad..3fc3201 100644
--- a/contrib/hstore_plperl/expected/hstore_plperlu.out
+++ b/contrib/hstore_plperl/expected/hstore_plperlu.out
@@ -1,6 +1,6 @@
-CREATE EXTENSION hstore;
-CREATE 

Re: [HACKERS] creating extension including dependencies

2015-09-07 Thread Petr Jelinek

On 2015-09-02 17:31, Andres Freund wrote:

On 2015-09-02 17:27:38 +0200, Andres Freund wrote:

1) Passing the list of parents through the cascade DefElem strikes me as
incredibly ugly.

For one the cascade option really should take a true/false type option
on the C level (so you can do defGetBoolean()), for another passing
through the list of parents via DefElem->arg seems wrong. You're
supposed to be able to copy parsenodes and at the very least that's
broken by the approach.


I think the fix here is to split off the bulk of CreateExtension() into
an internal function that takes additional parameters.



Yes that sounds cleaner. Just as a side note, List is a Node and does 
have copy support (and we pass List as DefElem->arg from gram.y in 
several places).


> 2) I don't like the control flow around the schema selection.
>
> It seems to be getting a bit arcane. How about instead moving the
> "extension \"%s\" must be installed in schema \"%s\" check into the if
> (control->schema != NULL) block and check for d_schema after it? That
> should look cleaner.
>

I did something like that in one of the revisions, the complaint there 
was that it changes order of errors you get in situation when the schema 
is not the same as the one in control file and it also does not exist.


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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-09-07 Thread Andres Freund
On 2015-09-07 20:56:50 +0200, Petr Jelinek wrote:
> Yes that sounds cleaner. Just as a side note, List is a Node and does have
> copy support (and we pass List as DefElem->arg from gram.y in several
> places).

I know - but the list element in this case don't have copy support, no?
You seem to have put plain C strings in there, right?

> > 2) I don't like the control flow around the schema selection.
> >
> > It seems to be getting a bit arcane. How about instead moving the
> > "extension \"%s\" must be installed in schema \"%s\" check into the if
> > (control->schema != NULL) block and check for d_schema after it? That
> > should look cleaner.
> >
> 
> I did something like that in one of the revisions, the complaint there was
> that it changes order of errors you get in situation when the schema is not
> the same as the one in control file and it also does not exist.

So what? That seems like a pretty harmless change - it's not like this
is something being hit day in/out right now.

Andres


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-09-07 Thread Andres Freund
On 2015-09-07 16:09:27 -0300, Alvaro Herrera wrote:
> Andres Freund wrote:
> > On 2015-09-07 20:56:50 +0200, Petr Jelinek wrote:
> > > Yes that sounds cleaner. Just as a side note, List is a Node and does have
> > > copy support (and we pass List as DefElem->arg from gram.y in several
> > > places).
> > 
> > I know - but the list element in this case don't have copy support, no?
> > You seem to have put plain C strings in there, right?
> 
> Seems slightly easier to use makeString(), no?

It'd still be a god forsakenly ugly API to use DefElems for this. Imo
not being able to specify a boolean argument for a boolean value pretty
much hints at it being the wrong approach. I don't see any reason to go
that way rather than split the 'cycle detection' state into an argument
to CreateExtensionInternal() or something.

Greetings,

Andres Freund


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-09-07 Thread Petr Jelinek

On 2015-09-07 21:09, Alvaro Herrera wrote:

Andres Freund wrote:

On 2015-09-07 20:56:50 +0200, Petr Jelinek wrote:

Yes that sounds cleaner. Just as a side note, List is a Node and does have
copy support (and we pass List as DefElem->arg from gram.y in several
places).


I know - but the list element in this case don't have copy support, no?
You seem to have put plain C strings in there, right?


Seems slightly easier to use makeString(), no?



Yes, but I think Andres is correct when saying DefElem->arg is not 
nicest place to put it to.


Looking at the code again, splitting the function is actually not that 
easy since the cascaded extension creation has to execute all the same 
code/checks as CreateExtension does; It might be better to just add the 
parameter to the CreateExtension and call it with NIL value from utility.c.


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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-09-07 Thread Alvaro Herrera
Andres Freund wrote:
> On 2015-09-07 20:56:50 +0200, Petr Jelinek wrote:
> > Yes that sounds cleaner. Just as a side note, List is a Node and does have
> > copy support (and we pass List as DefElem->arg from gram.y in several
> > places).
> 
> I know - but the list element in this case don't have copy support, no?
> You seem to have put plain C strings in there, right?

Seems slightly easier to use makeString(), no?

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-09-02 Thread Andres Freund
Hi,

I'm looking at committing this patch. I found some nitpick-level things
that I can easily fixup. But I dislike two things:

1) Passing the list of parents through the cascade DefElem strikes me as
incredibly ugly.

For one the cascade option really should take a true/false type option
on the C level (so you can do defGetBoolean()), for another passing
through the list of parents via DefElem->arg seems wrong. You're
supposed to be able to copy parsenodes and at the very least that's
broken by the approach.

2) I don't like the control flow around the schema selection.

It seems to be getting a bit arcane. How about instead moving the
"extension \"%s\" must be installed in schema \"%s\" check into the if
(control->schema != NULL) block and check for d_schema after it? That
should look cleaner.

Greetings,

Andres Freund


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-09-02 Thread Andres Freund
On 2015-09-02 17:27:38 +0200, Andres Freund wrote:
> 1) Passing the list of parents through the cascade DefElem strikes me as
> incredibly ugly.
> 
> For one the cascade option really should take a true/false type option
> on the C level (so you can do defGetBoolean()), for another passing
> through the list of parents via DefElem->arg seems wrong. You're
> supposed to be able to copy parsenodes and at the very least that's
> broken by the approach.

I think the fix here is to split off the bulk of CreateExtension() into
an internal function that takes additional parameters.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-07-31 Thread Petr Jelinek

On 2015-07-31 03:03, Michael Paquier wrote:

On Thu, Jul 30, 2015 at 10:58 PM, Petr Jelinek wrote:

On 2015-07-27 15:18, Michael Paquier wrote:

Something also has not been discussed yet: what to do with new_version
and old_version (the options of CreateExtensionStmt)? As of now if
those options are defined they are not passed down to the parent
extensions but shouldn't we raise an error if they are used in
combination with CASCADE? In any case, I think that the behavior
chosen should be documented.



I don't see why we should raise error, they are used just for the top-level
extension and I think it makes sense that way. CASCADE documentation says
SCHEMA option is cascaded to required extensions, do we need to say
something more than that (ie explicitly saying that the old_version and
new_version aren't)?


OK, let's do so then. I think that we should still document the fact
that the old and new version strings and not passed to the parent
extensions when cascade is used for clarity. Something like:
Other options are not recursively applied when the CASCASE clause is used.

I have been through this patch one last time and changed the following:
- Improved documentation: missing markups with literal, SCHEMA is a
clause and not a parameter, added explanation that options other than
SCHEMA are not applied recursively with CASCADE
- Corrected .gitignore in test_extensions, log/ was missing.
Those are minor things though, hence I just switched patch as Ready
for committer.



Yeah I agree with those changes, thanks for the review.


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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-07-30 Thread Petr Jelinek

On 2015-07-27 15:18, Michael Paquier wrote:

On Sun, Jul 26, 2015 at 1:01 AM, Petr Jelinek wrote:

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


+In case the extension specifies schema in its control file, the schema
s/schema/literalschema//

+++ b/src/test/modules/test_extensions/.gitignore
@@ -0,0 +1,3 @@
+# Generated subdirectories
+/results/
+/tmp_check/
test_extensions/.gitignore is missing /log/.

Something also has not been discussed yet: what to do with new_version
and old_version (the options of CreateExtensionStmt)? As of now if
those options are defined they are not passed down to the parent
extensions but shouldn't we raise an error if they are used in
combination with CASCADE? In any case, I think that the behavior
chosen should be documented.



I don't see why we should raise error, they are used just for the 
top-level extension and I think it makes sense that way. CASCADE 
documentation says SCHEMA option is cascaded to required extensions, do 
we need to say something more than that (ie explicitly saying that the 
old_version and new_version aren't)?


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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-07-30 Thread Michael Paquier
On Thu, Jul 30, 2015 at 10:58 PM, Petr Jelinek wrote:
 On 2015-07-27 15:18, Michael Paquier wrote:
 Something also has not been discussed yet: what to do with new_version
 and old_version (the options of CreateExtensionStmt)? As of now if
 those options are defined they are not passed down to the parent
 extensions but shouldn't we raise an error if they are used in
 combination with CASCADE? In any case, I think that the behavior
 chosen should be documented.


 I don't see why we should raise error, they are used just for the top-level
 extension and I think it makes sense that way. CASCADE documentation says
 SCHEMA option is cascaded to required extensions, do we need to say
 something more than that (ie explicitly saying that the old_version and
 new_version aren't)?

OK, let's do so then. I think that we should still document the fact
that the old and new version strings and not passed to the parent
extensions when cascade is used for clarity. Something like:
Other options are not recursively applied when the CASCASE clause is used.

I have been through this patch one last time and changed the following:
- Improved documentation: missing markups with literal, SCHEMA is a
clause and not a parameter, added explanation that options other than
SCHEMA are not applied recursively with CASCADE
- Corrected .gitignore in test_extensions, log/ was missing.
Those are minor things though, hence I just switched patch as Ready
for committer.
-- 
Michael
diff --git a/contrib/hstore_plperl/expected/hstore_plperl.out b/contrib/hstore_plperl/expected/hstore_plperl.out
index cf384eb..25fc506 100644
--- a/contrib/hstore_plperl/expected/hstore_plperl.out
+++ b/contrib/hstore_plperl/expected/hstore_plperl.out
@@ -1,6 +1,6 @@
-CREATE EXTENSION hstore;
-CREATE EXTENSION plperl;
-CREATE EXTENSION hstore_plperl;
+CREATE EXTENSION hstore_plperl CASCADE;
+NOTICE:  installing required extension hstore
+NOTICE:  installing required extension plperl
 SELECT transforms.udt_schema, transforms.udt_name,
routine_schema, routine_name,
group_name, transform_type
diff --git a/contrib/hstore_plperl/expected/hstore_plperlu.out b/contrib/hstore_plperl/expected/hstore_plperlu.out
index 8c689ad..3fc3201 100644
--- a/contrib/hstore_plperl/expected/hstore_plperlu.out
+++ b/contrib/hstore_plperl/expected/hstore_plperlu.out
@@ -1,6 +1,6 @@
-CREATE EXTENSION hstore;
-CREATE EXTENSION plperlu;
-CREATE EXTENSION hstore_plperlu;
+CREATE EXTENSION hstore_plperlu CASCADE;
+NOTICE:  installing required extension hstore
+NOTICE:  installing required extension plperlu
 SELECT transforms.udt_schema, transforms.udt_name,
routine_schema, routine_name,
group_name, transform_type
diff --git a/contrib/hstore_plperl/sql/hstore_plperl.sql b/contrib/hstore_plperl/sql/hstore_plperl.sql
index 0f70f14..9398aed 100644
--- a/contrib/hstore_plperl/sql/hstore_plperl.sql
+++ b/contrib/hstore_plperl/sql/hstore_plperl.sql
@@ -1,6 +1,4 @@
-CREATE EXTENSION hstore;
-CREATE EXTENSION plperl;
-CREATE EXTENSION hstore_plperl;
+CREATE EXTENSION hstore_plperl CASCADE;
 
 SELECT transforms.udt_schema, transforms.udt_name,
routine_schema, routine_name,
diff --git a/contrib/hstore_plperl/sql/hstore_plperlu.sql b/contrib/hstore_plperl/sql/hstore_plperlu.sql
index 3cfb2fd..8d8508c 100644
--- a/contrib/hstore_plperl/sql/hstore_plperlu.sql
+++ b/contrib/hstore_plperl/sql/hstore_plperlu.sql
@@ -1,6 +1,4 @@
-CREATE EXTENSION hstore;
-CREATE EXTENSION plperlu;
-CREATE EXTENSION hstore_plperlu;
+CREATE EXTENSION hstore_plperlu CASCADE;
 
 SELECT transforms.udt_schema, transforms.udt_name,
routine_schema, routine_name,
diff --git a/contrib/hstore_plpython/expected/hstore_plpython.out b/contrib/hstore_plpython/expected/hstore_plpython.out
index b7a6a92..55f4efe 100644
--- a/contrib/hstore_plpython/expected/hstore_plpython.out
+++ b/contrib/hstore_plpython/expected/hstore_plpython.out
@@ -1,5 +1,5 @@
-CREATE EXTENSION plpython2u;
-CREATE EXTENSION hstore_plpython2u;
+CREATE EXTENSION hstore_plpython2u CASCADE;
+NOTICE:  installing required extension plpython2u
 -- test hstore - python
 CREATE FUNCTION test1(val hstore) RETURNS int
 LANGUAGE plpythonu
diff --git a/contrib/hstore_plpython/sql/hstore_plpython.sql b/contrib/hstore_plpython/sql/hstore_plpython.sql
index 9ff2ebc..d55beda 100644
--- a/contrib/hstore_plpython/sql/hstore_plpython.sql
+++ b/contrib/hstore_plpython/sql/hstore_plpython.sql
@@ -1,5 +1,4 @@
-CREATE EXTENSION plpython2u;
-CREATE EXTENSION hstore_plpython2u;
+CREATE EXTENSION hstore_plpython2u CASCADE;
 
 
 -- test hstore - python
diff --git a/contrib/ltree_plpython/expected/ltree_plpython.out b/contrib/ltree_plpython/expected/ltree_plpython.out
index 934529e..9bee6be 100644
--- a/contrib/ltree_plpython/expected/ltree_plpython.out
+++ b/contrib/ltree_plpython/expected/ltree_plpython.out
@@ -1,5 +1,5 @@
-CREATE EXTENSION plpython2u;
-CREATE EXTENSION ltree_plpython2u;
+CREATE EXTENSION ltree_plpython2u CASCADE;
+NOTICE:  

Re: [HACKERS] creating extension including dependencies

2015-07-27 Thread Michael Paquier
On Sun, Jul 26, 2015 at 1:01 AM, Petr Jelinek wrote:
 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...

+In case the extension specifies schema in its control file, the schema
s/schema/literalschema//

+++ b/src/test/modules/test_extensions/.gitignore
@@ -0,0 +1,3 @@
+# Generated subdirectories
+/results/
+/tmp_check/
test_extensions/.gitignore is missing /log/.

Something also has not been discussed yet: what to do with new_version
and old_version (the options of CreateExtensionStmt)? As of now if
those options are defined they are not passed down to the parent
extensions but shouldn't we raise an error if they are used in
combination with CASCADE? In any case, I think that the behavior
chosen should be documented.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-07-25 Thread Petr Jelinek

On 2015-07-25 14:37, Michael Paquier wrote:

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


+  para
+   If schema is not same as the one in extension's control file and
+   the literalCASCADE/ 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
From c13e2919466bb9001666a63361ff560d1779bde4 Mon Sep 17 00:00:00 2001
From: Petr Jelinek pjmodos@pjmodos.net
Date: Sat, 13 Jun 2015 19:49:10 +0200
Subject: [PATCH] CREATE EXTENSION CASCADE

---
 contrib/hstore_plperl/expected/hstore_plperl.out   |   6 +-
 contrib/hstore_plperl/expected/hstore_plperlu.out  |   6 +-
 contrib/hstore_plperl/sql/hstore_plperl.sql|   4 +-
 contrib/hstore_plperl/sql/hstore_plperlu.sql   |   4 +-
 .../hstore_plpython/expected/hstore_plpython.out   |   4 +-
 contrib/hstore_plpython/sql/hstore_plpython.sql|   3 +-
 contrib/ltree_plpython/expected/ltree_plpython.out |   4 +-
 contrib/ltree_plpython/sql/ltree_plpython.sql  |   3 +-
 doc/src/sgml/ref/create_extension.sgml |  42 +
 src/backend/commands/extension.c   | 103 ++---
 src/backend/parser/gram.y  |   4 +
 src/bin/psql/tab-complete.c|   7 +-
 src/test/modules/Makefile  |   1 +
 src/test/modules/test_extensions/.gitignore|   3 +
 src/test/modules/test_extensions/Makefile  |  21 +
 .../test_extensions/expected/test_extensions.out   |  30 ++
 .../test_extensions/sql/test_extensions.sql|  18 
 .../modules/test_extensions/test_ext1--1.0.sql |   3 +
 src/test/modules/test_extensions/test_ext1.control |   5 +
 .../modules/test_extensions/test_ext2--1.0.sql |   3 +
 src/test/modules/test_extensions/test_ext2.control |   4 +
 .../modules/test_extensions/test_ext3--1.0.sql |   3 +
 src/test/modules/test_extensions/test_ext3.control |   3 +
 .../test_extensions/test_ext_cyclic1--1.0.sql  |   3 +
 .../test_extensions/test_ext_cyclic1.control   |   4 +
 .../test_extensions/test_ext_cyclic2--1.0.sql  |   3 +
 .../test_extensions/test_ext_cyclic2.control   |   4 +
 src/tools/msvc/Mkvcbuild.pm|   2 +-
 28 files changed, 263 insertions(+), 37 deletions(-)
 create mode 100644 src/test/modules/test_extensions/.gitignore
 create mode 100644 src/test/modules/test_extensions/Makefile
 create mode 100644 src/test/modules/test_extensions/expected/test_extensions.out
 create mode 100644 src/test/modules/test_extensions/sql/test_extensions.sql
 create mode 100644 src/test/modules/test_extensions/test_ext1--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext1.control
 create mode 100644 src/test/modules/test_extensions/test_ext2--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext2.control
 create mode 100644 

Re: [HACKERS] creating extension including dependencies

2015-07-25 Thread Michael Paquier
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 literalSCHEMA/ parameter. The actual
+behavior of the literalSCHEMA/ 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 literalCASCADE/ 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


Re: [HACKERS] creating extension including dependencies

2015-07-24 Thread Petr Jelinek

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


--
 Petr Jelinek  http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services
From a5e59422c162818002f5946542ad47373bf3eb12 Mon Sep 17 00:00:00 2001
From: Petr Jelinek pjmo...@pjmodos.net
Date: Sat, 13 Jun 2015 19:49:10 +0200
Subject: [PATCH] CREATE EXTENSION CASCADE

---
 contrib/hstore_plperl/expected/hstore_plperl.out   |   6 +-
 contrib/hstore_plperl/expected/hstore_plperlu.out  |   6 +-
 contrib/hstore_plperl/sql/hstore_plperl.sql|   4 +-
 contrib/hstore_plperl/sql/hstore_plperlu.sql   |   4 +-
 .../hstore_plpython/expected/hstore_plpython.out   |   4 +-
 contrib/hstore_plpython/sql/hstore_plpython.sql|   3 +-
 contrib/ltree_plpython/expected/ltree_plpython.out |   4 +-
 contrib/ltree_plpython/sql/ltree_plpython.sql  |   3 +-
 doc/src/sgml/ref/create_extension.sgml |  40 ++
 src/backend/commands/extension.c   | 158 -
 src/backend/parser/gram.y  |   4 +
 src/bin/psql/tab-complete.c|   7 +-
 src/test/modules/Makefile  |   1 +
 src/test/modules/test_extensions/.gitignore|   3 +
 src/test/modules/test_extensions/Makefile  |  22 +++
 .../test_extensions/expected/test_extensions.out   |  24 
 .../test_extensions/sql/test_extensions.sql|  15 ++
 .../modules/test_extensions/test_ext1--1.0.sql |   3 +
 src/test/modules/test_extensions/test_ext1.control |   5 +
 .../modules/test_extensions/test_ext2--1.0.sql |   3 +
 src/test/modules/test_extensions/test_ext2.control |   4 +
 .../modules/test_extensions/test_ext3--1.0.sql |   3 +
 src/test/modules/test_extensions/test_ext3.control |   3 +
 .../test_extensions/test_ext_cyclic1--1.0.sql  |   3 +
 .../test_extensions/test_ext_cyclic1.control   |   4 +
 .../test_extensions/test_ext_cyclic2--1.0.sql  |   3 +
 .../test_extensions/test_ext_cyclic2.control   |   4 +
 src/tools/msvc/Mkvcbuild.pm|   2 +-
 28 files changed, 287 insertions(+), 58 deletions(-)
 create mode 100644 src/test/modules/test_extensions/.gitignore
 create mode 100644 src/test/modules/test_extensions/Makefile
 create mode 100644 src/test/modules/test_extensions/expected/test_extensions.out
 create mode 100644 src/test/modules/test_extensions/sql/test_extensions.sql
 create mode 100644 src/test/modules/test_extensions/test_ext1--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext1.control
 create mode 100644 src/test/modules/test_extensions/test_ext2--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext2.control
 create mode 100644 src/test/modules/test_extensions/test_ext3--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext3.control
 create mode 100644 src/test/modules/test_extensions/test_ext_cyclic1--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext_cyclic1.control
 create mode 100644 src/test/modules/test_extensions/test_ext_cyclic2--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext_cyclic2.control

diff --git a/contrib/hstore_plperl/expected/hstore_plperl.out b/contrib/hstore_plperl/expected/hstore_plperl.out
index cf384eb..25fc506 100644
--- a/contrib/hstore_plperl/expected/hstore_plperl.out
+++ b/contrib/hstore_plperl/expected/hstore_plperl.out
@@ -1,6 +1,6 @@
-CREATE EXTENSION hstore;
-CREATE EXTENSION plperl;
-CREATE EXTENSION hstore_plperl;
+CREATE EXTENSION hstore_plperl CASCADE;
+NOTICE:  installing required extension hstore
+NOTICE:  installing required extension plperl
 SELECT transforms.udt_schema, transforms.udt_name,
routine_schema, routine_name,
group_name, transform_type
diff --git a/contrib/hstore_plperl/expected/hstore_plperlu.out 

Re: [HACKERS] creating extension including dependencies

2015-07-21 Thread Michael Paquier
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.

OK, I'm fine with that, aka with CASCADE and a SCHEMA specified we use
it if needed or ignore it otherwise (if I am following correctly).

CREATE EXTENSION foo SCHEMA bar will fail if the extension is not
relocatable *and* does not have a schema specified in its control
file. A non-relocatable extension can be initially created anywhere.
It just cannot be moved afterwards from its original schema.

 Obviously we've gotta document all this properly.

Sure. That's a sine-qua-non condition for this patch.
Regards,
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-07-21 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Mon, Jul 20, 2015 at 10:29 PM, Michael Paquier
 michael.paqu...@gmail.com wrote:
 In short I would give up on the DEFAULT SCHEMA business, and
 add a new flag in the control file to decide if a given extension
 passes down the schema name of its child when created in cascade,
 default being true for the potential issues with search_path not
 pointing to public.

 Well, so far, it seems like this decision is something where different
 DBAs might have different policies.  If you put the flag in the
 control file, you're saying it is the extension developer's decision,
 which may not be best.

I have doubts about that too.  But really, why have a flag at all
anywhere?  If we are doing a CASCADE, and the referenced extension needs a
schema, the alternatives are either (1) let it have one, or (2) fail.
I am not seeing that (2) is a superior alternative in any circumstances.

We will need to document that the behavior of CASCADE is install all
needed extensions into the schema you specify, but what's wrong with
that?  If the user wants to put them in different schemas, he cannot
use CASCADE in any case.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-07-21 Thread Robert Haas
On Mon, Jul 20, 2015 at 10:29 PM, Michael Paquier
michael.paqu...@gmail.com wrote: In  short I would give up on the
DEFAULT SCHEMA business, and
 add a new flag in the control file to decide if a given extension
 passes down the schema name of its child when created in cascade,
 default being true for the potential issues with search_path not
 pointing to public.

Well, so far, it seems like this decision is something where different
DBAs might have different policies.  If you put the flag in the
control file, you're saying it is the extension developer's decision,
which may not be best.

Maybe I'm confused.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-07-21 Thread Petr Jelinek

On 2015-07-21 15:48, Tom Lane wrote:

Robert Haas robertmh...@gmail.com writes:

On Mon, Jul 20, 2015 at 10:29 PM, Michael Paquier
michael.paqu...@gmail.com wrote:

In short I would give up on the DEFAULT SCHEMA business, and
add a new flag in the control file to decide if a given extension
passes down the schema name of its child when created in cascade,
default being true for the potential issues with search_path not
pointing to public.



Well, so far, it seems like this decision is something where different
DBAs might have different policies.  If you put the flag in the
control file, you're saying it is the extension developer's decision,
which may not be best.


I have doubts about that too.  But really, why have a flag at all
anywhere?  If we are doing a CASCADE, and the referenced extension needs a
schema, the alternatives are either (1) let it have one, or (2) fail.
I am not seeing that (2) is a superior alternative in any circumstances.

We will need to document that the behavior of CASCADE is install all
needed extensions into the schema you specify, but what's wrong with
that?  If the user wants to put them in different schemas, he cannot
use CASCADE in any case.



Yes this is the behavior I want as well. 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).


Or course yes would then mean CREATE EXTENSION foo SCHEMA bar will 
fail if foo is not relocatable but CREATE EXTENSION foo SCHEMA bar 
CASCADE will succeed and install foo into schema foo instead of 
bar and only relocatable dependencies will go to bar. OTOH 
non-relocatable dependencies will go to their own schema no matter what 
user specifies in the command so I guess it's ok to just document that 
this is the behavior of CASCADE. As you say if somebody wants control 
over each individual extension they can't use CASCADE anyway.


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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-07-21 Thread Tom Lane
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.

Obviously we've gotta document all this properly.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-07-20 Thread Petr Jelinek

On 2015-07-19 17:16, Michael Paquier wrote:

On Sat, Jul 18, 2015 at 8:00 AM, Petr Jelinek p...@2ndquadrant.com wrote:

On 2015-07-15 06:07, Michael Paquier wrote:


On Fri, Jul 10, 2015 at 11:28 PM, Tom Lane t...@sss.pgh.pa.us wrote:


Andres Freund and...@anarazel.de writes:


On July 10, 2015 4:16:59 PM GMT+02:00, Tom Lane t...@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).


Hm...

First, the current patch has a surprising behavior because it fails to
create an extension in cascade when creation is attempted on a custom
schema:
=# create schema po;
CREATE SCHEMA
=# create extension hstore_plperl with schema po cascade;
NOTICE:  0: installing required extension hstore
LOCATION:  CreateExtension, extension.c:1483
NOTICE:  0: installing required extension plperl
LOCATION:  CreateExtension, extension.c:1483
ERROR:  42704: type hstore does not exist
LOCATION:  typenameType, parse_type.c:258
Time: 30.071 ms
To facilitate the life of users, I think that the parent extensions
should be created on the same schema as its child by default. In this
case hstore should be created in po, not public.



That's actually a bug because the previous version of the patch did not 
set the reqext correctly after creating the required extension.




Hence, as a schema can only be specified in a control file for a
non-relocatable extension, I think that the schema name propagated to
the root extensions should be the one specified in the control file of
the child if it is defined in it instead of the one specified by user
(imagine for example an extension created in cascade that requires
adminpack, extension that can only be deployed in pg_catalog), and
have the root node use its own schema if it has one in its control
file by default.

For example in this case:
foo1 (WITH SCHEMA hoge) - foo2 (schema = pg_catalog) - foo2_1
 |
 |-- foo2_2
 --- foo3 (no schema)
With this command:
CREATE EXTENSION foo1 WITH SCHEMA hoge;
foo3 is created on schema po. foo2, as well its required dependencies
foo2_1 and foo2_2 are created on pg_catalog.

Now DEFAULT SCHEMA is trying to achieve: Hey, I want to define foo2_1
and foo2_2 on schema hoge. This may be worth achieving (though IMO I
think that foo1 should have direct dependencies with foo2_1 and
foo2_2), but I think that we should decide what is the default
behavior we want, and implement the additional behavior in a second
patch, separated from the patch of this thread. Personally I am more a
fan of propagating to parent extensions the schema of the child and
not of its grand-child by default, but I am not alone here :)



This is something that I as a user of the feature specifically don't 
want to happen as I install extensions either to common schema (for some 
utility ones) or into separate schemas (for the rest), but I never want 
the utility extension to go to the same schema as the other ones. This 
is especially true when installing non-relocatable extension which 
depends on relocatable one. Obviously, it does not mean that nobody else 
wants this though :)


--
 Petr Jelinek  http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services
From 54529b7e8bf2e5554569369ff976befbac981a25 Mon Sep 17 00:00:00 2001
From: Petr Jelinek pjmodos@pjmodos.net
Date: Sat, 13 Jun 2015 19:49:10 +0200
Subject: [PATCH] CREATE EXTENSION CASCADE

---
 contrib/hstore_plperl/expected/hstore_plperl.out   |   6 +-
 contrib/hstore_plperl/expected/hstore_plperlu.out  |   6 +-
 contrib/hstore_plperl/sql/hstore_plperl.sql|   4 +-
 contrib/hstore_plperl/sql/hstore_plperlu.sql   |   4 +-
 .../hstore_plpython/expected/hstore_plpython.out   |   4 +-
 contrib/hstore_plpython/sql/hstore_plpython.sql|   3 +-
 contrib/ltree_plpython/expected/ltree_plpython.out |   4 +-
 contrib/ltree_plpython/sql/ltree_plpython.sql  |   3 +-
 

Re: [HACKERS] creating extension including dependencies

2015-07-20 Thread Michael Paquier
On Mon, Jul 20, 2015 at 10:20 PM, Petr Jelinek p...@2ndquadrant.com wrote:
 On 2015-07-19 17:16, Michael Paquier wrote:

 On Sat, Jul 18, 2015 at 8:00 AM, Petr Jelinek p...@2ndquadrant.com
 wrote:

 On 2015-07-15 06:07, Michael Paquier wrote:


 On Fri, Jul 10, 2015 at 11:28 PM, Tom Lane t...@sss.pgh.pa.us wrote:


 Andres Freund and...@anarazel.de writes:


 On July 10, 2015 4:16:59 PM GMT+02:00, Tom Lane t...@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).


 Hm...

 First, the current patch has a surprising behavior because it fails to
 create an extension in cascade when creation is attempted on a custom
 schema:
 =# create schema po;
 CREATE SCHEMA
 =# create extension hstore_plperl with schema po cascade;
 NOTICE:  0: installing required extension hstore
 LOCATION:  CreateExtension, extension.c:1483
 NOTICE:  0: installing required extension plperl
 LOCATION:  CreateExtension, extension.c:1483
 ERROR:  42704: type hstore does not exist
 LOCATION:  typenameType, parse_type.c:258
 Time: 30.071 ms
 To facilitate the life of users, I think that the parent extensions
 should be created on the same schema as its child by default. In this
 case hstore should be created in po, not public.


 That's actually a bug because the previous version of the patch did not set
 the reqext correctly after creating the required extension.


 Hence, as a schema can only be specified in a control file for a
 non-relocatable extension, I think that the schema name propagated to
 the root extensions should be the one specified in the control file of
 the child if it is defined in it instead of the one specified by user
 (imagine for example an extension created in cascade that requires
 adminpack, extension that can only be deployed in pg_catalog), and
 have the root node use its own schema if it has one in its control
 file by default.

 For example in this case:
 foo1 (WITH SCHEMA hoge) - foo2 (schema = pg_catalog) - foo2_1
  |
  |-- foo2_2
  --- foo3 (no schema)
 With this command:
 CREATE EXTENSION foo1 WITH SCHEMA hoge;
 foo3 is created on schema po. foo2, as well its required dependencies
 foo2_1 and foo2_2 are created on pg_catalog.

 Now DEFAULT SCHEMA is trying to achieve: Hey, I want to define foo2_1
 and foo2_2 on schema hoge. This may be worth achieving (though IMO I
 think that foo1 should have direct dependencies with foo2_1 and
 foo2_2), but I think that we should decide what is the default
 behavior we want, and implement the additional behavior in a second
 patch, separated from the patch of this thread. Personally I am more a
 fan of propagating to parent extensions the schema of the child and
 not of its grand-child by default, but I am not alone here :)


 This is something that I as a user of the feature specifically don't want to
 happen as I install extensions either to common schema (for some utility
 ones) or into separate schemas (for the rest), but I never want the utility
 extension to go to the same schema as the other ones. This is especially
 true when installing non-relocatable extension which depends on relocatable
 one. Obviously, it does not mean that nobody else wants this though :)

So, in your patch the default behavior is to create parent extensions
on schema public if the child extension created specifies a schema:
=# create schema po;
CREATE SCHEMA
=# create extension test_ext2 cascade schema po;
CREATE EXTENSION
=# \dx  test_ext*
  List of installed extensions
   Name| Version | Schema |   Description
---+-++--
 test_ext2 | 1.0 | po | Test extension 2
 test_ext3 | 1.0 | public | Test extension 3
(2 rows)
=# drop extension test_ext3 cascade;
NOTICE:  0: drop cascades to extension test_ext2
LOCATION:  reportDependentObjects, dependency.c:1009
DROP EXTENSION
=# create 

Re: [HACKERS] creating extension including dependencies

2015-07-19 Thread Michael Paquier
On Sat, Jul 18, 2015 at 8:00 AM, Petr Jelinek p...@2ndquadrant.com wrote:
 On 2015-07-15 06:07, Michael Paquier wrote:

 On Fri, Jul 10, 2015 at 11:28 PM, Tom Lane t...@sss.pgh.pa.us wrote:

 Andres Freund and...@anarazel.de writes:

 On July 10, 2015 4:16:59 PM GMT+02:00, Tom Lane t...@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).

Hm...

First, the current patch has a surprising behavior because it fails to
create an extension in cascade when creation is attempted on a custom
schema:
=# create schema po;
CREATE SCHEMA
=# create extension hstore_plperl with schema po cascade;
NOTICE:  0: installing required extension hstore
LOCATION:  CreateExtension, extension.c:1483
NOTICE:  0: installing required extension plperl
LOCATION:  CreateExtension, extension.c:1483
ERROR:  42704: type hstore does not exist
LOCATION:  typenameType, parse_type.c:258
Time: 30.071 ms
To facilitate the life of users, I think that the parent extensions
should be created on the same schema as its child by default. In this
case hstore should be created in po, not public.

And actually by looking at the code I can guess that when DEFAULT
SCHEMA is used and that a non-relocatable extension with a schema
defined is created in cascade this will error-out as well. That's not
really user-friendly..

Hence, as a schema can only be specified in a control file for a
non-relocatable extension, I think that the schema name propagated to
the root extensions should be the one specified in the control file of
the child if it is defined in it instead of the one specified by user
(imagine for example an extension created in cascade that requires
adminpack, extension that can only be deployed in pg_catalog), and
have the root node use its own schema if it has one in its control
file by default.

For example in this case:
foo1 (WITH SCHEMA hoge) - foo2 (schema = pg_catalog) - foo2_1
|
|-- foo2_2
--- foo3 (no schema)
With this command:
CREATE EXTENSION foo1 WITH SCHEMA hoge;
foo3 is created on schema po. foo2, as well its required dependencies
foo2_1 and foo2_2 are created on pg_catalog.

Now DEFAULT SCHEMA is trying to achieve: Hey, I want to define foo2_1
and foo2_2 on schema hoge. This may be worth achieving (though IMO I
think that foo1 should have direct dependencies with foo2_1 and
foo2_2), but I think that we should decide what is the default
behavior we want, and implement the additional behavior in a second
patch, separated from the patch of this thread. Personally I am more a
fan of propagating to parent extensions the schema of the child and
not of its grand-child by default, but I am not alone here :)

 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.

That's done correctly.

+   /*
+* Propagate the CASCADE option and
add current extenssion
+* to the list for checking the cyclic
dependencies.
+*/
s/extenssion/extension/

+   /* Check for cyclic dependency between
extension. */
s/extension/extensions/

psql tab completion should be completed with cascade. See the part
with WITH SCHEMA in tab-complete.c.

Regards,
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-07-12 Thread David Fetter
On Tue, Jul 07, 2015 at 10:14:49AM -0700, David E. Wheeler wrote:
 On Jul 7, 2015, at 6:41 AM, Andres Freund and...@anarazel.de wrote:
 
  At the minimum I'd like to see that CREATE EXTENSION foo; would
  install install extension 'bar' if foo dependended on 'bar' if
  CASCADE is specified. Right now we always error out saying that
  the dependency on 'bar' is not fullfilled - not particularly
  helpful.
 
 +1
 
 If `yum install foo` also installs bar, and `pgxn install foo`
 downloads, builds, and installs bar, it makes sense to me that
 `CREATE EXTENSION foo` would install bar if it was available, and
 complain if it wasn’t.

This is this baseline sane behavior.  Getting the full dependency
tree, although it would be very handy, would require more
infrastructure than we have now.

Cheers,
David.
-- 
David Fetter da...@fetter.org http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-07-10 Thread Heikki Linnakangas

On 07/09/2015 07:05 PM, Petr Jelinek wrote:

On 2015-07-07 15:41, Andres Freund wrote:

On 2015-07-07 22:36:29 +0900, Fujii Masao wrote:

On Mon, Jun 15, 2015 at 7:50 AM, Petr Jelinek p...@2ndquadrant.com wrote:

Hi,

I am getting tired installing manually required extensions manually. I was
wondering if we might want to add option to CREATE SEQUENCE that would allow
automatic creation of the extensions required by the extension that is being
installed by the user.


I'm wondering how much helpful this feature is. Because, even if we can save
some steps for CREATE EXTENSION by using the feature, we still need to
manually find out, download and install all the extensions that the target
extension depends on. So isn't it better to implement the tool like yum, i.e.,
which performs all those steps almost automatically, rather than the proposed
feature? Maybe it's outside PostgreSQL core.


That doesn't seem to make much sense to me. Something like yum can't
install everything in all relevant databases. Sure, yum will be used to
install dependencies between extensions on the filesystem level.

At the minimum I'd like to see that CREATE EXTENSION foo; would install
install extension 'bar' if foo dependended on 'bar' if CASCADE is
specified. Right now we always error out saying that the dependency on
'bar' is not fullfilled - not particularly helpful.


That's what the proposed patch does (with slightly different syntax but
syntax is something that can be changed easily).


This seems quite reasonable, but I have to ask: How many extensions are 
there out there that depend on another extension? Off the top of my 
head, I can't think of any..


- Heikki


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-07-10 Thread Tom Lane
Andres Freund and...@anarazel.de writes:
 On July 10, 2015 4:16:59 PM GMT+02:00, Tom Lane t...@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.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-07-10 Thread Tom Lane
Michael Paquier michael.paqu...@gmail.com writes:
 On Fri, Jul 10, 2015 at 10:09 PM, Heikki Linnakangas hlinn...@iki.fi
 This seems quite reasonable, but I have to ask: How many extensions are
 there out there that depend on another extension? Off the top of my head, I
 can't think of any..

 With transforms there are such dependencies, and there are 3 in contrib/:
 hstore_plperl, hstore_plpython and ltree_plpython.

It's reasonable to expect that such cases will become more common as the
extension community matures.  It wasn't something we especially had to
worry about in the initial implementation of extensions, but it seems
totally worthwhile to me to add it now.

FWIW, I agree with using CASCADE to signal a request to create required
extension(s) automatically.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-07-10 Thread Andres Freund
On July 10, 2015 4:16:59 PM GMT+02:00, Tom Lane t...@sss.pgh.pa.us wrote:
Andres Freund and...@anarazel.de writes:
 I think we should copy the SCHEMA option here and document that we
use
 the same schema. But it needs to be done in a way that doesn't error
out
 if the extension is not relocatable...

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.

Andres

--- 
Please excuse brevity and formatting - I am writing this on my mobile phone.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-07-10 Thread Andres Freund
On 2015-06-15 00:50:08 +0200, Petr Jelinek wrote:
 + /* 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);

I think we should copy the SCHEMA option here and document that we use
the same schema. But it needs to be done in a way that doesn't error out
if the extension is not relocatable...


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-07-10 Thread Vladimir Borodin

 10 июля 2015 г., в 16:09, Heikki Linnakangas hlinn...@iki.fi написал(а):
 
 On 07/09/2015 07:05 PM, Petr Jelinek wrote:
 On 2015-07-07 15:41, Andres Freund wrote:
 On 2015-07-07 22:36:29 +0900, Fujii Masao wrote:
 On Mon, Jun 15, 2015 at 7:50 AM, Petr Jelinek p...@2ndquadrant.com wrote:
 Hi,
 
 I am getting tired installing manually required extensions manually. I was
 wondering if we might want to add option to CREATE SEQUENCE that would 
 allow
 automatic creation of the extensions required by the extension that is 
 being
 installed by the user.
 
 I'm wondering how much helpful this feature is. Because, even if we can 
 save
 some steps for CREATE EXTENSION by using the feature, we still need to
 manually find out, download and install all the extensions that the target
 extension depends on. So isn't it better to implement the tool like yum, 
 i.e.,
 which performs all those steps almost automatically, rather than the 
 proposed
 feature? Maybe it's outside PostgreSQL core.
 
 That doesn't seem to make much sense to me. Something like yum can't
 install everything in all relevant databases. Sure, yum will be used to
 install dependencies between extensions on the filesystem level.
 
 At the minimum I'd like to see that CREATE EXTENSION foo; would install
 install extension 'bar' if foo dependended on 'bar' if CASCADE is
 specified. Right now we always error out saying that the dependency on
 'bar' is not fullfilled - not particularly helpful.
 
 That's what the proposed patch does (with slightly different syntax but
 syntax is something that can be changed easily).
 
 This seems quite reasonable, but I have to ask: How many extensions are there 
 out there that depend on another extension? Off the top of my head, I can't 
 think of any..

pg_stat_kcache depends on pg_stat_statements, for example.

 
 - Heikki
 
 
 -- 
 Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org 
 mailto:pgsql-hackers@postgresql.org)
 To make changes to your subscription:
 http://www.postgresql.org/mailpref/pgsql-hackers 
 http://www.postgresql.org/mailpref/pgsql-hackers

--
May the force be with you…
https://simply.name



Re: [HACKERS] creating extension including dependencies

2015-07-10 Thread Tom Lane
Andres Freund and...@anarazel.de writes:
 I think we should copy the SCHEMA option here and document that we use
 the same schema. But it needs to be done in a way that doesn't error out
 if the extension is not relocatable...

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

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-07-09 Thread Petr Jelinek

On 2015-07-07 15:41, Andres Freund wrote:

On 2015-07-07 22:36:29 +0900, Fujii Masao wrote:

On Mon, Jun 15, 2015 at 7:50 AM, Petr Jelinek p...@2ndquadrant.com wrote:

Hi,

I am getting tired installing manually required extensions manually. I was
wondering if we might want to add option to CREATE SEQUENCE that would allow
automatic creation of the extensions required by the extension that is being
installed by the user.


I'm wondering how much helpful this feature is. Because, even if we can save
some steps for CREATE EXTENSION by using the feature, we still need to
manually find out, download and install all the extensions that the target
extension depends on. So isn't it better to implement the tool like yum, i.e.,
which performs all those steps almost automatically, rather than the proposed
feature? Maybe it's outside PostgreSQL core.


That doesn't seem to make much sense to me. Something like yum can't
install everything in all relevant databases. Sure, yum will be used to
install dependencies between extensions on the filesystem level.

At the minimum I'd like to see that CREATE EXTENSION foo; would install
install extension 'bar' if foo dependended on 'bar' if CASCADE is
specified. Right now we always error out saying that the dependency on
'bar' is not fullfilled - not particularly helpful.



That's what the proposed patch does (with slightly different syntax but 
syntax is something that can be changed easily).


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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-07-07 Thread David E. Wheeler
On Jul 7, 2015, at 6:41 AM, Andres Freund and...@anarazel.de wrote:

 At the minimum I'd like to see that CREATE EXTENSION foo; would install
 install extension 'bar' if foo dependended on 'bar' if CASCADE is
 specified. Right now we always error out saying that the dependency on
 'bar' is not fullfilled - not particularly helpful.

+1

If `yum install foo` also installs bar, and `pgxn install foo` downloads, 
builds, and installs bar, it makes sense to me that `CREATE EXTENSION foo` 
would install bar if it was available, and complain if it wasn’t.

Best,

David



smime.p7s
Description: S/MIME cryptographic signature


Re: [HACKERS] creating extension including dependencies

2015-07-07 Thread Andres Freund
On 2015-07-07 22:36:29 +0900, Fujii Masao wrote:
 On Mon, Jun 15, 2015 at 7:50 AM, Petr Jelinek p...@2ndquadrant.com wrote:
  Hi,
 
  I am getting tired installing manually required extensions manually. I was
  wondering if we might want to add option to CREATE SEQUENCE that would allow
  automatic creation of the extensions required by the extension that is being
  installed by the user.
 
 I'm wondering how much helpful this feature is. Because, even if we can save
 some steps for CREATE EXTENSION by using the feature, we still need to
 manually find out, download and install all the extensions that the target
 extension depends on. So isn't it better to implement the tool like yum, i.e.,
 which performs all those steps almost automatically, rather than the proposed
 feature? Maybe it's outside PostgreSQL core.

That doesn't seem to make much sense to me. Something like yum can't
install everything in all relevant databases. Sure, yum will be used to
install dependencies between extensions on the filesystem level.

At the minimum I'd like to see that CREATE EXTENSION foo; would install
install extension 'bar' if foo dependended on 'bar' if CASCADE is
specified. Right now we always error out saying that the dependency on
'bar' is not fullfilled - not particularly helpful.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] creating extension including dependencies

2015-07-07 Thread Fujii Masao
On Mon, Jun 15, 2015 at 7:50 AM, Petr Jelinek p...@2ndquadrant.com wrote:
 Hi,

 I am getting tired installing manually required extensions manually. I was
 wondering if we might want to add option to CREATE SEQUENCE that would allow
 automatic creation of the extensions required by the extension that is being
 installed by the user.

I'm wondering how much helpful this feature is. Because, even if we can save
some steps for CREATE EXTENSION by using the feature, we still need to
manually find out, download and install all the extensions that the target
extension depends on. So isn't it better to implement the tool like yum, i.e.,
which performs all those steps almost automatically, rather than the proposed
feature? Maybe it's outside PostgreSQL core.

Regards,

-- 
Fujii Masao


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] creating extension including dependencies

2015-06-14 Thread Petr Jelinek

Hi,

I am getting tired installing manually required extensions manually. I 
was wondering if we might want to add option to CREATE SEQUENCE that 
would allow automatic creation of the extensions required by the 
extension that is being installed by the user.


I also wrote some prototype patch that implements this.

I call it prototype mainly because the syntax (CREATE EXTENSION ... 
RECURSIVE) could be improved, I originally wanted to do something like 
INCLUDING DEPENDENCIES but that need news (unreserved) keyword and I 
don't think it's worth it, plus it's wordy.


--
 Petr Jelinek  http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services
From 7d5db7faae0b30a70ceae020638fa7779810339d Mon Sep 17 00:00:00 2001
From: Petr Jelinek pjmo...@pjmodos.net
Date: Sat, 13 Jun 2015 19:49:10 +0200
Subject: [PATCH] CREATE EXTENSION RECURSIVE

---
 doc/src/sgml/ref/create_extension.sgml | 11 
 src/backend/commands/extension.c   | 61 --
 src/backend/parser/gram.y  |  4 ++
 src/test/modules/test_extensions/.gitignore|  3 ++
 src/test/modules/test_extensions/Makefile  | 22 
 .../test_extensions/expected/test_extensions.out   | 16 ++
 .../test_extensions/sql/test_extensions.sql| 11 
 .../modules/test_extensions/test_ext1--1.0.sql |  0
 src/test/modules/test_extensions/test_ext1.control |  4 ++
 .../modules/test_extensions/test_ext2--1.0.sql |  0
 src/test/modules/test_extensions/test_ext2.control |  4 ++
 .../modules/test_extensions/test_ext3--1.0.sql |  0
 src/test/modules/test_extensions/test_ext3.control |  3 ++
 .../test_extensions/test_ext_cyclic1--1.0.sql  |  0
 .../test_extensions/test_ext_cyclic1.control   |  4 ++
 .../test_extensions/test_ext_cyclic2--1.0.sql  |  0
 .../test_extensions/test_ext_cyclic2.control   |  4 ++
 17 files changed, 143 insertions(+), 4 deletions(-)
 create mode 100644 src/test/modules/test_extensions/.gitignore
 create mode 100644 src/test/modules/test_extensions/Makefile
 create mode 100644 src/test/modules/test_extensions/expected/test_extensions.out
 create mode 100644 src/test/modules/test_extensions/sql/test_extensions.sql
 create mode 100644 src/test/modules/test_extensions/test_ext1--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext1.control
 create mode 100644 src/test/modules/test_extensions/test_ext2--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext2.control
 create mode 100644 src/test/modules/test_extensions/test_ext3--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext3.control
 create mode 100644 src/test/modules/test_extensions/test_ext_cyclic1--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext_cyclic1.control
 create mode 100644 src/test/modules/test_extensions/test_ext_cyclic2--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext_cyclic2.control

diff --git a/doc/src/sgml/ref/create_extension.sgml b/doc/src/sgml/ref/create_extension.sgml
index a1e7e4f8..d151fd6 100644
--- a/doc/src/sgml/ref/create_extension.sgml
+++ b/doc/src/sgml/ref/create_extension.sgml
@@ -25,6 +25,7 @@ CREATE EXTENSION [ IF NOT EXISTS ] replaceable class=parameterextension_name
 [ WITH ] [ SCHEMA replaceable class=parameterschema_name/replaceable ]
  [ VERSION replaceable class=parameterversion/replaceable ]
  [ FROM replaceable class=parameterold_version/replaceable ]
+ [ RECURSIVE ]
 /synopsis
  /refsynopsisdiv
 
@@ -139,6 +140,16 @@ CREATE EXTENSION [ IF NOT EXISTS ] replaceable class=parameterextension_name
/para
   /listitem
  /varlistentry
+
+ varlistentry
+  termliteralRECURSIVE//term
+  listitem
+   para
+Try to install extension including the required dependencies
+recursively.
+   /para
+  /listitem
+ /varlistentry
   /variablelist
  /refsect1
 
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 5cc74d0..4395519 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -46,6 +46,7 @@
 #include funcapi.h
 #include mb/pg_wchar.h
 #include miscadmin.h
+#include nodes/makefuncs.h
 #include storage/fd.h
 #include tcop/utility.h
 #include utils/builtins.h
@@ -1176,10 +1177,13 @@ CreateExtension(CreateExtensionStmt *stmt)
 	DefElem*d_schema = NULL;
 	DefElem*d_new_version = NULL;
 	DefElem*d_old_version = NULL;
+	DefElem	   *d_recursive = NULL;
 	char	   *schemaName;
 	Oid			schemaOid;
 	char	   *versionName;
 	char	   *oldVersionName;
+	bool		recursive = false;
+	List	   *recursive_parents = NIL;
 	Oid			extowner = GetUserId();
 	ExtensionControlFile *pcontrol;
 	ExtensionControlFile *control;
@@ -1263,6 +1267,14 @@ CreateExtension(CreateExtensionStmt *stmt)
 		 errmsg(conflicting or redundant options)));
 			d_old_version = defel;
 		}

Re: [HACKERS] creating extension including dependencies

2015-06-14 Thread Pavel Stehule
+1

Is it working in runtime too?
 Dne 15.6.2015 0:51 napsal uživatel Petr Jelinek p...@2ndquadrant.com:

 Hi,

 I am getting tired installing manually required extensions manually. I was
 wondering if we might want to add option to CREATE SEQUENCE that would
 allow automatic creation of the extensions required by the extension that
 is being installed by the user.

 I also wrote some prototype patch that implements this.

 I call it prototype mainly because the syntax (CREATE EXTENSION ...
 RECURSIVE) could be improved, I originally wanted to do something like
 INCLUDING DEPENDENCIES but that need news (unreserved) keyword and I don't
 think it's worth it, plus it's wordy.

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


 --
 Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
 To make changes to your subscription:
 http://www.postgresql.org/mailpref/pgsql-hackers