Re: [HACKERS] in-catalog Extension Scripts and Control parameters (templates?)
Jaime Casanova ja...@2ndquadrant.com writes: the name). I'm talking about get_ext_ver_list_from_catalog() which is a different function. Oh. I see it now. Sorry about that. It's blindly fixed in my git repo and I'm going to send an updated patch soon now™ which will include the fix. Thanks for insisting here… Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] in-catalog Extension Scripts and Control parameters (templates?)
On Sat, Jul 6, 2013 at 2:25 PM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Jaime Casanova ja...@2ndquadrant.com writes: not sure if you're wrong. but at the very least, you miss a heap_freetuple(oldtup) there, because get_catalog_object_by_oid() Well, oldtup can be either a syscache copy or a heap tuple. I've been looking at other call sites and they don't free their tuple either. So I'm leaving it at that for now. no, that code is not unchanged because function get_ext_ver_list_from_catalog() comes from your patch. Yes. Here's the relevant hunk: No. That's get_ext_ver_list_from_files() and that function is unchanged (except for the name). I'm talking about get_ext_ver_list_from_catalog() which is a different function. -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación Phone: +593 4 5107566 Cell: +593 987171157 -- 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] in-catalog Extension Scripts and Control parameters (templates?)
Jaime Casanova ja...@2ndquadrant.com writes:
not sure if you're wrong. but at the very least, you miss a
heap_freetuple(oldtup) there, because get_catalog_object_by_oid()
Well, oldtup can be either a syscache copy or a heap tuple. I've been
looking at other call sites and they don't free their tuple either. So
I'm leaving it at that for now.
no, that code is not unchanged because function
get_ext_ver_list_from_catalog() comes from your patch.
Yes. Here's the relevant hunk:
***
*** 997,1003 get_nearest_unprocessed_vertex(List *evi_list)
* the versions that can be reached in one step from that version.
*/
static List *
! get_ext_ver_list(ExtensionControlFile *control)
{
List *evi_list = NIL;
int extnamelen = strlen(control-name);
--- 1093,1099
* the versions that can be reached in one step from that version.
*/
static List *
! get_ext_ver_list_from_files(ExtensionControl *control)
{
List *evi_list = NIL;
int extnamelen = strlen(control-name);
***
So the content of the function has not changed. I'm not opposed to
trying to fix it, I just don't think it would be wise to do so as part
of the extension templates patch, as I do believe that the problem
should manifest itself in head too: it's the same code under a new
function's name.
Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
--
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] in-catalog Extension Scripts and Control parameters (templates?)
On Mon, Jun 24, 2013 at 6:20 AM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Jaime Casanova ja...@2ndquadrant.com writes: just tried to build this one, but it doesn't apply cleanly anymore... specially the ColId_or_Sconst contruct in gram.y Please find attached a new version of the patch, v7, rebased to current master tree and with some more cleanup. I've been using the new grammar entry NonReservedWord_or_Sconst, I'm not sure about that. Hi, code review (haven't read all the code) - The error message in aclchk.c:5175 isn't very clear, i mean the user should see something better than uptmpl if (!HeapTupleIsValid(tuple)) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_OBJECT), errmsg(extension uptmpl with OID %u does not exist, ext_uptmpl_oid))); - In alter.c you made AlterObjectRename_internal non static and replaced a SearchSysCache1 call with a get_catalog_object_by_oid one. Now, in its comment that function says that is for simple cases. And because of the things you're doing it seems to me this isn't a simple case. Maybe instead of modifying it you should create other function RenameExtensionTemplateInternal, just like tablecmd.c does? btw, get_catalog_object_by_oid will execute a SearchSysCacheCopy1 so should be calling heap_freetuple(oldtuple) - This is a typo i guess: AtlerExtensionTemplateRename - In event_triggers.c, it seems the intention was to keep the event_trigger_support array in order, any reason to for not doing it? - extension.c: In function ‘get_ext_ver_list_from_catalog’: extension.c:1150:25: warning: variable ‘evi’ set but not used [-Wunused-but-set-variable] Functionality === i tried this: create template for extension test version 'abc' with (nosuperuser) as $$ create function f1(i int) returns int as $_$ select 1; $_$ language sql; $$; CREATE TEMPLATE FOR EXTENSION create template for extension test from 'abc' to 'xyz' with (nosuperuser) as $$ create function f2(i int) returns int as $_$ select 1; $_$ language sql; $$; CREATE TEMPLATE FOR EXTENSION create template for extension test from 'xyz' to '123' with (nosuperuser) as $$ create function f3(i int) returns int as $_$ select 1; $_$ language sql; $$; CREATE TEMPLATE FOR EXTENSION create extension test version '123'; CREATE EXTENSION postgres=# \df List of functions Schema | Name | Result data type | Argument data types | Type +--+--+-+-- (0 rows) Actually, what this did was to create an 123 schema and it puts the functions there. But that schema is inaccesible and undroppable: select * from 123.f1(1); ERROR: schema 123 does not exist drop schema 123; ERROR: schema 123 does not exist -- postgres=# create template for extension test2 version '1.0' with (nosuperuser) as $$ create function f1(i int) returns int as $_$ select 1; $_$ language sql; $$; CREATE TEMPLATE FOR EXTENSION postgres=# create template for extension test2 from '1.0' to '1.1' with (nosuperuser) as $$ create function f2(i int) returns int as $_$ select 1; $_$ language sql; $$; CREATE TEMPLATE FOR EXTENSION postgres=# create template for extension test2 from '1.0' to '2.1' with (nosuperuser) as $$ create function f3(i int) returns int as $_$ select 1; $_$ language sql; $$; CREATE TEMPLATE FOR EXTENSION postgres=# create extension test2 version '2.1'; CREATE EXTENSION In this case only f1() and f3() exists, because the extension is going from 1.0 to 2.1. is this expected? and, yes... the functions are in the schema 2.1 -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación Phone: +593 4 5107566 Cell: +593 987171157 -- 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] in-catalog Extension Scripts and Control parameters (templates?)
On Thu, Jul 4, 2013 at 2:42 AM, Jaime Casanova ja...@2ndquadrant.com wrote: create extension test version '123'; CREATE EXTENSION postgres=# \df List of functions Schema | Name | Result data type | Argument data types | Type +--+--+-+-- (0 rows) Actually, what this did was to create an 123 schema and it puts the functions there. But that schema is inaccesible and undroppable: and dropping the extension let the schema around -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación Phone: +593 4 5107566 Cell: +593 987171157 -- 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] in-catalog Extension Scripts and Control parameters (templates?)
On Thu, Jul 4, 2013 at 12:46 AM, Jaime Casanova ja...@2ndquadrant.com wrote: On Thu, Jul 4, 2013 at 2:42 AM, Jaime Casanova ja...@2ndquadrant.com wrote: create extension test version '123'; CREATE EXTENSION postgres=# \df List of functions Schema | Name | Result data type | Argument data types | Type +--+--+-+-- (0 rows) Actually, what this did was to create an 123 schema and it puts the functions there. But that schema is inaccesible and undroppable: and dropping the extension let the schema around Hm? I guess '123' is not schema, but it's version. -- Hitoshi Harada -- 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] in-catalog Extension Scripts and Control parameters (templates?)
Hi,
Please find attached version 8 of the patch, with fixes for almost all
reported problems. Thanks a lot to you reviewers for finding them!
I need some help with:
- toast tables for new catalog tables
- extension.c:1150:25: warning: variable ‘evi’ set but not used
See details below.
Hitoshi Harada umi.tan...@gmail.com writes:
- Why do we need with() clause even if I don't need it?
Not needed anymore, regression test addded to cover the new grammar form.
foo=# create template for extension ex2 version '1.0' with (requires ex1)
as $$ $$;
ERROR: template option requires takes an argument
Not sure how to handle the grammar for that case, given that I'm using
the same tricks as in the CREATE ROLE options in order to avoid adding
new keywords in the patch.
- create template ex2, create extension ex2, alter template ex2 rename to
ex3, create extension ex3, drop template ex3;
foo=# drop template for extension ex3 version '1.0';
ERROR: cannot drop unrecognized object 3179 16429 0 because other objects
depend on it
Fixed in the attached.
- a template that is created in another template script does not appear to
depend on the parent template.
What I understand you meant is when doing CREATE TEMPLATE FOR EXTENSION
from within a CREATE EXTENSION script. That is now covered in the
attached.
- Without control file/template, attempt to create a new extension gives:
foo=# create extension plv8;
ERROR: extension plv8 has no default control template
but can it be better, like extension plv8 has no default control file or
template?
Updated the error message, it now looks like that:
create extension plv8;
ERROR: 42704: Extension plv8 is not available from
/Users/dim/pgsql/ddl/share/extension nor as a template
LOCATION: read_extension_control, extension.c:676
STATEMENT: create extension plv8;
- Looks like both tables don't have toast tables. Some experiment gives:
ERROR: row is too big: size 8472, maximum size 8160
Not sure why. That's not fixed in the attached.
Alvaro Herrera alvhe...@2ndquadrant.com writes:
Very minor comment here: these SGML id tags:
+ refentry id=SQL-ALTEREXTENSIONTEMPLATE
Changed to SQL-ALTER-TEMPLATE-FOR-EXTENSION, same with CREATE and DROP
commands, update the cross references.
Alvaro Herrera alvhe...@2ndquadrant.com writes:
What if read_extension_control_file() fails because of an out-of-memory
error? I think you need to extend that function to have a more useful
API, not rely on it raising a specific error. There is at least one
more case when you're calling that function in the same way.
Good point. I'm now using something really simple:
if (access(get_extension_control_filename(extname), F_OK) == 0)
{
ereport(ERROR,
(errcode(ERRCODE_DUPLICATE_OBJECT),
errmsg(extension \%s\ is already
available, extname)));
}
After pondering about it for a while, that doesn't strike me as a
modularity violation severe enough to warrant more complex changes.
Jaime Casanova ja...@2ndquadrant.com writes:
- The error message in aclchk.c:5175 isn't very clear, i mean the user
should see something better than uptmpl
Fixed in the attached.
- In alter.c you made AlterObjectRename_internal non static and
replaced a SearchSysCache1 call with a get_catalog_object_by_oid one.
Now, in its comment that function says that is for simple cases. And
because of the things you're doing it seems to me this isn't a simple
case. Maybe instead of modifying it you should create other function
RenameExtensionTemplateInternal, just like tablecmd.c does?
The get_catalog_object_by_oid() is doing a SearchSysCache1 when that's
relevant and possible, so I don't think I changed enough things around
to warrant a different API. I'm open to hearing about why I'm wrong if
that's the case, though.
- This is a typo i guess: AtlerExtensionTemplateRename
Fixed in the attached.
- In event_triggers.c, it seems the intention was to keep the
event_trigger_support array in order, any reason to for not doing it?
Fixed in the attached.
- extension.c: In function ‘get_ext_ver_list_from_catalog’:
extension.c:1150:25: warning: variable ‘evi’ set but not used
[-Wunused-but-set-variable]
I don't have the warning here, and that code is unchanged from master's
branch, only the name of the function did change. Do you have the same
warning with master? which version of gcc are you using?
Actually, what this did was to create an 123 schema and it puts the
functions there.
There was a fault in the way default values are assigned to auxilliary
control entries in pg_extension_control when creating a template for
updating an extension. That's been fixed in the attached, a a new
regression test has been added.
In this case only f1() and f3() exists, because the extension is going
from 1.0 to 2.1. is this expected?
You can use the following SQL statement to debug
Re: [HACKERS] in-catalog Extension Scripts and Control parameters (templates?)
On Thu, Jul 4, 2013 at 7:32 AM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: - In alter.c you made AlterObjectRename_internal non static and replaced a SearchSysCache1 call with a get_catalog_object_by_oid one. Now, in its comment that function says that is for simple cases. And because of the things you're doing it seems to me this isn't a simple case. Maybe instead of modifying it you should create other function RenameExtensionTemplateInternal, just like tablecmd.c does? The get_catalog_object_by_oid() is doing a SearchSysCache1 when that's relevant and possible, so I don't think I changed enough things around to warrant a different API. I'm open to hearing about why I'm wrong if that's the case, though. not sure if you're wrong. but at the very least, you miss a heap_freetuple(oldtup) there, because get_catalog_object_by_oid() - extension.c: In function ‘get_ext_ver_list_from_catalog’: extension.c:1150:25: warning: variable ‘evi’ set but not used [-Wunused-but-set-variable] I don't have the warning here, and that code is unchanged from master's branch, only the name of the function did change. Do you have the same warning with master? which version of gcc are you using? no, that code is not unchanged because function get_ext_ver_list_from_catalog() comes from your patch. it's seems the thing is that function get_ext_ver_info() is append a new ExtensionVersionInfo which is then returned and assigned to an *evi pointer that is never used. i'm sure that evi in line 1150 is only because you need to receive the returned value. Maybe you could use (void) get_ext_ver_info() (or it should be (void *)?) to avoid that assignment and keep compiler quiet PS: i'm on gcc (Debian 4.7.2-5) 4.7.2 -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación Phone: +593 4 5107566 Cell: +593 987171157 -- 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] in-catalog Extension Scripts and Control parameters (templates?)
Very minor comment here: these SGML id tags: + refentry id=SQL-ALTEREXTENSIONTEMPLATE are pretty important, because they become the URL for the specific page in the reference docs. So I think you should fix them to be the correct spelling of the command alter template for extension, and also perhaps add an hyphen or two. Maybe SQL-ALTER-EXTENSION-FOR-TEMPLATE. (We're inconsistent about adding hyphens; most URLs don't have hyphens after then sql- bit, so sql-altertablespace, but we have some examples of the opposite such as sql-commit-prepared and sql-drop-owned.) -- Álvaro Herrerahttp://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] in-catalog Extension Scripts and Control parameters (templates?)
I think this is unlikely to work reliably:
+ PG_TRY();
+ {
+ ExtensionControl *control = read_extension_control_file(extname);
+
+ if (control)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_DUPLICATE_OBJECT),
+errmsg(extension \%s\ is already available, extname)));
+ }
+ }
+ PG_CATCH();
+ {
+ /* no control file found is good news for us */
+ }
+ PG_END_TRY();
What if read_extension_control_file() fails because of an out-of-memory
error? I think you need to extend that function to have a more useful
API, not rely on it raising a specific error. There is at least one
more case when you're calling that function in the same way.
It'd probably work to have a boolean return (found/not found), and
return the ExtensionControl structure through a pointer.
--
Álvaro Herrerahttp://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] in-catalog Extension Scripts and Control parameters (templates?)
On Mon, Jun 24, 2013 at 4:20 AM, Dimitri Fontaine dimi...@2ndquadrant.frwrote: Jaime Casanova ja...@2ndquadrant.com writes: just tried to build this one, but it doesn't apply cleanly anymore... specially the ColId_or_Sconst contruct in gram.y Please find attached a new version of the patch, v7, rebased to current master tree and with some more cleanup. I've been using the new grammar entry NonReservedWord_or_Sconst, I'm not sure about that. I played a bit with this patch. - If I have control file that has the same name as template, create extension picks up control file? Is this by design? - Though control file is kind of global information among different databases, pg_extension_template is not. Sounds a little weird to me. - Why do we need with() clause even if I don't need it? - I copied and paste from my plv8.control file to template script, and MODULE_PATHNAME didn't work. By design? - foo=# create template for extension ex2 version '1.0' with (requires ex1) as $$ $$; ERROR: template option requires takes an argument - create template ex2, create extension ex2, alter template ex2 rename to ex3, create extension ex3, drop template ex3; foo=# drop template for extension ex3 version '1.0'; ERROR: cannot drop unrecognized object 3179 16429 0 because other objects depend on it - a template that is created in another template script does not appear to depend on the parent template. - Without control file/template, attempt to create a new extension gives: foo=# create extension plv8; ERROR: extension plv8 has no default control template but can it be better, like extension plv8 has no default control file or template? - Do we really need separate tables, pg_extension_template and pg_extension_control? - Looks like both tables don't have toast tables. Some experiment gives: ERROR: row is too big: size 8472, maximum size 8160 Thanks, -- Hitoshi Harada
Re: [HACKERS] in-catalog Extension Scripts and Control parameters (templates?)
Hi, Thanks a lot for your review! Some answers here, new version of the patch with fixes by tuesday. Hitoshi Harada umi.tan...@gmail.com writes: - If I have control file that has the same name as template, create extension picks up control file? Is this by design? Yes. That should allow to answer most of Heikki's security complaint, but isn't enough to allow us to open the feature to non superuser if I understand correctly. - Though control file is kind of global information among different databases, pg_extension_template is not. Sounds a little weird to me. I think that's a limitation of the old model and we don't want to turn templates for extensions into being shared catalogs. At least that's my understanding of the design consensus. - Why do we need with() clause even if I don't need it? Will have a fresh look, thanks. - I copied and paste from my plv8.control file to template script, and MODULE_PATHNAME didn't work. By design? Yes. MODULE_PATHNAME is deprecated by the control file parameter of the same name, and there's no reason to use it in extension templates even if we got to support C coded extensions in them, which is not the case now. foo=# create template for extension ex2 version '1.0' with (requires ex1) as $$ $$; ERROR: template option requires takes an argument Will see about that. - create template ex2, create extension ex2, alter template ex2 rename to ex3, create extension ex3, drop template ex3; foo=# drop template for extension ex3 version '1.0'; ERROR: cannot drop unrecognized object 3179 16429 0 because other objects depend on it Well, if I'm following, you're trying to remove a non-existing object. I guess you would prefer a better error message, right? - a template that is created in another template script does not appear to depend on the parent template. I don't think that should be automatically the case, even if I admit I didn't think about that case. - Without control file/template, attempt to create a new extension gives: foo=# create extension plv8; ERROR: extension plv8 has no default control template but can it be better, like extension plv8 has no default control file or template? Will rework. - Do we really need separate tables, pg_extension_template and pg_extension_control? Yes, to be able to have the same feature as we have today with auxilliary control files, that is change properties of the extension from a version to the next. - Looks like both tables don't have toast tables. Some experiment gives: ERROR: row is too big: size 8472, maximum size 8160 Will fix in next version of the patch. Thanks, -- Dimitri Fontaine06 63 07 10 78 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] in-catalog Extension Scripts and Control parameters (templates?)
On Thu, Jun 27, 2013 at 2:49 AM, Dimitri Fontaine dimi...@2ndquadrant.frwrote: Hi, Thanks a lot for your review! Some answers here, new version of the patch with fixes by tuesday. Hitoshi Harada umi.tan...@gmail.com writes: - create template ex2, create extension ex2, alter template ex2 rename to ex3, create extension ex3, drop template ex3; foo=# drop template for extension ex3 version '1.0'; ERROR: cannot drop unrecognized object 3179 16429 0 because other objects depend on it Well, if I'm following, you're trying to remove a non-existing object. I guess you would prefer a better error message, right? Right. unrecognized object x y z doesn't look good. - a template that is created in another template script does not appear to depend on the parent template. I don't think that should be automatically the case, even if I admit I didn't think about that case. Really? My understanding is everything that is created under extension depends on the extension, which depends on the template. Why only template is exception? -- Hitoshi Harada
Re: [HACKERS] in-catalog Extension Scripts and Control parameters (templates?)
Hitoshi Harada umi.tan...@gmail.com writes: - a template that is created in another template script does not appear to depend on the parent template. I don't think that should be automatically the case, even if I admit I didn't think about that case. Really? My understanding is everything that is created under extension depends on the extension, which depends on the template. Why only template is exception? Oh sorry, I understood you meant at CREATE TEMPLATE FOR EXTENSION time rather than at CREATE EXTENSION time, and that you were refering to dependency as in the require control parameter. The pg_depend entry against the extension should be there, will fix. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] in-catalog Extension Scripts and Control parameters (templates?)
On Thu, Jun 27, 2013 at 5:49 AM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: I think that's a limitation of the old model and we don't want to turn templates for extensions into being shared catalogs. At least that's my understanding of the design consensus. I agree. -- 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] in-catalog Extension Scripts and Control parameters (templates?)
Jaime Casanova ja...@2ndquadrant.com writes: just tried to build this one, but it doesn't apply cleanly anymore... specially the ColId_or_Sconst contruct in gram.y Please find attached a new version of the patch, v7, rebased to current master tree and with some more cleanup. I've been using the new grammar entry NonReservedWord_or_Sconst, I'm not sure about that. In particular it seems I had forgotten to fix the owner support in pg_dump as noted by Heikki, it's now properly addressed in the attached version. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support templates.v7.patch.gz Description: Binary data -- 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] in-catalog Extension Scripts and Control parameters (templates?)
On Wed, Apr 3, 2013 at 8:29 AM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Hi, Dimitri Fontaine dimi...@2ndquadrant.fr writes: Documentation doesn't build, multiple errors. In addition to the reference pages, there should be a section in the main docs about these templates. I'm now working on that, setting up the documentation tool set. Fixed in the attached version 6 of the patch. just tried to build this one, but it doesn't apply cleanly anymore... specially the ColId_or_Sconst contruct in gram.y -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación Phone: +593 4 5107566 Cell: +593 987171157 -- 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] in-catalog Extension Scripts and Control parameters (templates?)
Jaime Casanova ja...@2ndquadrant.com writes: just tried to build this one, but it doesn't apply cleanly anymore... specially the ColId_or_Sconst contruct in gram.y Will rebase tomorrow, thanks for the notice! -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] in-catalog Extension Scripts and Control parameters (templates?)
Hi, Dimitri Fontaine dimi...@2ndquadrant.fr writes: Documentation doesn't build, multiple errors. In addition to the reference pages, there should be a section in the main docs about these templates. I'm now working on that, setting up the documentation tool set. Fixed in the attached version 6 of the patch. I couldn't get to fix the documentation build tool chain on my main workstation, so I had to revive a VM where I was lucky enough to find my old setup was still working, all with shared directories, VPATH build setup etc. Anyways. I also expanded a little on the docs we did have, adding a section in the main extension chapter and some example in the CREATE TEMPLATE FOR EXTENSION reference page. That version has no flaw that I'm aware of and implements the design we reached after almost 2 years on the topic which is more complex than it would appear at first sight. I hope that you will like the patch if not the feature itself, as I have big plans for it in the near future. Many thanks to all involved for helping with the design and the reviewing, regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support templates.v6.patch.gz Description: Binary data -- 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] in-catalog Extension Scripts and Control parameters (templates?)
Heikki Linnakangas hlinnakan...@vmware.com writes: I'm quite worried about the security ramifications of this patch. Today, if you're not sure if a system has e.g sslinfo installed, you can safely just run CREATE EXTENSION sslinfo. With this patch, that's no longer true, because foo might not be the extension you're looking for. Mallory With the attached patch, you can't create a template for an extension that is already available, so to protect against your scenario you only have to make sslinfo available. Please also note that when actually installing the sslinfo extension, the one from the system will get prefered over the one from the templates, should you have both available. Now, I can see why you would still think it's not enough. Baring better ideas, the current patch restricts the feature to superusers. Documentation doesn't build, multiple errors. In addition to the reference pages, there should be a section in the main docs about these templates. I'm now working on that, setting up the documentation tool set. postgres=# create template for extension myextension version '1.0' with () as 'foobar'; CREATE TEMPLATE FOR EXTENSION postgres=# create extension myextension; ERROR: syntax error at or near foobar LINE 1: create extension myextension; ^ Confusing error message. Introducing a syntax error in hstore--1.1.sql leads to the same message. Even if we agree that it must be changed, I think that's for another patch. ~# create extension hstore; ERROR: syntax error at or near foobar at character 115 STATEMENT: create extension hstore; ERROR: syntax error at or near foobar postgres=# create template for extension myextension version '1.0' with () as $$create table foobar(i int4) $$; CREATE TEMPLATE FOR EXTENSION postgres=# create extension myextension; CREATE EXTENSION postgres=# select * from foobar; ERROR: relation foobar does not exist LINE 1: select * from foobar; ^ Where did that table go? The control-schema was not properly initialized when not given by the user. That's fixed, and I added a regression test. Ah, here... Where did that 1.0 schema come from? Fixed too, was the same bug. postgres= create template for extension myextension version '1.0' with (schema public) as $$ create function evilfunc() returns int4 AS evilfunc' language internal; $$; CREATE TEMPLATE FOR EXTENSION postgres= create extension myextension version '1.0';ERROR: permission denied for language internal postgres= drop template for extension myextension version '1.0'; ERROR: extension with OID 16440 does not exist Something wrong with catalog caching. Works for me, I couldn't reproduce the bug here, working from Álvaro's version 4 of the patch. Maybe he did already fix it, and you tested my version 3? $ make -s install /usr/bin/install: cannot stat `./hstore--1.0.sql': No such file or directory make: *** [install] Error 1 Installing hstore fails. Fixed in the attached. Seeing that makes me think that you actually used Álvaro's version 4, though. postgres= create template for extension sslinfo version '1.0' with (schema public) as $$ create function evilfunc() returns int4 AS evilfunc' language internal; $$; ERROR: extension sslinfo is already available Expected. postgres= create template for extension sslinfo2 version '1.0' with (schema public) as $$ create function evilfunc() returns int4 AS evilfunc' language internal; $$; CREATE TEMPLATE FOR EXTENSION postgres= alter template for extension sslinfo2 rename to sslinfo; ALTER TEMPLATE FOR EXTENSION If we check for an existing extension at CREATE, should also check for that in ALTER ... RENAME TO. Indeed, good catch. Fixed in the attached version 5 of the patch. I didn't add a regression test for that case, because we need to know which extensions are available when we try to obtain this error, and I don't know how to do that. We could create a template for the extension foobar, then foobar2, then rename foobar2 to foobar, but that wouldn't exercise the same code path. Thanks again for your reviewing, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support templates.v5.patch.gz Description: Binary data -- 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] in-catalog Extension Scripts and Control parameters (templates?)
Hi, Thanks you for testing and reporting those strange bugs, I should be able to fix them by Tuesday at the earliest. Heikki Linnakangas hlinnakan...@vmware.com writes: create template for extension sslinfo version '1.0' with (schema public) as $$ DO EVIL STUFF $$; What you're saying is that we should restrict the capability to superusers only, where I didn't think about those security implications before and though that it wouldn't be a necessary limitation. I will add the usual superuser() checks in the next version of the patch. Documentation doesn't build, multiple errors. In addition to the reference pages, there should be a section in the main docs about these templates. I really would like for a simpler setup to build documentation on my OS of choice, and realize that's no excuse. Will clean that up in next version of the patch. postgres=# create template for extension myextension version '1.0' with () as 'foobar'; CREATE TEMPLATE FOR EXTENSION postgres=# create extension myextension; ERROR: syntax error at or near foobar LINE 1: create extension myextension; ^ Confusing error message. Do we need to compute a different error message when applying the script from a template or from a file on-disk? Also please keep in mind that those error messages are typically to be seen by the extension's author. postgres=# create template for extension myextension version '1.0' with () as $$create table foobar(i int4) $$; CREATE TEMPLATE FOR EXTENSION postgres=# create extension myextension; CREATE EXTENSION postgres=# select * from foobar; ERROR: relation foobar does not exist LINE 1: select * from foobar; ^ Where did that table go? Well that's not the answer I wanted to make, but: select c.oid, relname, nspname from pg_class c join pg_namespace n on n.oid = c.relnamespace where relname ~ 'foobar'; oid | relname | nspname ---+-+- 41412 | foobar | 1.0 (1 row) Ah, here... Where did that 1.0 schema come from? I need to sort that out. Didn't have that problem in my tests (included in the regression tests), will add your test case and see about fixing that bug in the next version of the patch. postgres= create template for extension myextension version '1.0' with (schema public) as $$ create function evilfunc() returns int4 AS evilfunc' language internal; $$; CREATE TEMPLATE FOR EXTENSION postgres= create extension myextension version '1.0';ERROR: permission denied for language internal postgres= drop template for extension myextension version '1.0'; ERROR: extension with OID 16440 does not exist Something wrong with catalog caching. Or something wrong with dependencies maybe… will have a look at that too, and add some regression tests. $ make -s install /usr/bin/install: cannot stat `./hstore--1.0.sql': No such file or directory make: *** [install] Error 1 Installing hstore fails. Works for me. Anyway that part was to show up how we could have been managing the hstore 1.1 update in the past, I don't intend for it to get commited unless specifically asked to do so. I guess I should now remove hstore changes from the patch now, and will do so in the next version of the patch. If we check for an existing extension at CREATE, should also check for that in ALTER ... RENAME TO. Indeed. Will fix that too. Also: pg_dump does not dump the owner of an extension template correctly. Will look into that too. Thanks for your reviewing and testing, sorry to have missed those bugs. The new version of the patch, early next week, will include fixes for all of those and some more testing. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] in-catalog Extension Scripts and Control parameters (templates?)
On Wed, Mar 27, 2013 at 10:32 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Surely creating an extension template must be a superuser-only operation, in which case this is an issue because Mallory could also have just blown up the world directly if he's already a superuser anyway. Yeah .. (except this is NOT an issue) If the current patch isn't enforcing that, it's 100% broken. Even if it's not enforcing that, it's not 100% broken, it only contains one more bug we need to fix. Sure. I didn't mean that such a mistake would make the patch unsalvageable, only that it would be disastrous from a security point of view. But as you say, pretty easy to fix. -- 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] in-catalog Extension Scripts and Control parameters (templates?)
On 15.03.2013 23:00, Alvaro Herrera wrote:
Dimitri Fontaine wrote:
Please find attached v3 of the Extension Templates patch, with full
pg_dump support thanks to having merged default_full_version, appended
with some regression tests now that it's possible.
Here's a rebased version; there were some merge conflicts with master.
I also fixed some compiler warnings.
I'm quite worried about the security ramifications of this patch. Today,
if you're not sure if a system has e.g sslinfo installed, you can safely
just run CREATE EXTENSION sslinfo. With this patch, that's no longer
true, because foo might not be the extension you're looking for.
Mallory might've done this:
create template for extension sslinfo version '1.0' with (schema public)
as $$ DO EVIL STUFF $$;
Now if you run CREATE EXTENSION sslinfo as superuser, you've been
compromised. This is not only a problem when sitting at a psql console,
it also just became really dangerous to run pg_dump backups without
ensuring that all the extensions are installed beforehand. That's
exactly the situation we wanted to avoid when extensions were introduced
in the first place.
Things get even more complicated if there's version 1.0 of sslinfo
already installed, and you create an extension template for sslinfo
version 1.1. Is that possible? How does it behave?
Below are some random bugs that I bumped into while testing. These could
be fixed, but frankly I think this should be rejected for security reasons.
Documentation doesn't build, multiple errors. In addition to the
reference pages, there should be a section in the main docs about these
templates.
postgres=# create template for extension myextension version '1.0' with () as
'foobar';
CREATE TEMPLATE FOR EXTENSION
postgres=# create extension myextension;
ERROR: syntax error at or near foobar
LINE 1: create extension myextension;
^
Confusing error message.
postgres=# create template for extension myextension version '1.0' with () as
$$create table foobar(i int4) $$;
CREATE TEMPLATE FOR EXTENSION
postgres=# create extension myextension;
CREATE EXTENSION
postgres=# select * from foobar;
ERROR: relation foobar does not exist
LINE 1: select * from foobar;
^
Where did that table go?
postgres=# create template for extension myextension version '1.0' with () as
$$ create function myfunc() returns int4 AS $f$ select 123; $f$ language sql;
$$;
CREATE TEMPLATE FOR EXTENSION
postgres=# create extension myextension version '1.0';
CREATE EXTENSION
postgres=# select * from pg_namespace; nspname | nspowner |
nspacl
+--+---
pg_toast | 10 |
pg_temp_1 | 10 |
pg_toast_temp_1| 10 |
pg_catalog | 10 | {heikki=UC/heikki,=U/heikki}
public | 10 | {heikki=UC/heikki,=UC/heikki}
information_schema | 10 | {heikki=UC/heikki,=U/heikki}
1.0| 10 |
(7 rows)
Ah, here... Where did that 1.0 schema come from?
postgres= create template for extension myextension version '1.0' with (schema
public) as $$ create function evilfunc() returns int4 AS 'evilfunc' language
internal; $$;
CREATE TEMPLATE FOR EXTENSION
postgres= create extension myextension version '1.0';ERROR: permission denied
for language internal
postgres= drop template for extension myextension version '1.0';
ERROR: extension with OID 16440 does not exist
Something wrong with catalog caching.
$ make -s install
/usr/bin/install: cannot stat `./hstore--1.0.sql': No such file or directory
make: *** [install] Error 1
Installing hstore fails.
postgres= create template for extension sslinfo version '1.0' with (schema
public) as $$ create function evilfunc() returns int4 AS 'evilfunc' language
internal; $$;
ERROR: extension sslinfo is already available
postgres= create template for extension sslinfo2 version '1.0' with (schema
public) as $$ create function evilfunc() returns int4 AS 'evilfunc' language
internal; $$;
CREATE TEMPLATE FOR EXTENSION
postgres= alter template for extension sslinfo2 rename to sslinfo;
ALTER TEMPLATE FOR EXTENSION
If we check for an existing extension at CREATE, should also check for
that in ALTER ... RENAME TO.
- 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] in-catalog Extension Scripts and Control parameters (templates?)
On Wed, Mar 27, 2013 at 10:16 AM, Heikki Linnakangas hlinnakan...@vmware.com wrote: I'm quite worried about the security ramifications of this patch. Today, if you're not sure if a system has e.g sslinfo installed, you can safely just run CREATE EXTENSION sslinfo. With this patch, that's no longer true, because foo might not be the extension you're looking for. Mallory might've done this: create template for extension sslinfo version '1.0' with (schema public) as $$ DO EVIL STUFF $$; Surely creating an extension template must be a superuser-only operation, in which case this is an issue because Mallory could also have just blown up the world directly if he's already a superuser anyway. If the current patch isn't enforcing that, it's 100% broken. -- 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] in-catalog Extension Scripts and Control parameters (templates?)
On 27.03.2013 16:16, Heikki Linnakangas wrote: Below are some random bugs that I bumped into while testing. These could be fixed, but frankly I think this should be rejected for security reasons. Also: pg_dump does not dump the owner of an extension template correctly. - 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] in-catalog Extension Scripts and Control parameters (templates?)
Robert Haas escribió: On Wed, Mar 27, 2013 at 10:16 AM, Heikki Linnakangas hlinnakan...@vmware.com wrote: I'm quite worried about the security ramifications of this patch. Today, if you're not sure if a system has e.g sslinfo installed, you can safely just run CREATE EXTENSION sslinfo. With this patch, that's no longer true, because foo might not be the extension you're looking for. Mallory might've done this: create template for extension sslinfo version '1.0' with (schema public) as $$ DO EVIL STUFF $$; Surely creating an extension template must be a superuser-only operation, in which case this is an issue because Mallory could also have just blown up the world directly if he's already a superuser anyway. Yeah .. (except this is NOT an issue) If the current patch isn't enforcing that, it's 100% broken. Even if it's not enforcing that, it's not 100% broken, it only contains one more bug we need to fix. -- Álvaro Herrerahttp://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] in-catalog Extension Scripts and Control parameters (templates?)
Alvaro Herrera alvhe...@2ndquadrant.com writes: Here's a rebased version; there were some merge conflicts with master. Thanks! I also fixed some compiler warnings. I haven't reviewed the patch in any detail yet. One thing that jump at me from the code style perspective is the strange way it deals with isnull from heap_getattr. (I think most of these should just elog(ERROR) if a null attr is found). I think you're right here, yes. Another thing is that I don't find the name uptmpl very clear. Any suggestion is welcome, I don't like it very much either. Keeping the template.c file name seems wrong -- exttemplate.c maybe? (I renamed the parse nodes to ExtTemplate) I've been wondering about that. My thinking is that we're providing a new set of TEMPLATE objects and commands and the commands are designed so that it's easy to add more objects in there. The implementation too is quite open to that, with routing functions in template.c. Now I've choosen to implement the Extension templates in the same file because currently those are the only kind of Templates that we manage, and this project seems to prefer big files rather than too many files. That said, I'm not wedded to this choice. There was a strange bug in pg_dump; it used qto where I thought qversion was appropriate. I changed it (I looked at this hunk more closely than most others because there was a compiler warning here, but I didn't verify that it works.) It's quite hard for me to spot the hunk you're talking about without setting up a whole new branch to extract the work you did in the new diff, but maybe I'm just missing a diff-between-diffs tool? You seem to love using Capitalized Letters for some things in error messages; I don't find these very pretty, and anyway they violate our style guidelines. (I think these are in elog() not ereport() calls, but still) I'll make sure to remember about that, thanks. Regards, -- Dimitri Fontaine06 63 07 10 78 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] in-catalog Extension Scripts and Control parameters (templates?)
Alvaro Herrera alvhe...@2ndquadrant.com writes: Here's a rebased version; there were some merge conflicts with master. Um ... what's with those hstore changes? 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] in-catalog Extension Scripts and Control parameters (templates?)
Tom Lane t...@sss.pgh.pa.us writes: Um ... what's with those hstore changes? Just showing how we could deal with shipping 1.1 in the future, it's not meant to be applied as-is. Part of the feature set in there comes from when Robert complained that we can't have CREATE EXTENSION hstore; install version 1.1 from 1.0 plus 1.0--1.1 upgrade path. It so happens that with extension templates, that features is needed for pg_dump purposes, so it's included in this patch. I've left the hstore changes to show it off only. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] in-catalog Extension Scripts and Control parameters (templates?)
Craig Ringer cr...@2ndquadrant.com writes: There hasn't been visible movement on this work since the 22'nd when you posted v3 and it was flagged for further review. Nobody's stepped up, can we get any interest in this? I hope we can, it's a pretty important development as far as I'm concerned, a building block for other improvements that won't need further assistance from core code. What's your opinion on the state of this patch? Are you satisfied with the proposed patch as it stands? Any particular areas you think need attention in review or during final committer examination? Any security concerns? I think the patch is ready for a commiter. What I think the commiter will want to change is here: - hstore changes The patch reverts the hstore--1.1.sql changes to show that with the default_major_version included before, we could have chosen to ship with hstore--1.0.sql and hstore--1.0--1.1.sql and install 1.1 by default in more recent releases - docs We might need to add some more high-level docs about the feature, like a worked out example in the main Extension section (35.15), but I felt time pressed and that's typically something that can be done while in beta - catalog names This patch needs 3 new catalogs, named pg_extension_control, pg_extension_template and pg_extension_uptmpl for the Templates you use to Update an extension (not the same natural PK as the ones you use to insert). The decision to use 3 catalogs has been validated earlier by Tom. The focus point is on the naming: uptmpl is meant to be as short as possible while still being easy to understand. Is that the case? - psql support When compared to current EXTENSION facilities, psql support here would mean the ability to see an extension's scripts and control file from psql directly, and we didn't feel like we should add that after tall. So there's no psql support in that patch, other than including the TEMPLATEs in pg_available_extensions(). - pg_available_extension_versions() support Oooops, I didn't add that yet. Follow-up patch needed. Do we want a new full patch or just a patch on-top of that for later applying? This patch certainly is big enough as it is… - Assert() HeapTuple's catalog In the function extract_ctlversion() I would like to be able to Assert() that the given tuple is from the right catalog and didn't see how to do that Other than that, the patch implements 3 new catalogs and associated commands, and route those commands in a way that the new grammar additions are not tied to EXTENSION TEMPLATEs but rather generic as far as TEMPLATEs are concerned. Regards, -- Dimitri Fontaine06 63 07 10 78 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] in-catalog Extension Scripts and Control parameters (templates?)
On 2013-03-04 11:51:36 +0100, Dimitri Fontaine wrote: - Assert() HeapTuple's catalog In the function extract_ctlversion() I would like to be able to Assert() that the given tuple is from the right catalog and didn't see how to do that -t_tableOid. Haven't read the patch, so I am not sure whether thats a good check to make. Its not 100% useful, because several places neglect to set tableOid but I have patch to remedy that (as part of the logical decoding work). I can send that patch separated from other stuff if there's interest. Greetings, Andres Freund -- Andres Freund 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] in-catalog Extension Scripts and Control parameters (templates?)
On 02/23/2013 12:03 AM, Dimitri Fontaine wrote: Hi, Please find attached v3 of the Extension Templates patch, with full pg_dump support thanks to having merged default_full_version, appended with some regression tests now that it's possible. There hasn't been visible movement on this work since the 22'nd when you posted v3 and it was flagged for further review. Nobody's stepped up, can we get any interest in this? What's your opinion on the state of this patch? Are you satisfied with the proposed patch as it stands? Any particular areas you think need attention in review or during final committer examination? Any security concerns? -- Craig Ringer 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] in-catalog Extension Scripts and Control parameters (templates?)
Hi, Please find attached v3 of the Extension Templates patch, with full pg_dump support thanks to having merged default_full_version, appended with some regression tests now that it's possible. The patch also implements ALTER RENAME and OWNER facilities for those new templates objects. Dimitri Fontaine dimi...@2ndquadrant.fr writes: Now, back to Extension Templates: the pg_dump output from the attached patch is not smart enough to cope with an extension that has been upgraded, it will only install the *default* version of it. That's been fixed by merging in the default_full_version patch. There are two ways that I see about addressing that point: - implement default_full_version support for CREATE EXTENSION and have it working both in the case of file based installation and template based installation, then pg_dump work is really straightforward; CREATE EXTENSION pair VERSION '1.2'; -- will install 1.0 then update And that just works at pg_restore time, automatically, without pg_dump having to know anything about how. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support templates.v3.patch.gz Description: Binary data -- 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] in-catalog Extension Scripts and Control parameters (templates?)
Hi,
Please find attached v2 of the Extension Templates patch, with pg_dump
support and assorted fixes. It's still missing ALTER RENAME and OWNER
facilities, and owner in the dump. There's a design point I want to
address with some input before getting there, though. Hence this email.
Dimitri Fontaine dimi...@2ndquadrant.fr writes:
We now have those new catalogs:
- pg_extension_control
- pg_extension_template
- pg_extension_uptmpl
What I did here in pg_dump is adding a new dumpable object type
DO_EXTENSION_TEMPLATE where in fact we're fetching entries from
pg_extension_control and pg_extension_template and uptmpl.
The thing is that we now have a control entry for any script to play, so
that we can ALTER the control properties of any known target version.
Also, an extension installed from a template keeps a dependency towards
the control entry of that template, so that the dump is done with the
right ordering.
Now, the tricky part that's left over. Say that you have an extension
pair with 3 versions available, and those upgrade paths (edited for
brevity):
~# select * from pg_extension_update_paths('pair');
source | target | path
++---
1.0| 1.1| 1.0--1.1
1.0| 1.2| 1.0--1.1--1.2
1.1| 1.2| 1.1--1.2
CREATE EXTENSION pair VERSION '1.2';
PostgreSQL didn't know how to do that before, and still does not. That
feature is implemented in another patch of mine for 9.3, quietly waiting
for attention to get back to it, and answering to a gripe initially
expressed by Robert:
https://commitfest.postgresql.org/action/patch_view?id=968
Given the ability to install an extension from a default_version then
apply the update path to what the user asked, we would have been able
to ship hstore 1.0 and 1.0--1.1 script in 9.2, without having to
consider dropping the 1.0 version yet.
Now, back to Extension Templates: the pg_dump output from the attached
patch is not smart enough to cope with an extension that has been
upgraded, it will only install the *default* version of it.
There are two ways that I see about addressing that point:
- implement default_full_version support for CREATE EXTENSION and have
it working both in the case of file based installation and template
based installation, then pg_dump work is really straightforward;
CREATE EXTENSION pair VERSION '1.2'; -- will install 1.0 then update
- add smarts into pg_dump to understand the shortest path of
installation and upgrade going from the current default_version to
the currently installed version of a template based extension so as
to be able to produce the right order of commands, as e.g.:
CREATE EXTENSION pair;-- default is 1.0
ALTER EXTENSION pair UPDATE TO '1.2'; -- updates to 1.1 then 1.2
As you might have guessed already, if I'm going to implement some smarts
in the system to cope with installation time update paths, I'd rather do
it once in the backend rather than hack it together in pg_dump only for
the template based case.
Should I merge the default_full_version patch into the Extension
Template one, or would we rather first see about commiting the default
one then the template one, or the other way around, or something else I
didn't think about?
Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
templates.v2.patch.gz
Description: Binary data
--
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] in-catalog Extension Scripts and Control parameters (templates?)
Dimitri Fontaine dimi...@2ndquadrant.fr writes: Please find attached a new version of the patch, answering to most of your reviewing points. I'll post another version shortly with support for pg_dump and alter owner/rename. So, as far as pg_dump is concerned, I have a trick question here. We now have those new catalogs: - pg_extension_control - pg_extension_template - pg_extension_uptmpl When doing CREATE EXTENSION, if we did use a template to find the control information about it, we record a dependency. The template and update_template (named uptmpl to make the name shorter) catalog entries also have a pg_depend dependency towards the pg_extension_control. Now, at pg_dump time, I want to be dumping the templates for the extension and for updating the extension *before* the extension itself. It seems to me that the dependency as setup will guarantee that. The trick is that I don't have anything to dump for a given control entry itself. So I could either add some more commands so that pg_dump can setup the control then the template for creating or updating an extension, or just have a dumpExtensionControl() that does nothing. I'm not sure about which one to pick. Did I explain the problem properly enough for someone to chime in? Now that I've written this in that email, I think I'm going to go for the new command. But maybe we have some precedent for objects that we list in pg_dump only for solving several steps dependency lookups? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] in-catalog Extension Scripts and Control parameters (templates?)
Dimitri Fontaine dimi...@2ndquadrant.fr writes: Now that I've written this in that email, I think I'm going to go for the new command. But maybe we have some precedent for objects that we list in pg_dump only for solving several steps dependency lookups? Yes, pg_dump has lots of objects that might not appear in a dump. The most recent examples are the section fence objects ... 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] in-catalog Extension Scripts and Control parameters (templates?)
Hi,
Please find attached a new version of the patch, answering to most of
your reviewing points. I'll post another version shortly with support
for pg_dump and alter owner/rename.
The main priority was to confirm that the implementation is conforming
to the rought specs and design we agreed before with Tom and Heikki, in
order to be able to adjust anything I would have misunderstood there.
Stephen Frost sfr...@snowman.net writes:
What's with removing the OBJECT_TABLESPACE case? Given that
Merge artifact or fat fingers, something like that.
ExtensionControlFile seemed like a good name, just changing that to
ExtensionControl doesn't seem as nice, tho that's a bit of bike
shedding, I suppose.
Yeah, well, the values in there now can be fetched from a catalog, so I
though I should reflect that change somehow. Will revert to the previous
name if that's the consensus.
I'm not sure we have a 'dile system'... :)
Also pretty sure we only have one catalog
('get_ext_ver_list_from_catalogs')
Fixed.
'Template' seems like a really broad term which might end up being
associated with things beyond extensions, yet there are a number of
places where you just use 'TEMPLATE', eg, ACL_KIND_TEMPLATE. Seems like
it might become an issue later.
Fixed. Any other straight TEMPLATE reference would be another error when
cleaning up the patch, though my reading tonight didn't catch'em.
Also, no pg_dump/restore support..? Seems like that'd be useful..
Yeah, that's the obvious next step. The design is that we absolutely
want to dump and restore those templates, that's the key here :)
That's just a real quick run-through with my notes. If this patch is
really gonna go into 9.3, I'll try to take a deeper look.
Thanks for that!
Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
templates.v1.patch.gz
Description: Binary data
--
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] in-catalog Extension Scripts and Control parameters (templates?)
* Dimitri Fontaine (dimi...@2ndquadrant.fr) wrote:
Please find attached a preliminary patch following the TEMPLATE ideas,
and thanks in particular to Tom and Heikki for a practical design about
how to solve that problem!
Given that it's preliminary and v0 and big and whatnot, it seems like
it should be bounced to post-9.3. Even so, I did take a look through
it, probably mostly because I'd really like to see this feature. :)
What's with removing the OBJECT_TABLESPACE case? Given that
get_object_address_tmpl() seems to mainly just fall into a couple of
case statements to split out the different options, I'm not sure that
having that function is useful, or perhaps it should be 2 distinct
functions?
ExtensionControlFile seemed like a good name, just changing that to
ExtensionControl doesn't seem as nice, tho that's a bit of bike
shedding, I suppose.
I'm not sure we have a 'dile system'... :)
For my 2c, I wish we could do something better than having to support
both on-disk conf files and in-database configs. Don't have any
particular solution to that tho.
Also pretty sure we only have one catalog
('get_ext_ver_list_from_catalogs')
'Template' seems like a really broad term which might end up being
associated with things beyond extensions, yet there are a number of
places where you just use 'TEMPLATE', eg, ACL_KIND_TEMPLATE. Seems like
it might become an issue later.
Just a side-note, there's also some whitespace issues.
Also, no pg_dump/restore support..? Seems like that'd be useful..
That's just a real quick run-through with my notes. If this patch is
really gonna go into 9.3, I'll try to take a deeper look.
Thanks,
Stephen
signature.asc
Description: Digital signature
Re: [HACKERS] in-catalog Extension Scripts and Control parameters (templates?)
On 2013-01-18 12:45:02 -0500, Stephen Frost wrote: * Dimitri Fontaine (dimi...@2ndquadrant.fr) wrote: Please find attached a preliminary patch following the TEMPLATE ideas, and thanks in particular to Tom and Heikki for a practical design about how to solve that problem! Given that it's preliminary and v0 and big and whatnot, it seems like it should be bounced to post-9.3. Even so, I did take a look through it, probably mostly because I'd really like to see this feature. :) To be fair, the patch start its life pretty early on in the cycle and only got really reviewed (and I think updated) later. I just got rewritten into this form based on review. 'Template' seems like a really broad term which might end up being associated with things beyond extensions, yet there are a number of places where you just use 'TEMPLATE', eg, ACL_KIND_TEMPLATE. Seems like it might become an issue later. I think Tom came up with that name and while several people (including me and I think also Dim) didn't really like it nobody has come up with a better name so far. Greetings, Andres Freund -- Andres Freund 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] in-catalog Extension Scripts and Control parameters (templates?)
* Andres Freund (and...@2ndquadrant.com) wrote: On 2013-01-18 12:45:02 -0500, Stephen Frost wrote: 'Template' seems like a really broad term which might end up being associated with things beyond extensions, yet there are a number of places where you just use 'TEMPLATE', eg, ACL_KIND_TEMPLATE. Seems like it might become an issue later. I think Tom came up with that name and while several people (including me and I think also Dim) didn't really like it nobody has come up with a better name so far. 'Extension Template' is fine, I was just objecting to places in the code where it just says 'TEMPLATE'. I imagine we might have some 'XXX Template' at some point in the future and then we'd have confusion between is this an *extension* template or an XXX template?. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] in-catalog Extension Scripts and Control parameters (templates?)
Stephen Frost sfr...@snowman.net writes: 'Extension Template' is fine, I was just objecting to places in the code where it just says 'TEMPLATE'. I imagine we might have some 'XXX Template' at some point in the future and then we'd have confusion between is this an *extension* template or an XXX template?. We already do: see text search templates. The code tends to call those TSTEMPLATEs, so I'd suggest ACL_KIND_EXTTEMPLATE or some such. I agree with Stephen's objection to use of the bare word template. 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] in-catalog Extension Scripts and Control parameters (templates?)
Tom Lane t...@sss.pgh.pa.us writes: We already do: see text search templates. The code tends to call those TSTEMPLATEs, so I'd suggest ACL_KIND_EXTTEMPLATE or some such. I agree with Stephen's objection to use of the bare word template. Yes, me too, but I had a hard time to convince myself of using such a wordy notation. I will adjust the patch. Is that all I have to adjust before finishing the command set support? :) Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] in-catalog Extension Scripts and Control parameters (templates?)
* Dimitri Fontaine (dimi...@2ndquadrant.fr) wrote: Tom Lane t...@sss.pgh.pa.us writes: We already do: see text search templates. The code tends to call those TSTEMPLATEs, so I'd suggest ACL_KIND_EXTTEMPLATE or some such. I agree with Stephen's objection to use of the bare word template. Yes, me too, but I had a hard time to convince myself of using such a wordy notation. I will adjust the patch. Is that all I have to adjust before finishing the command set support? :) I'm keeping a healthy distance away from *that*.. ;) Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] in-catalog Extension Scripts and Control parameters (templates?)
Dimitri Fontaine dimi...@2ndquadrant.fr writes: Please find attached a preliminary patch following the TEMPLATE ideas, FYI, I've added it to the commitfest: https://commitfest.postgresql.org/action/patch_view?id=1032 Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] in-catalog Extension Scripts and Control parameters (templates?)
Hi, Please find attached a preliminary patch following the TEMPLATE ideas, and thanks in particular to Tom and Heikki for a practical design about how to solve that problem! Tom Lane t...@sss.pgh.pa.us writes: - Extension Scripts are now stored in the catalogs, right? Only for these new-style thingies. I am not suggesting breaking the existing file-based implementation, only offering a parallel catalog-based implementation too. We'd have to think about what to do for name collisions --- probably having the catalog entry take precedence is okay, but is there an argument for something else? The attached patch is implementing TEMPLATEs only for these new-style thingies. Conflicts are checked at template creation time, and at create extension time we do the file system lookup first, so that's the winner. [ need separate catalogs for install scripts and update scripts ] Check. You'll find the 3 of them in the attached unfinished patch (install, update, control). Actually, given the text search precedent, I'm not sure why you're so against TEMPLATE. So I called them TEMPLATE and I tried hard to leave that term open to other uses. As a result the main syntax is CREATE TEMPLATE FOR EXTENSION … ALTER TEMPLATE FOR EXTENSION … DROP TEMPLATE FOR EXTENSION … No new keyword has been added to the parser in the making of this patch. You'll find some usage examples in the regression tests part of the patch, and the new commands have received the very minimum documentation coverage. I intend to fill in the docs some more before calling it ready for commit, of course. I'm at a point where I need feedback before continuing though, and I think Tom is in the best position to provide it given the previous exchanges. The $2.56 question being what would be the pg_dump policy of the extension templates objects? The ones that are catalog objects, not file objects, should be dumped I think. So, the current version of the patch has no support for pg_dump and psql yet, and most ALTER commands in the grammar are not yet implemented. In the lacking list we can also add ALTER … OWNER TO / RENAME and COMMENT, both for the new catalog objects and the extension to be created from them. I think we could transfer the COMMENT on the template from the pg_extension_control (so that you can change the comment at upgrade) to the extension, but wanted to talk about that first. The alternative is to simply add a comment column to the pg_extension_control catalog, along with a grammar rule to get the information from the commands. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support templates.v0.patch.gz Description: Binary data -- 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] in-catalog Extension Scripts and Control parameters (templates?)
Dimitri Fontaine dimi...@2ndquadrant.fr writes: Tom Lane t...@sss.pgh.pa.us writes: I think a separate kind of extension template object would make a lot more sense. I'm on board now. We still have some questions to answer, and here's a worked out design proposal for implementing my understanding of your extension's template idea: - Extension Scripts are now stored in the catalogs, right? Only for these new-style thingies. I am not suggesting breaking the existing file-based implementation, only offering a parallel catalog-based implementation too. We'd have to think about what to do for name collisions --- probably having the catalog entry take precedence is okay, but is there an argument for something else? [ need separate catalogs for install scripts and update scripts ] Check. pg_extension_control(extension, version, default_version, default_full_version, module_pathname, relocatable, superuser, schema, requires) Given that the feature is going to be restricted to pure-SQL extensions, I'm pretty sure we can do without module_pathname, and maybe some other things. - The naming TEMPLATE appears to me to be too much of a generic naming for our usage here, so I'm not sure about it yet. Yeah, possibly, but I don't have a better idea yet. I don't like either PARAMETERS or SCRIPT --- for one thing, those don't convey the idea that this is an object in its own right rather than an attribute of an extension. Oh actually TEMPLATE is already a keyword thanks to text search, Actually, given the text search precedent, I'm not sure why you're so against TEMPLATE. That would mean that ALTER EXTENSION could create objects in other catalogs for an extension that does not exists itself yet, but is now known available (select * from pg_available_extensions()). Man, that is just horrid. It brings back exactly the confusion we're trying to eliminate by using the template terminology. We don't want it to look like manipulating a template has anything to do with altering an extension of the same name (which might or might not even be installed). The $2.56 question being what would be the pg_dump policy of the extension templates objects? The ones that are catalog objects, not file objects, should be dumped I think. Now, my understanding is that CREATE EXTENSION would check for templates being already available in the catalogs and failing to find them would have to do the extra steps of creating them from disk files as a preparatory step, right? (backward compatibility requirement) Wrong. There is no reason whatsoever to load file-based stuff into catalogs. That just adds complication and overhead to cases that work already, and will break update cases (what happens when a package update changes the files?). I don't think we could easily match a .so with an extension's template so I won't be proposing that, but we could quite easily match them now with extensions, because we're going to have to LOAD the module while creating_extension = true. One more time: this mode has nothing to do with extensions that involve a .so. It's for extensions that can be represented purely as scripts, and thus are self-contained in the proposed catalog entries. 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] in-catalog Extension Scripts and Control parameters (templates?)
Tom Lane t...@sss.pgh.pa.us writes: Only for these new-style thingies. I am not suggesting breaking the existing file-based implementation, only offering a parallel catalog-based implementation too. We'd have to think about what to do for name collisions --- probably having the catalog entry take precedence is okay, but is there an argument for something else? Yeah, well, I would have prefered to have two ways to fill-in the templates then only work from the templates. That would solve the name collision problem, and I guess would allow to share more code. The other thing is that I want to support extensions that use both models: say the prototype has been written in pl/pythonu but the next version now is switching to C/.so… [ need separate catalogs for install scripts and update scripts ] Check. Ok. pg_extension_control(extension, version, default_version, default_full_version, module_pathname, relocatable, superuser, schema, requires) Given that the feature is going to be restricted to pure-SQL extensions, I'm pretty sure we can do without module_pathname, and maybe some other things. I already removed directory from that list beause once in the catalogs you don't care where the files might have been found. MODULE_PATHNAME is about how to read the script that we would store in the catalogs, not sure we can bypass that. Yeah, possibly, but I don't have a better idea yet. I don't like either PARAMETERS or SCRIPT --- for one thing, those don't convey the idea that this is an object in its own right rather than an attribute of an extension. A template is something that needs to be instanciated with specific parameters, and can get used any number of times with different sets of parameters to build each time a new object. It's nothing like what we're talking about, or I don't understand it at all. My understanding is that we store the extension sources in our catalogs so as to be able to execute them later. The only option we have that looks like a template parameter would be the SCHEMA, the others are about picking the right sources/script. Actually, given the text search precedent, I'm not sure why you're so against TEMPLATE. See above, my understanding of your proposal is not matching the definition I know of that term. That would mean that ALTER EXTENSION could create objects in other catalogs for an extension that does not exists itself yet, but is now known available (select * from pg_available_extensions()). Man, that is just horrid. It brings back exactly the confusion we're trying to eliminate by using the template terminology. We don't want it to look like manipulating a template has anything to do with altering an extension of the same name (which might or might not even be installed). I still can't help but thinking in terms of populating the templates one way or the other and then using the templates to create or update the extension itself. We could maybe have a command akin to yum update or apt-get update that would refresh the TEMPLATEs from disk (handling name conflicts, file name parsing and control files parsing), and some options to the EXTENSION commands to force a refresh before working? So either REFRESH EXTENSION TEMPLATES; ALTER EXTENSION hstore UPDATE TO '1.2'; or ALTER EXTENSION hstore UPDATE TO '1.2' WITH TEMPLATE REFRESH; So my horrid proposal above would mean that the REFRESH option defaults to true, and is also available to CREATE EXTENSION. I'm not sure how much less horrid that makes it, but I sure hope it allows to better explain / convey my vision about the thing. The $2.56 question being what would be the pg_dump policy of the extension templates objects? The ones that are catalog objects, not file objects, should be dumped I think. Agreed. Wrong. There is no reason whatsoever to load file-based stuff into catalogs. That just adds complication and overhead to cases that work already, and will break update cases (what happens when a package update changes the files?). What happens if the extension that was a created from a template is now maintained on-disk (switch from pl/perlu to C)? What if the extension that was on-disk because you couldn't use a template in 9.1 and 9.2 now wants to be managed by the template system? What if the PGXN guys think template are a perfect solution to integrate into their client tool but the debian and yum packagers prefer to ship disk-based extensions? And you want to switch from one packaging system to the other? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
